Handling ranges better - Githubissues

cannawen / metric_units_reddit_bot

Reddit bot converting imperial units to metric units

https://www.reddit.com/user/metric_units

GNU General Public License v3.0

78 stars 34 forks source link

Handling ranges better #61

Closed nalinbhardwaj closed 6 years ago

nalinbhardwaj commented 6 years ago

Replacing number object by numbers which contains an array allowing more intuitive handling of values in pairs(and in future, tuples).
Adding tests to properly test said changes.

Related issue: #41

TODO: Once this is merged, I'll make a separate PR to add support for joining keywords other than - since this PR is already quite large.

nalinbhardwaj commented 6 years ago

@cannawen I think I have made most of the changes I had to, 2 things I think I would like for you to note during these changes are:

At current line 473, distanceMap() was being passed a redundant "km" parameter, due #49 . I'm not sure if this was to-do change to distanceMap(), but I've removed it.
I've made a containerVal which is meant to represent the value in range that would affect the conversion unit, this is decided independently(hence it can be changed in future for each unit independently, max, min, whatever). Currently containerVal = max().

cannawen commented 6 years ago

Ah yes, thanks for removing the extra "km"! Also, looks like your branch needs a rebase or merge :( boo, conflicts.

nalinbhardwaj commented 6 years ago

@cannawen done all the changes, merge if they are fine to you. :) Let me know once the changes are live, I'll check once to make sure I didn't break anything not tested in tests.

cannawen commented 6 years ago

@nalinbhardwaj This is now live! v0.11.9