[PATCH] Amend divetrip model to use int units
Dirk Hohndel
dirk at hohndel.org
Wed Apr 24 10:36:18 PDT 2013
On Wed, 2013-04-24 at 16:57 +0100, amit.k.chaudhuri at gmail.com wrote:
> From: Amit Chaudhuri <amit.k.chaudhuri at gmail.com>
>
> Amend the DiveItem class to avoid float in favour of int. Add getters
> which return display friendly QStrings reflecting user preferences for
> (e.g.) depth.
>
> Modify DiveTripModel to support controlled alignment by column; right
> align for depth and duration.
>
> Fix problems with utf8 encoding on rating stars, degree symbols and
> O2 subscript.
>
> Signed-off-by: Amit Chaudhuri <amit.k.chaudhuri at gmail.com>
> ---
> diff --git a/qt-ui/models.cpp b/qt-ui/models.cpp
> index 40307d0..0ba0783 100644
> --- a/qt-ui/models.cpp
> +++ b/qt-ui/models.cpp
> @@ -8,6 +8,8 @@
> #include "../dive.h"
> #include "../divelist.h"
>
> +#include <QtDebug>
> +
> extern struct tank_info tank_info[100];
>
> CylindersModel::CylindersModel(QObject* parent): QAbstractTableModel(parent)
> @@ -292,14 +294,16 @@ void TankInfoModel::update()
> class DiveItem
> {
> public:
> - explicit DiveItem(): number(0), dateTime(QString()), duration(0.0), depth(0.0), location(QString()) {parentItem = 0;}
> - explicit DiveItem(int num, QString dt, float, float, QString loc, DiveItem *parent = 0);
> + explicit DiveItem(): number(0), dateTime(QString()), seconds(0), mm(0), location(QString()) {parentItem = 0;}
> + explicit DiveItem(int num, QString dt, int, int, QString loc, DiveItem *parent = 0);
Well, this is an improvement, but I wonder why we aren't using
Subsurface's native types here.
Or let me ask that actually as a question:
Would it be better to pass in duration_t, depth_t, etc? If "no", why?
> +QString DiveItem::displayDepth() const
> +{
> + const int scale = 1000;
> + QString fract, str;
> + if (get_units()->length == units::METERS){
> + fract = QString::number((unsigned)(mm%scale)/10);
> + str = QString("%1.%2").arg((unsigned)(mm/scale)).arg(fract);
> + }
> + if (get_units()->length == units::FEET){
> + str = QString::number(mm_to_feet(mm),'f',2);
> + }
> + return str;
> +}
YES! Ignoring whitespace (I really want a space on each side of a binary
operator - will fix) I really like this. Quick questions - what's the
advantage of using this vs. using the FRACTION macros that we already
have:
#define FRACTION(n,x) ((unsigned)(n)/(x)),((unsigned)(n)%(x))
ahh, I see... you need the ".arg().arg()" syntax. So we need a QFRACTION
macro :-)
We should be able to pass the number of decimals into this as an
optional argument - I'm sure Qt supports that...
displayDepth(mDecimal, fDecimal)
> +
> +QString DiveItem::displayDuration() const
> +{
> + int hrs, mins, secs, val;
> + const int minutes_hour = 60;
> + const int seconds_minute= 60;
> +
> + val = seconds;
> + secs = seconds % seconds_minute;
> + val /= seconds_minute;
> + mins = val % seconds_minute;
> + val /= minutes_hour;
> + hrs = val % minutes_hour;
> +
> + QString displayTime;
> + if (hrs > 0)
> + displayTime = QString("%1:%2:%3").arg(hrs).arg(mins).arg(secs);
> + else
> + displayTime = QString("%1:%2").arg(mins).arg(secs);
> +
> + return displayTime;
> +}
similarly here it would be nice to be able to say "round to next minute
and don't display seconds"
But overall I really like this. I'll push it after minor cleanup.
/D
More information about the subsurface
mailing list