Closed jeremypw closed 2 years ago
@danrabbit I am not sure that the current approach is sufficiently scalable so any suggestions as to which external conversion database or package we could leverage would be welcome. Not sure whether we want to cover all possible conversions or just stick to common units? An advantage of the current method (maybe the only one) is that it handles ambiguous units.
Could probably reduce the number of factors by including dimensional analysis - i.e. since 1 liter = 1dm3, since 1dm = 0.1m we already know 1 liter = (0.1)^3 m3. Its a balance between a long table and more complex code.
Opening for review in order get ideas about desired scope and acceptable formats of input etc.
I am curious btw, why wouldn't we use an existing library like qalc
for this? No control over the input syntax and in some way harder to maintain because of a dependency on external code?
Re qalc
both reasons you suggested are right. The user input is not known to be a calculation or conversion so it is helpful to have control on how it is parsed and how ambiguities are dealt with. This is only intended to be quick
limited tool for common conversions (I included some more obscure units for demonstration/development purposes), in the same way that the calculator plugin is not intended to emulate the calculator app. I also wanted to avoid another dependency. A lot of the code is involved in parsing the input and you would probably have to keep a fair amount of that to get the required input for an external library (and you would have to parse the output).
There may be some scope for slimming down the code (especially if you reduce the flexibility regarding input or scope of conversions) but I think the size is manageable. Refinements like handling complex conversions by dimensional analysis would increase the code considerably and are probably outside the scope of this plugin.
The user can always type into Google or install e.g. Qalculate! for advanced stuff.
@peteruithoven Thanks for your efforts in reviewing this. I have addressed your comments so far and look forward to your further comments.
Matching with decimal point and unit description with embedded space:
@peteruithoven Thanks for the review - I'll go through your suggestions later today. Regarding plurals, this would be difficult to accomodate in a localizable way as there is no framework for reverse translation. Ignoring a trailing 's' would fail for e.g. 'inch'. So I would prefer to leave that for a future refinement if a reasonable solution can be found. Alternatives to =>
can be accomodated as long as they are unlikely to appear in other types of query.
Ah yeah the future improvement ideas should not be a part of this PR, let's get this out there first :) I'll probably create issues for them when this gets merged. Good points on the plurals.
@peteruithoven I think I have addressed most of your points, with the exception of regex parsing. It might take me some time to work out how to do that so you might consider merging this or pushing a commit if you have time.
Fixes #114
Allows conversion between measurement units (not variable conversions like currency)
<number><unit> => <unit>
or<unit> => <unit>
(whitespace ignored)<number>
is omitted it is assumed to be "1.0"Could maybe copy the database of units from e.g. the
units
package and parse this. This would give access to a very wide range of units (many of which are obscure) but the philosophy of that database is different - each unit is defined in terms of only one other unit (except some units are fundamental or are multidimensional) and prefixes are handled separately so determining the applicable factor is not at all straightforward.At present uses a hard-coded array of Unit structs that has a similar, but simpler structure to
definitions.units
from theUnits
package - each unit is defined in terms of only one other unit using simple integers or fractions where possible within a unit system. Each system/type of unit has a base_unit and the base_units of non-metric systems are all linked to the corresponding base metric unit. Links between the required units are traced to calculate the necessary conversion factor. This makes the code more complex but the table shorter. The calculation is no longer passed out tobc
.At present links are traced by simply iterating through the units as the table is not very large. In the unlikely event of performance becoming an issue the table could be used to construct a Map when a conversion is first requested.
The number of entries in the table is reduced by handling (metric) prefixes and dimensions (square, cubic) separately.
Unit testing has been included that includes validating the data format as well as testing the results of representative user input.
The total number of conversions is (theoretically) (
<no of prefixes> * <no of entries> * <no of dimensions) ^ 2
although a lot of these will be invalid (e.g. meter => kg) or not very useful (e.g. megainches). This is currently (20 34 3) ^ 2 = approx 4 million.Not handled so far includes (to be decided which, if any need be addressed):