3
\$\begingroup\$

I am refactoring a part of big old system developed in Java. I came across a situation where we are migrating some data types, and there will be use of both types. For compatibility, as all the old part will remain using id as an int, I simply overloaded the constructor, and the old getter now parse a integer value.

The thing is, how is the proper way to compare this data? Does this way make any sense, or is there a simpler way to compare it, considering that id can be both, a number or a word.

import java.util.ArrayList;
import java.util.List;
import org.apache.commons.lang3.StringUtils;

public class SomeItem implements Comparable<SomeItem> {
    private String id;    

    public SomeItem(String id) {
        this.id = id;
    }

    public SomeItem(int no) {
        this(String.valueOf(no));
    }

    public String getId() {
        return id;
    }

    public int getNo() {
        return Integer.parseInt(id);
    }

    @Override
    public int compareTo(SomeItem that) {
        if (StringUtils.isNumeric(this.getId()) && StringUtils.isNumeric(that.getId())) {
            return this.getNo() - that.getNo();
        } else {
            return this.getId().compareTo(that.getId());
        }
    }
}
\$\endgroup\$
  • \$\begingroup\$ Do you mean that the old parts will call getNo(), and the new parts will call getId()? \$\endgroup\$ – h.j.k. Mar 16 '16 at 1:01
  • \$\begingroup\$ Yes. The old part will continue the same way (which is getNo) as it's to massive for a total refactor at the moment, and everything that it's new will be using getId() \$\endgroup\$ – Patrick Bard Mar 16 '16 at 1:30
  • \$\begingroup\$ One thing I don't get: if the old parts will construct SomeItem via the int-based constructor, wouldn't that already solve your problem inside compareTo(SomeItem)? \$\endgroup\$ – h.j.k. Mar 16 '16 at 2:06
2
\$\begingroup\$
this.getNo() - that.getNo()

may violate the contract of compareTo() if the subtraction overflows or underflows which is a likely event if the compared values are integer IDs of a large set of elements.

Instead you should explicitly compare the two:

return Integer.compare(this.getNo(), that.getNo());

Another way to avoid numerical comparison altogether is to format numbers as fixed-width strings with a DecimalFormat and compare the resulting strings (including some example code at the end):

import java.text.DecimalFormat;

public class SomeItem implements Comparable<SomeItem> {

    private static final DecimalFormat fmt = new DecimalFormat();

    static {
        int len = 19; // = String.valueOf(Long.MAX_VALUE).length();
        fmt.setMinimumIntegerDigits(len);
        fmt.setMaximumIntegerDigits(len);
        fmt.setPositivePrefix("+");
        fmt.setGroupingUsed(false);
    }

    private String id;

    public SomeItem(String id) {
        this.id = id;
    }

    public SomeItem(int no) {
        this(fmt.format(no));
    }

    public String getId() {
        return id;
    }

    public int getNo() {
        return Integer.parseInt(id);
    }

    @Override
    public int compareTo(SomeItem that) {
        return this.getId().compareTo(that.getId());
    }

    public static void main(String[] args)
    {
        java.io.PrintStream o = System.out;

        SomeItem[] items = new SomeItem[] {
            new SomeItem(1),
            new SomeItem(-1),
            new SomeItem(Integer.MAX_VALUE),
            new SomeItem(Integer.MIN_VALUE)
        };

        for (SomeItem i: items)
            System.out.format("id=%s, no=%+020d%n", i.getId(), i.getNo());
    }
}

Additionally, I based the amount of digits on long instead of int to provide room for (future) growth. I would have replaced the latter by the former entirely if Java converted the return value of #getNo() silently from long to int.

\$\endgroup\$

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.