1
\$\begingroup\$

I have a requirement to implement a generic UniqueIdentifier which can cover identifiers like OrderId, TrackingID, scannableId and other Ids. The ids, as they are now, are either Long or String.
Below is my implementation of the UniqueIdentifier class --

public class UniqueIdentifier {
    String stringValue;
    Long longValue;
    boolean isLong;
    boolean isString;

    public UniqueIdentifier(String value) {
        setStringValue(value);
        try 
        {
            setLongValue(Long.parseLong(value));
        }
        catch(NumberFormatException nex) 
        {
            setLong(false);
        }
    }
    public UniqueIdentifier(Long value) {
        setLongValue(value);
        setStringValue(Long.toString(value));
    }
    public String getStringValue() {
        return stringValue;
    }
    private void setStringValue(String stringValue) {
        this.stringValue = stringValue;
    }
    public Long getLongValue() {
        return longValue;
    }
    private void setLongValue(Long longValue) {
        this.longValue = longValue;
    }
    public boolean isLong() {
        return isLong;
    }
    private void setLong(boolean isLong) {
        this.isLong = isLong;
    }
    public boolean isString() {
        return isString;
    }
    private void setString(boolean isString) {
        this.isString = isString;
    }       
}

Is there anything that i can do better here

\$\endgroup\$
  • \$\begingroup\$ Are you using an ORM, hibernate/JPA or anyhing else? Give examples of how you would use this class. As it is, it's not very usable. Most important of all, where and how did this requirement for a generic unique identifier came? \$\endgroup\$ – abuzittin gillifirca Mar 28 '13 at 11:28
4
\$\begingroup\$

I think this one screams for usage of generics.

ie.

public class UniqueIdentifier<T> {
    T theValue;
    public UniqueIdentifier( T t ) {
        theValue = t;
    }
    public T getValue() {
        return theValue;
    }
}

Can even be extened with a nice factory method.

| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ Of course they might actually be needing just an Entity<T> where T is the type of the id, either Long or String. \$\endgroup\$ – abuzittin gillifirca Mar 28 '13 at 14:07
1
\$\begingroup\$

Although your requirements are not clear, namely, why unwrapped Long and String objects are not enough or if an identifier can be both simultaneously, I'd say that this would be better served by the following:

public abstract class UniqueIdentifier { /* no data */ }

public class LongIdentifier extends UniqueIdentifier { ... }
public class StringIdentifier extends UniqueIdentifier { ... }

In addition:

  • make sure you define equals and hashCode, and maybe, implement Comparable, as identifiers tend to be used as map keys

  • omit setters, making your identifiers immutable, again, for using them as map keys

You can check for the class of each object with instanceof, but you should worry if you need to use it a lot. Would such functionality be better server by additional methods in the base class?

| improve this answer | |
\$\endgroup\$
0
\$\begingroup\$

You can get rid of the isLong and isString fields, because they will always be false! They're initialised to false on construction of a UniqueIdentifier and you never set them to true.

This points to a deeper problem: the isLong and longValue have more to do with each other than your code makes clear (similarly for isString and stringValue). I think your intent is that longValue either contains a number (in which case isLong should be true), or longValue is null and isLong is false. If that's the case, you should get rid of the isLong field and setter, and replace the getter by

public boolean getLong() {
    return longValue == null;
}

There's a problem if you want to be able to use UniqueIdentifier((Long) null) as something that contains an 'absent' Long value, but I think you should rethink your design if you need that.

You could also look into using the Optional<T> type from Guava or one of it's (many) variants.

| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ I can not follow this: "will always be false". If he calls setString(true), then it is true, not false. - I think you mistyped the method name in your code example, it should be isLong()? \$\endgroup\$ – tb- Mar 30 '13 at 3:37
  • \$\begingroup\$ setString(boolean) is a private method, and is never called from the code, so isString can't change from its initial value of false. \$\endgroup\$ – yatima2975 Mar 30 '13 at 4:23

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.