Hacker News Reader

Comment by remram | Hacker News Reader

This returns 0 for '1.2.3' and '1.2'. Might be fine if you are sure they have the same number of components, but otherwise is worth checking.

View on HN

Another slightly more general approach, which turns the string into a tuple of numbers and then uses tuple comparison, which gets "1.2.3" > "1.2" right, but does yield "1.2.0" > "1.2" which may or may not be what you want:

  v1 = tuple(map(int, v1.split('.')))
  v2 = tuple(map(int, v2.split('.')))
  return (v1 > v2) - (v1 < v2)

(In Python 2 days, the last line could have been `return cmp(v1, v2)`, but sadly cmp() and .__cmp__() were removed in Python 3.)

And both of these implementations demonstrate an advantage of using someone else’s library: it has probably had more care put into it. And implementing these sorts of things yourself often leads you to compromise on functionality—though at the same time, using a misfitting library also leads to compromises. It can always go both ways.

I think you're demonstrating why libraries are slower.

For the specific use-case the OP had, the sorts of comparisons you're doing are unnecessary, so a custom-tuned compare is faster.

My experience is that it’s frightfully common for people to unintentionally oversimplify their implementation, or to forget that their implementation is too specific and improperly use it in a more general context. For example, to think they are only dealing with /^[0-9][0-9][.][0-9][0-9][.][0-9][0-9][.][0-9][0-9][0-9][0-9]$/ (and if the syntax really is that simple, then simple str comparison would be enough, as another comment suggested), but then discover somewhere down the line that perhaps some of the two-digit components can actually grow to three digits, or perhaps some suffix is added, or an additional component; or perhaps just use it for a different type of version string somewhere else in the program. Perhaps years later. (Mind you, if it violates your format expectations it may be better to immediately raise ValueError rather than using potentially-different semantics as you’d get if you used, say, packaging.version.parse().) As it stands, the provided implementation didn’t mention its limitations in its documentation. That is bad and makes it much more likely to be used improperly. It should have said something like “compares version numbers in 'XX.XX.XX.XXXX' format”.

(I’m not speaking for BYO or library philosophies, merely describing considerations and caveats of both.)

I think you're demonstrating why libraries are slower.

… because they’re probably better thought-out and less fragile?

Sure, that, among other reasons.

If the goal is to always have the best thought-out and least-fragile solution for any given code operation, then the trade-offs are that it's possible the library solution is going to not fit quite as perfectly as it night, and it will almost certainly be slower.

GP had made clear the format was fixed, and writing 1 or 2 tests for it takes seconds, so I don't really agree with your conclusion.

Personally I mostly wanted to offer a warning to somebody else who might take this code.

For our purposes, we never have strings that are other than the form XX.XX.XX.XXXX so there was no reason to generalize. Which improves the speed, makes it easier to read, and provides all the more reason not to use a library.

Meanwhile I've had to make changes to hand-rolled version comparison code in multiple systems because the original developers didn't account for the second segment of the version being greater than 9.

I'm absolutely not saying that you should have used a library, but pointing out the other side of the problem. If they had used an existing library to handle the versions, it wouldn't have had this issue. And test cases wouldn't help here because if they didn't think to code for multiple digits in the second spot, they likely wouldn't have thought to test for it.

There's enough benefits and drawbacks to library vs roll-your-own that I don't rigidly stick to one way or the other.

Or, you would have the same issue and the library developer wouldn't have time/care to fix it...

Also good sweng practices would dictate to put it in one function that is used in all places so you only need to fix it once.

Reply on news.ycombinator.com