1
\$\begingroup\$

I was looking at this answer on SO to handle units in java. The concept was interesting to me, except I didn't like that it had to be per object that had mass, instead of encapsulating a Mass class on its own so the pattern need not be repeated. This is what I came up with.

public class Main
{

  public static void main(String args[]){
    Mass x = Mass.from_oz(2.3);
    System.out.println("x in kg: " + x.get_kg());
    System.out.println("x in oz: " + x.get_oz());

    Mass y = Mass.from_lb(1.2);
    System.out.println("y in kg: " + y.get_kg());
    System.out.println("y in lb: " + y.get_lb());

    Mass z = Mass.add(x, y); 
    System.out.println("x + y in kg: " + z.get_kg());

    z.add(Mass.from_kg(2.0));
    System.out.println("x + y + 2.0kg in kg: " + z.get_kg());

  }  
}

public class Mass{
    private double mass;
    private static double LB_PER_KG = 2.20462;
    private static double OZ_PER_LB = 16;

    private static double lb_to_kg(double value){
        return value * (1/LB_PER_KG);
    }
    private static double kg_to_lb(double value){
        return value * (LB_PER_KG);
    }
    private static double kg_to_oz(double value){
        return value * (LB_PER_KG*OZ_PER_LB);
    }
    private static double oz_to_kg(double value){
        return value * (1/(LB_PER_KG*OZ_PER_LB));
    }

    public double get_kg(){
        return this.mass;
    }
    public double get_lb(){
        return kg_to_lb(this.mass);
    }
    public double get_oz(){
        return kg_to_oz(this.mass);
    }


    public void set_kg(double value){
        this.mass = value;
    }
    public void set_lb(double value){
        this.mass = lb_to_kg(value);
    }
    public void set_oz(double value){
        this.mass = oz_to_kg(value);
    }


    private Mass(double value){
        set_kg(value);
    }

    public static Mass from_kg(double value){
        return new Mass(value);
    }
    public static Mass from_lb(double value){
        return new Mass(lb_to_kg(value));
    }
    public static Mass from_oz(double value){
        return new Mass(oz_to_kg(value));
    }

    public Mass add(Mass rhs){
        this.mass += rhs.mass;
        return this;
    }
    public Mass sub(Mass rhs){
        this.mass -= rhs.mass;
        return this;
    }
    public Mass div(Mass rhs){
        this.mass *= rhs.mass;
        return this;
    }
    public Mass mul(Mass rhs){
        this.mass /= rhs.mass;
        return this;
    }

    public static Mass add(Mass lhs, Mass rhs){
        return new Mass(lhs.mass).add(rhs);
    }
    public static Mass sub(Mass lhs, Mass rhs){
        return new Mass(lhs.mass).sub(rhs);
    }
    public static Mass mul(Mass lhs, Mass rhs){
        return new Mass(lhs.mass).mul(rhs);
    }
    public static Mass div(Mass lhs, Mass rhs){
        return new Mass(lhs.mass).div(rhs);
    }

}

Looking for any criticism, in addition to answers to these questions:

  • am I doing class arithmetic in java right? It doesn't have overloading of operators, so I'm not sure what the convention is for arithmetic.

  • I didn't use lower camel case for the unit part of functions and variables, despite lower camel case being the convention, I felt it was harder to see toKg or toKG especially when this theoretically could be used with different units where Nm and nm would be two different things.

  • is there a better way to manage the conversion functions, seem like I have to add two every time I add a new unit? what about units that are power of 10 convertible like for kg vs g? Is there a special pattern I could employ there (eg kg, g, mg, etc...)?

\$\endgroup\$
  • 1
    \$\begingroup\$ Mass/Mass would be unitless, Mass*Mass would be in \$ kg^2 \$ \$\endgroup\$ – AJNeufeld Aug 14 '18 at 0:24
  • \$\begingroup\$ @AJNeufeld Oh wow, I didn't think about that, you are right! the only reason you would divide or multiply is for unit-less scalars, \$\endgroup\$ – opa Aug 14 '18 at 2:12
  • 1
    \$\begingroup\$ Depends on what you are doing. Distance * distance is area. Distance/time is velocity. Velocity/time is acceleration. Acceleration*mass is force. If you are going for a full unit calculator, you’d have many base quantities (mass, distance, time, charge, temperature, lumens, ...) and a class Quantity would be a scalar combined with various powers of those base quantities (positive, negative and zero). Addition and subtraction would be defined only when the unit dimensions matched. Multiplication and division would always be defined. \$\endgroup\$ – AJNeufeld Aug 14 '18 at 2:23
2
\$\begingroup\$

For just Mass, I would define some constants.

public final static Mass kg = new Mass(1);
public final static Mass lb = new Mass(1/2.2, kg);
public final static Mass oz = new Mass(1/16.0, lb);

And define your conversions with the “to” (and “from”) units passed in.

public double in(Mass unit) {
    return mass / unit.mass;
}

public static double convert(double quantity, Mass from, Mass to) {
    return quantity * from.mass / to.mass;
}

Of course, you’d need to remove the “set” members, and make the Mass a constant value class.

private final double mass;

public Mass(double kg) {
    mass =  kg;
}

public Mass(double quantity, Mass unit) {
    mass = quantity * unit.mass;
}

New units just mean defining new class constants.

Mass weight = new Mass( 100, Mass.kg );
double pounds = weight.in( Mass.lb );
double ounces = weight.in( Mass.oz );

As described in the comments, you could expand this to a complete unit calculator, if you tracked the power of each unit dimension in the quantity class.

\$\endgroup\$
  • \$\begingroup\$ I'm not sure about the converter, ideally masses should only work with other masses so you could forget about the actual unit of masses, however using the units as a constant is much more manageable than making a new function for each unit. \$\endgroup\$ – opa Aug 14 '18 at 14:35
  • \$\begingroup\$ Also could you add the information about different units of measure resulting from division and multiplication in this answer? That was also a problem with my version, but as it stands that information only exists in comments (and could be deleted at any time). \$\endgroup\$ – opa Aug 14 '18 at 14:37
1
\$\begingroup\$

In addition to AJNeufeld's answer:

You have two add() methods.

public Mass add(Mass rhs){
    this.mass += rhs.mass;
    return this;
}
public static Mass add(Mass lhs, Mass rhs){
    return new Mass(lhs.mass).add(rhs);
}

The first one can be the cause of nasty debugging sessions as you modify the contents of "this" Mass while from the method signature (returning a Mass instance) your caller might expect a fresh Mass instance to be created and the two input instances to stay unmodified. Java programmers will typically expect that behaviour as they know e.g. Java's BigInteger.add() method.

When programming in Java, it's always worth to think about object identity. Generally, you should only provide methods that keep your current object's identity intact. E.g. if your Person instance represents Donald Trump, you shouldn't allow something like donaldTrump.setName("Barack Obama");, as that fundamentally changes the identity of that instance. It's not at all the same person, so it shouldn't be the same Java instance.

In your case, an instance of Mass represents some mass value. Have a look at the following lines of code using your current API:

Mass mass10kg = Mass.from_kg(10.0);
mass10kg.add(Mass.from_kg(15.0));

The second line modifies the Mass instance that previously represented 10kg to now represent 25kg. Is that still the same mass? Surely not, so I'd recommend to not do it that way.

With Java instances, it's a good idea to make the fields that define the object's identity final, so there's no chance to tamper with that identity. And then, your add() methods should read:

public Mass add(Mass rhs){
    return new Mass(this.mass + rhs.mass);
}
public static Mass add(Mass lhs, Mass rhs){
    return new Mass(lhs.mass + rhs.mass);
}

These two methods do quite the same thing, so one of them is redundant. We Java programmers are more used to the non-static style, so I'd eliminate the static method.

One more thing: I recommend to implement a toString() method, so Mass instances don't show up as "Mass@123abc456", but have a readable representation like e.g. "25 kg". And have your IDE create equals() and hashCode() implementations based on the field mass.

\$\endgroup\$
  • \$\begingroup\$ I don't come from java, the first behavior you describe would not be intuitive to me, what do you do if you want to += to a mass object? I also completely disagree with your second mass10kg example, the whole point is that you don't need to ever remember what the units where since they were the same underneath. You need to be able to apply arithmetic, and change mass values, otherwise that defeats the purpose of the whole framework. You certainly wouldn't want to say the units in the name (what the original question was hoping to avoid) and the amount is an implementation detail. \$\endgroup\$ – opa Aug 20 '18 at 13:07
  • \$\begingroup\$ You typically need to be able to change e.g. a person's mass, and not to change the Mass instance The Java way is person.mass = person.mass.add(m2);. It works by assigning a different Mass instance to the Person.mass field (in C terminology: make Person.mass point to a different Mass structure \$\endgroup\$ – Ralf Kleberhoff Aug 21 '18 at 19:55
  • \$\begingroup\$ How? You'd get an error for redefining variable name within same scope. \$\endgroup\$ – opa Aug 21 '18 at 19:59
  • \$\begingroup\$ I didn't believe this was common practice until looking at BigDecimal. \$\endgroup\$ – opa Aug 21 '18 at 20:05
  • \$\begingroup\$ Yes, it is common practice and has saved me a lot of debugging headaches... \$\endgroup\$ – Ralf Kleberhoff Aug 21 '18 at 20:06

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.