Version comparison function

Question

I have this version comparison function in java.

Any cleaner way to write this without all the nested 'if' statements?

Note that the suggestions must be in Java code only.

@Override
public int compareTo(Version o)
{
    int result = this.major.compareTo(o.major);
    if (result == 0)
    {
        result = this.minor.compareTo(o.minor);
        if (result == 0)
        {
            result = this.minor2.compareTo(o.minor2);
            if (result == 0)
            {
                result = this.suffix.compareTo(o.suffix);
            }
        }
    }

    return result;
}

Edit:

Just to be clear on the 'type' of the fields of the Version class:

Integer major, minor, minor2;
String suffix;

The nesting pretty much reflects the cascading structure of the data - why do you want to get rid of it? PHP-Coder shows an legitimate solution but I wouldn't necessarily prefer it. — user unknown
– user unknown, Commented Apr 18, 2011 at 11:46
If your only concern is readability you may convert the version to a string and then just compare. — Inanc Gumus
– Inanc Gumus, Commented Apr 28, 2011 at 18:04
with a pure string comparison, 10.0.0 will be less than 9.0.0. — pdeva
– pdeva, Commented Apr 29, 2011 at 3:43
I just found a Java Library (Java SemVer) that supports the Semantic Versioning Specification. Download the library/source and use it. The usage is pretty simple. Version v1 = Version.valueOf("1.2.3"); Version v2 = Version.valueOf("1.2.0"); System.out.println(v1.compareTo(v2)); — sunil
– sunil, Commented Mar 18, 2015 at 11:27

user unknown · Accepted Answer · 2011-04-18 11:54:54Z

If you expect often, that two versions will be the same reference, you could start your method with a check for that. And you may strip the noisy 'this'. However, the cascading nature of the check is pretty fine expressed in the nesting - I would keep it.

@Override
public int compareTo (Version o)
{
    if (this == o)
        return 0;
    int result = major.compareTo (o.major);
    if (result == 0)
    {
        result = minor.compareTo (o.minor);
        if (result == 0)
        {
            result = minor2.compareTo (o.minor2);
            if (result == 0)
            {
                result = suffix.compareTo (o.suffix);
            }
        }
    }
    return result;
}

php-coder · Accepted Answer · 2011-05-09 16:38:51Z

13

One point which I may suggest is reorder if statements to reduce indentation:

@Override
public int compareTo(Version o)
{
    int result = this.major.compareTo(o.major);
    if (result != 0)
    {
        return result;
    }

    result = this.minor.compareTo(o.minor);
    if (result != 0)
    {
        return result;
    }

    result = this.minor2.compareTo(o.minor2);
    if (result != 0)
    {
         return result;
    }

    return this.suffix.compareTo(o.suffix);
}

Also, I think you can omit this keyword.

edited May 9, 2011 at 16:38

answered Apr 18, 2011 at 4:03

php-coder

1,0026 silver badges6 bronze badges

2

\$\begingroup\$ I agree about the suffix, and because of the premature return, no else is needed. But the cascading nature of the problem is imho clearer to see from nested structure. \$\endgroup\$

user unknown
– user unknown

2011-04-18 11:23:44 +00:00
Commented Apr 18, 2011 at 11:23

Add a comment |

Drahakar · Accepted Answer · 2011-04-17 20:51:39Z

5

In the apache.common package (very usefull by the way)

There is a CompareToBuilder. Works like a charm (like the ToStringBuilder)

http://commons.apache.org/lang/api-2.6/org/apache/commons/lang/builder/CompareToBuilder.html

edited Apr 17, 2011 at 20:51

answered Apr 17, 2011 at 20:39

Drahakar

1995 bronze badges

\$\begingroup\$ cant add dependency to a 3rd party library for a small function. Any improvement will have to be a refactoring of the code. \$\endgroup\$

pdeva
– pdeva

2011-04-17 20:41:25 +00:00
Commented Apr 17, 2011 at 20:41
\$\begingroup\$ Trust me, including this package will help your code on many different level, take a look at all the functions it offer. If you really can't include it in your code, then it is a good proof of concept that it can be done with reflexion. Answer to the question: So you could avoid all those if with reflexion. \$\endgroup\$

Drahakar
– Drahakar

2011-04-17 20:45:41 +00:00
Commented Apr 17, 2011 at 20:45
\$\begingroup\$ The down side is that reflexion is quite slow, compared with "normal" code. \$\endgroup\$

Drahakar
– Drahakar

2011-04-17 20:47:37 +00:00
Commented Apr 17, 2011 at 20:47
3

\$\begingroup\$ @OJ, bloat is all the code out there implementing this exact pattern without using a common code to do so. \$\endgroup\$

Winston Ewert
– Winston Ewert

2011-04-18 20:23:19 +00:00
Commented Apr 18, 2011 at 20:23
2

\$\begingroup\$ @Winston: Sorry, but I think that's utter rubbish. I don't need to construct a class, fill it with stuff and call a method just to compare a few elements together. Bloat is not "lack of code reuse", bloat is writing wrappers for extremely small things like this. Seriously, comparing internal values does not require "infrastructure" or "builder" classes like this. If you feel compelled to use them, fine, so long as I don't have to maintain it. I don't agree with rubbish like this being taught to others though. \$\endgroup\$

OJ.
– OJ.

2011-04-18 22:44:46 +00:00
Commented Apr 18, 2011 at 22:44

| Show 5 more comments

Carl Manaster · Accepted Answer · 2011-04-18 13:48:50Z

4

I like @php-coder's approach best, but I'd get rid of the unnecessary braces for even greater readability:

@Override public int compareTo(Version o) {
    if (this == o) return 0;

    int result = major.compareTo(o.major);
    if (result != 0) return result;

     result = minor.compareTo(o.minor);
     if (result != 0) return result;

     result = minor2.compareTo(o.minor2);
     if (result != 0) return result;

    return suffix.compareTo(o.suffix); 
}

Your compareTo method assumes that none of the fields can be null; watch out for that if you don't know for a fact it's always true.

answered Apr 18, 2011 at 13:48

Carl Manaster

1,31410 silver badges15 bronze badges

\$\begingroup\$ IMO it is a correct for the code to assume that o is non-null. It is best practice to assume that it is a programming error if o or a component field is null ... unless the javadocs clearly say otherwise. \$\endgroup\$

Stephen C
– Stephen C

2011-04-23 01:11:26 +00:00
Commented Apr 23, 2011 at 1:11

Add a comment |

Landei · Accepted Answer · 2011-04-23 19:21:00Z

This might be a little bit over-the-top, but how about a more object oriented approach? Assume this little helper enum:

public enum Compare {
    LT, EQ, GT;

    public int toInt() {
       switch(this) {
           case LT: return -1;
           case EQ: return 0;
           case GT: return 1;
           default: throw new AssertionError();    
       }    
    }

    public Compare cp(int v1, int v2) {
        if(this == EQ) return v1 < v2 ? LT : v1 > v2 ? GT : EQ;
        else return this;
    }

    //... here the cp methods for other primitives 

    public <T extends Comparable<T>> Compare cp(T v1, T v2) {
        if(this == EQ) {
            int result = v1.compareTo(v2);
            return result < 0 ? LT : result > 0 ? GT : EQ;
        } else {
            return this;
        }    
    }
}

Then we can write:

import static somepackage.Compare.*;

@Override
public int compareTo(Version that) {
   return EQ.cp(this.major, that.major)
            .cp(this.minor, that.minor)
            .cp(this.minor2, that.minor2)
            .toInt();
}

Note that unnecessary comparisions will be skipped in the cp methods.

[Update]

Apache Commons does something similar: http://commons.apache.org/lang/api-2.5/org/apache/commons/lang/builder/CompareToBuilder.html

I think it depends on the context. I wouldn't use this in performance critical code. However if I had a lot of big business objects, it could be more important to get comparision right than fast, and then this approach could be helpful. — Landei
– Landei, Commented Apr 23, 2011 at 13:14

Stack Exchange Network

Version comparison function

5 Answers 5

You must log in to answer this question.

Hot Network Questions

Version comparison function

5 Answers 5

You must log in to answer this question.

Related

Hot Network Questions