Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I created this abstract class:

public abstract class MeasurementType {
  protected String name;
  protected String[] units;

  public MeasurementType() {}

  public MeasurementType(String name) {
    this.name = name;
  }

  protected void setName(String name) {
    this.name = name;
  }

  protected String getName() {
    return name;
  }

  protected void setUnits(String[] values) {
    this.units = values;
  }
}

Then I thought to create to other classes that were its subtypes:

public class Acceleration extends MeasurementType {
  private String name;

  public Acceleration () {
    super();
  }

  public Acceleration(String name) {
    super();
  }
}

public class Density extends MeasurementType {
  private String name;

  public Density() {
    super();
  }

  public Density(String name) {
    super();
  }
}

Plenty to choose from but these were the first two types that cam to my mind, the first question might be what the need for the subtypes are since there isn't anything in those classes that differentiates them apart enough (at least in my implementations) to give them the right to exist as their own entities, but for the sake of the example, lets say they had their own arrays containing units for each distinct type, so density would have kilograms, grams etc.. and acceleration would have mph, kmph etc..

Finally, I have this run class which I quickly made up:

public class run {

  public run() {}

  public static MeasurementType testType() {
    Acceleration a = new Acceleration("meters per second");
    return (MeasurementType) a;
  }

  public static void main(String[] args) {
    MeasurementType mt = testType();
    System.out.println(mt.getClass().toString());
    System.out.println(mt.getClass().getName());

    String type = "acceleration";
    String[] units = new String[8];

    Object mType;
    if(type.equals("acceleration")) {
      mType = new Acceleration();
    } else if(type.equals("density")) {
      mType = new Density();
    } else {
      mType = new Object();
    }

    System.out.println(mType.getName());
  }
}

This is where I encountered the problem that I was thinking about. I was using strings to determine the type that I would instantiate, but the problem is, by default right at the end of the conditional statements I instantiate Object because otherwise the compiler would complain that the mType variable wouldn't have been initialised. I can't create a MeasurementType object since it is an abstract class.

I guess this is more of a design question, but is there a better way of determining the class to instantiate? Please assume that the String type = "acceleration" came from a drop down or somewhere from some user interface, I have just hard coded it in because it isn't really necessary for the question.

Could someone please shed some light on this design fault as I see it please?

share|improve this question

2 Answers 2

Using strings in themselves to determine the MeasurementType isn't the biggest problem, but you really don't need to create classes and subclasses and stuff for this.

Instead, use an enum!

public enum MeasurementType {
    DENSITY, ACCELERATION;
}

In this case you can use this code to grab the MeasurementType:

String type = ...;
MeasurementType mType = MeasurementType.valueOf(type.toLowerCase());

Note however that this will throw an exception if the type is not found. So instead, you might want to consider using a Map<String, MeasurementType> where you add all known MeasurementTypes (preferably in a static initializer block), or you can loop through MeasurementType.values() and scan for the one with the right name.

Also remember that enums can have methods, implement interfaces, override methods, and contain fields.

As I'm not sure what your ultimate goal is, here's an example of what an enum can do:

public enum MeasurementType {
    DENSITY("test"),
    ACCELERATION("test2") {
        @Override
        public int transform(int value) {
            return 0;
        }
    },
    SOMETHING_ELSE("test3")
    ;

    private final String test;

    private MeasurementType(String test) {
        this.test = test;
    }

    public int transform(int value) {
        return value;
    }
}
share|improve this answer

The standard name for MeasurementType is Dimension.

Individual dimensions like acceleration should be instances of Dimension, not subclasses:

Dimension Acceleration = new Dimension("acceleration", "m s^-2");

If for some reason you need the subclasses, they shouldn't duplicate the name field. They already inherit that from their parent.

The default constructors like Density(), and the setters like setName, are not useful, because it's not meaningful to have a nameless dimension or change its name. Don't provide features that can never be useful.

Units should be an argument to the constructor, not something to set later.

The simplest way to find a Dimension from its name is to look it up in a Map:

Map<String, Dimension> dimensions = new HashMap<String, Dimension>();
dimensions.put("acceleration", acceleration);
dimensions.put("accel", acceleration);
dimensions.put("density", new Dimension("density", "kg m^-3");

Dimension d = dimensions.get("pressure");
if (d != null) ...

by default right at the end of the conditional statements I instantiate Object because otherwise the compiler would complain

Don't create a value of the wrong class to satisfy the compiler! Use null instead, or throw an exception, or avoid the variable by returning the dimension directly rather than assigning to a variable.

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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