2
\$\begingroup\$

I have written a function that compares two version number strings (e.g. 1.2.0 and 1.2.2) and return 1 if the first string is greater, -1 if the second string is greater and 0 if the strings are equal for a code challenge that I'm attempting.

Also, it's guaranteed that both strings contain an equal number of numeric fields and all decimals are non-negative.

So,

  • 1.2.0 and 1.2.2 should return -1.
  • 1.05.4 and 1.5.3 should return 1.
  • 1.2.6.76 and 1.2.06.0076 should return 0.

My logic for the function was to simply split the string at each occurrence of the period ., compare each number and then add a character (e for equal, m for more and l for less) to a temporary variable z depending on the comparison result.

I then simply use some basic regex to return 1, -1 or 0 based on the content of the z variable as can be seen in the following Code Snippet:

function checkVersion(a,b) {
    let x=a.split('.').map(e=> parseInt(e));
    let y=b.split('.').map(e=> parseInt(e));
    let z = "";

    for(i=0;i<x.length;i++) {
        if(x[i] === y[i]) {
            z+="e";
        } else
        if(x[i] > y[i]) {
            z+="m";
        } else {
            z+="l";
        }
    }
    if (!z.match(/[l|m]/g)) {
      return 0;
    } else if (!z.match(/[l]/g)) {
      return 1;
    } else {
      return -1;
    }
}

console.log(checkVersion("1.2.2","1.2.0")); // returns 1 as expected
console.log(checkVersion("1.0.5","1.1.0")); // returns -1 as expected
console.log(checkVersion("1.0.5","1.00.05")); // returns 0 as expected
console.log(checkVersion("0.9.9.9.9.9.9","1.0.0.0.0.0.0")) // returns -1 as expected;

The above function seems to be working fine with any random two version numbers that I've tried so far but when I try to submit the above function for the challenge, there is always one hidden test with two unknown version numbers that keeps failing. What logic am I missing in the above code?


EDIT:

Thanks to @RomanPerekhrest's comment above, I have found out that my regex is the problem. Instead of using the 2nd regex, I just remove any occurence of e from the z variable using the split() method and then just check if the first character is m or l and now the function is working correctly as seen in the following Code Snippet:

function checkVersion(a,b) {
    let x=a.split('.').map(e=> parseInt(e));
    let y=b.split('.').map(e=> parseInt(e));
    let z = "";

    for(i=0;i<x.length;i++) {
        if(x[i] === y[i]) {
            z+="e";
        } else
        if(x[i] > y[i]) {
            z+="m";
        } else {
            z+="l";
        }
    }
    if (!z.match(/[l|m]/g)) {
      return 0;
    } else if (z.split('e').join('')[0] == "m") {
      return 1;
    } else {
      return -1;
    }
}

console.log(checkVersion("2.0.5","1.0.15")); // returns 1 as expected
console.log(checkVersion("1.2.2","1.2.0")); // returns 1 as expected
console.log(checkVersion("1.0.5","1.1.0")); // returns -1 as expected
console.log(checkVersion("1.0.5","1.00.05")); // returns 0 as expected
console.log(checkVersion("0.9.9.9.9.9.9","1.0.0.0.0.0.0")) // returns -1 as expected;


However, I still feel like there must be a shorter, more concise and cleaner way of doing this though. Any suggestions?

\$\endgroup\$
  • 1
    \$\begingroup\$ Check this console.log(checkVersion("2.0.5","1.0.15")); - it'll give -1 while the expected result is 1 \$\endgroup\$ – RomanPerekhrest Feb 4 at 13:59
  • \$\begingroup\$ @RomanPerekhrest Thank you! I have found the flaw to be in the regex and managed to make it work by replacing the 2nd regex with another approach of stripping the e from the z variable and then checking the first letter. However, I still feel like the above function could be further improved though. Any suggestions? \$\endgroup\$ – AndrewL64 Feb 4 at 14:37
  • \$\begingroup\$ You can just compare the numeric parts one by one, left to right until they differ or end is reached (in which case the versions are equal). No need for regular expression obstructions. Anyway you may want to check semver and adjust your algorithm to fit the semantic versioning rules. \$\endgroup\$ – slepic Feb 4 at 14:55
2
\$\begingroup\$

A small review;

  • Once you know that one version digit is larger than the other, you can exit immediately
  • You did not declare i with const or let
  • I would advise the use of a beautifier for your code, it's a bit compact in some places
  • The code does not handle well versions with different counts of digits
  • You should always pass the base, when you call parseInt

I wrote an alternative version with 2 extra tests;

function checkVersion(a, b) {
    const x = a.split('.').map(e => parseInt(e, 10));
    const y = b.split('.').map(e => parseInt(e, 10));

    for (const i in x) {
        y[i] = y[i] || 0;
        if (x[i] === y[i]) {
            continue;
        } else if (x[i] > y[i]) {
            return 1;
        } else {
            return -1;
        }
    }
    return y.length > x.length ? -1 : 0;
}

console.log(checkVersion("1.2.2", "1.2.0"), 1); // returns 1 as expected
console.log(checkVersion("1.0.5", "1.1.0"), -1); // returns -1 as expected
console.log(checkVersion("1.0.5", "1.00.05"), 0); // returns 0 as expected
console.log(checkVersion("0.9.9.9.9.9.9", "1.0.0.0.0.0.0"), -1) // returns -1 as expected;
console.log(checkVersion("1.0.5", "1.0"), 1); // returns 1 as expected
console.log(checkVersion("1.0", "1.0.5"), -1); // returns -1 as expected
console.log(checkVersion('2019.09', '2019.9'), 0) // returns 0

\$\endgroup\$
  • \$\begingroup\$ @konjin Nice! This looks way more concise than the one I did. But can I know what is the length of x and y in the ternary operator used for? For example, 1.0.5 and 1.00.05 both have different lengths but are the same value and 1.2.2 and 1.2.0 both have the same length but different values. So what exactly is the length comparison doing here? \$\endgroup\$ – AndrewL64 Feb 4 at 17:08
  • \$\begingroup\$ @AndrewL64 x.length is the number of dot separated numbers in the version string, not the length of the version string. x.length would be 2 for "1.0" and "1.00" but it would be 3 for "1.00.05". It does what is shown in the last test. \$\endgroup\$ – slepic Feb 4 at 17:12
  • \$\begingroup\$ checkVersion("1.0.0", "1.0") === 0 //expected 0 ; checkVersion("1.0", "1.0.0") === -1 //expected 0 \$\endgroup\$ – slepic Feb 4 at 17:18
  • \$\begingroup\$ @slepic The split() method returns an array so the length would give the number of dot seprated numbers without the dot yes. But I'm still a tad bit confused about what the ternary does exactly? \$\endgroup\$ – AndrewL64 Feb 4 at 17:19
  • 1
    \$\begingroup\$ What about checkVersion('2019.09', '2019.05')? The 09 is interpreted as octal number, for historic reasons. You should always add these to the test cases. \$\endgroup\$ – Roland Illig Feb 5 at 2:03

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.