[patch] Use subsurface integer units for dive item

Dirk Hohndel dirk at hohndel.org
Tue Apr 23 08:49:10 PDT 2013


On Tue, 2013-04-23 at 08:46 +0100, Amit Chaudhuri wrote:
> Hi Dirk,
> 
> 
> This patch swaps float for int in DiveItem to better align with
> subsurface.

As others have pointed out, this patch is relative to an older state of
the Qt branch - and as it happens, this code has been moved around and
actually rewritten...

Also, if I look at the code, I'm still not sure it does what we really
want. My vision was that the constructor would be called with
(timestamp_t, duration_t, depth_t, char *,...) and would maintain these
data types internally and only convert them to some other representation
when displaying things. And btw, we literally don't use floats for
displaying, either, we do integer math (look at the FRACTION macro).

The places we use floats are when we need to do actual conversions
(lbs/kg, pressures, etc). But we strive to keep things in integers as
much as we can. The reason for that is precision and reproducability.
That's why the types are defined the way they are (for example, salinity
is mass of 10 cubic meters of water... why? Because if people really
care about this then they want to be able to store values like 1.003
kg/l vs. 1.0025 kg/l - which translates into 10030 or 10025 kg for 10000
l and now nicely fits into an integer... so we never run into rounding
issues when storing these values...

> I also swapped tr() for Qtr_() to support the translations at the end
> of the process. This last step prevents compilation I think because
> Qtr_ is declared in qt-ui/common.h which I think is missing from
> origin/Qt.  But I could be wrong. I've had some odd stuff going on in
> another branch.
> 
> 
> If my assumption is wrong, please bounce the patch and I'll correct.

I saw you resend without the stupid Qtr_() thing (that was my brain
damage). But that still addresses the issue that this is relative to a
rather old state of the branch :-(

I hate seeing work like this wasted, but I don't think there's a
realistic way I can merge this. Please pull the latest Qt branch (and
talk to Tomaz as I know he has looked into this very question as well)

Thanks

/D




More information about the subsurface mailing list