2
\$\begingroup\$

"Version" a common notion in software, which helps to unique states of software development. I implemented a simple POJO for "version" in Java. I would like to have your review about this code. I'd like to have a Java class satisfying the following requirements:

  1. Composition. A version is composed by 3 parts: major, minor, and patch. All of them are required attributes.
  2. Digits. Each part must be an integer. Negative numbers are NOT allowed: a valid part value should be either 0 or positive number.
  3. Incrementation. Incrementing a version means incrementing the value of major, minor, or patch.
  4. Comparability. Versions are comparable. Such comparison should be numeral, and should not be lexicographical. For example, 0.0.10 is greater than 0.0.9.
  5. Serialization. A version object in Java should be able to serialize into a string representation. And a string representation can also be deserialized into a Java object.
  6. Reversibility. The serialization and deserialization of this object should be reversible. So an object should obtain its owner value after { serialization, deserialization }. So leading zeros are denied:
    1. Good: "0.0.1" -> v(0.0.1) -> "0.0.1"
    2. Bad: "0.0.01" -> v(0.0.1) -> "0.0.1" (not the same)
  7. Immutability. A version object should never be changed once its instantiated. (I don't know what would be the benefits, but I feel it's a good idea).

And here's my implementation:

package io.mincongh.maven;

import java.util.regex.Matcher;
import java.util.regex.Pattern;

/**
 * @author Mincong Huang
 */
public class Version implements Comparable<Version> {

  private final int major;

  private final int minor;

  private final int patch;

  /**
   * A version contains 3 groups of digits, separated by char '.'.
   * Each group of digits should be a positive integer, without
   * leading zeros.
   */
  private static final Pattern PATTERN =
      Pattern.compile("(0|[1-9]\\d*)\\.(0|[1-9]\\d*)\\.(0|[1-9]\\d*)");

  private Version(int major, int minor, int patch) {
    if (major < 0) {
      throw new IllegalArgumentException("Negative major version: " + major);
    }
    if (minor < 0) {
      throw new IllegalArgumentException("Negative minor version: " + minor);
    }
    if (patch < 0) {
      throw new IllegalArgumentException("Negative patch version: " + patch);
    }
    if (major == 0 && minor == 0 && patch == 0) {
      String msg = "At least one argument should be non-zero among major, minor, and patch.";
      throw new IllegalArgumentException(msg);
    }
    this.major = major;
    this.minor = minor;
    this.patch = patch;
  }

  public static Version of(int major, int minor, int patch) {
    return new Version(major, minor, patch);
  }

  public static Version parse(String version) {
    Matcher matcher = PATTERN.matcher(version);
    if (!matcher.matches()) {
      throw new IllegalArgumentException("Cannot parse version: " + version);
    }
    int M = Integer.parseInt(matcher.group(1));
    int m = Integer.parseInt(matcher.group(2));
    int p = Integer.parseInt(matcher.group(3));
    return new Version(M, m, p);
  }

  public Version nextMajor() {
    return new Version(major + 1, minor, patch);
  }

  public Version nextMinor() {
    return new Version(major, minor + 1, patch);
  }

  public Version nextPatch() {
    return new Version(major, minor, patch + 1);
  }

  public int getMajor() {
    return major;
  }

  public int getMinor() {
    return minor;
  }

  public int getPatch() {
    return patch;
  }

  public boolean isBefore(Version that) {
    return this.compareTo(that) < 0;
  }

  public boolean isAfter(Version that) {
    return this.compareTo(that) > 0;
  }

  @Override
  public boolean equals(Object o) {
    if (this == o) {
      return true;
    }
    if (!(o instanceof Version)) {
      return false;
    }

    Version version = (Version) o;

    if (major != version.major) {
      return false;
    }
    if (minor != version.minor) {
      return false;
    }
    return patch == version.patch;
  }

  @Override
  public int hashCode() {
    int result = major;
    result = 31 * result + minor;
    result = 31 * result + patch;
    return result;
  }

  @Override
  public int compareTo(Version that) {
    if (this.major != that.major) {
      return this.major - that.major;
    }
    if (this.minor != that.minor) {
      return this.minor - that.minor;
    }
    if (this.patch != that.patch) {
      return this.patch - that.patch;
    }
    return 0;
  }

  @Override
  public String toString() {
    return major + "." + minor + "." + patch;
  }

}

Example usages:

// Create a new version
Version a = Version.of(1, 0, 0);
Version b = Version.of(1, 0, 1);

// Compare
a.isBefore(b);
a.isAfter(b);

// Next versions
Version M = a.nextMajor();
Version m = a.nextMinor();
Version p = a.nextPatch();

What do you think about this implementation? Your review is very welcome. Thanks!

\$\endgroup\$
  • \$\begingroup\$ Did you know that in Maven versions, there's also the possibility of using a classifier, like 1.2.3-mystuff-SNAPSHOT ? I suppose you're happy without the classifiers? \$\endgroup\$ – Simon Forsberg Nov 21 '17 at 21:26
  • \$\begingroup\$ What are you planning to use this class for? \$\endgroup\$ – Simon Forsberg Nov 21 '17 at 21:26
  • \$\begingroup\$ This is just an exercise. I didn't know that there's a classifier, it would be better to include it and the SNAPSHOT. Is it possible to have timestamp too? Ideally, this class can handle the use cases of Maven. \$\endgroup\$ – Mincong Huang Nov 21 '17 at 21:29
  • 1
    \$\begingroup\$ As far as I know, timestamp is only included for SNAPSHOTs after you build a SNAPSHOT, I wouldn't say that they're a part of the version, unless you consider already built jars. So, maybe the right answer is both yes and no. \$\endgroup\$ – Simon Forsberg Nov 21 '17 at 21:33
1
\$\begingroup\$

Your code is very readable, well-formatted, very clear. Well done.

You have implemented hashCode and equals well and all other methods look well-implemented.

It's good that you define the Pattern only once. Your exceptions are clear and well-implemented.

If I have to complain about something, it would be these variable names:

int M = Integer.parseInt(matcher.group(1));
int m = Integer.parseInt(matcher.group(2));
int p = Integer.parseInt(matcher.group(3));

They could be major, minor, patch or parsedMajor etc.

One thing that you might want to think about is the implementation of nextMajor() and nextMinor(). Let's say that you have a version 1.2.3 and you bump the major version, does the next version then become 2.2.3? In your code, yes, but in reality that becomes 2.0.0. So your nextMajor should return new Version(major + 1, 0, 0) and nextMinor should return 1.3.0, so new Version(major, minor + 1, 0).

Besides this, again: Very well done!

For fun, I'd recommend that you implement support for classifier versions, like 1.2.3-mystuff-SNAPSHOT, otherwise I'd say you're ready for more complicated challenges because this one was apparently way too easy for you.

\$\endgroup\$
  • \$\begingroup\$ I didn't think about the problem of next versions. That's very important! And I'll try to implement the notion of classifier and snapshot, too. Thanks for your advice, Simon! \$\endgroup\$ – Mincong Huang Nov 21 '17 at 21:50

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.