[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