4
\$\begingroup\$

I am trying to decide between two designs of a class. The basic difference is the one calculates properties when they are called, and the other when the object is instantiated.

I would usually just take the first one, but the calculations call each other, so when calling all of them, they get recalculated multiple times.

I, however, still prefer the first code. It is cleaner. Easier, for me, to read. I also feel the calculations are probably small enough to not worry about as opposed to storing extra values.

Is there anything else I should be considering, or looking out for? Or any reason one is amazingly better than the other?

public class Angle1
{
    private decimal angle;

    public int Degree => (int)Math.Floor(angle);
    private decimal MinuteSecond => (angle - Degree) * 60;
    public int Minute => (int)Math.Floor(MinuteSecond);
    public decimal Second => (MinuteSecond - Minute) * 60;

    public Angle1(decimal angle)
    {
        this.angle = angle;
    }
}

public class Angle2
{
    private decimal angle;

    public int Degree { get; private set; }
    public int Minute { get; private set; }
    public decimal Second { get; private set; }

    public Angle2(decimal angle)
    {
        this.angle = angle;
        Degree = (int)Math.Floor(angle);
        var minSec = (angle - Degree) * 60;
        Minute = (int)Math.Floor(minSec);
        Second = (minSec - Minute) * 60;
    }
}

I have also thought of a "hybrid" option. Store values in private values. When a property is called the first time, the value is calculated, and the private value is set. When it is called again, the private value is returned.

This hybrid option is the one I naturally go to when properties are larger objects, datasets, or binary data. I feel in this instance, it would "dirty" the code more than necessary, for the size of the stored values (integers and decimals).

\$\endgroup\$
5
\$\begingroup\$

I'm leaning toward Angle2...


I also feel the calculations are probably small enough to not worry about

I infer that doing the calcs in the constructor is trivial as well - no problematic user-perceived lag, hesitation, slowness.


Communicating through design

How would I interpret - make an initial guess about - design, intent, and use from what I see here?

  • ... so when calling all of them, they get recalculated multiple times.

    • By doing something so obviously avoidable, there must be a reason. Somewhere.
  • Lazy loading, generally, suggests

    • the operation is time, CPU, whatever consuming to some degree
    • doing it all at once certainly is an issue
    • instantiation and/or re-calculation are not intrinsically inter-dependent.
    • angle is being mutated because there is no logic avoiding re-calc on every call.
    • an object is in a valid state with all, some, or none of the calculations done.
      • Client doesn't need guard code all over the place
      • Client does not have to know any particular call order.
        • If the client must know call order & all the involved properties to do it, why is Angle1 even a class?
  • Doing it all in the constructor suggests

    • This object is immutable
    • I'm guaranteeing valid state.
    • angle is not mutated
    • client code will be just that much cleaner, easier

however, still prefer the first code. It is cleaner. Easier, for me, to read.

You should write code that is "cleaner, and easier" to use.

Conclusion: Angle2 is better.

| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ There's another purpose of the lazy loading which may make Angle1 preferable in some circumstances. Lazy calculations are invaluable if memory is a major constraint. Angle2 has a much larger memory footprint. If the programmer may instantiate a few million Angle instances, Angle1 naturally communicates that intent. This is especially clear in examples like this where we really have one object that can be viewed two ways (a decimal angle or a DMS angle) \$\endgroup\$ – Cort Ammon Nov 5 '16 at 2:24
  • \$\begingroup\$ I really can't see the difference in using the two, there is a chance I missed something, but my intent was that from the outside, the two should be indistinguishable. \$\endgroup\$ – JonathanPeel Nov 5 '16 at 6:48
  • \$\begingroup\$ Yes, In this case the API is the same . What I meant to say is picking Angle1 because "it is cleaner, easier, and read-ier" is completely dismissing appropriate design considerations. Scale that to a million lines of code then the lack of coherence and consistency hits you right between the frontal lobes and future you is in for many, many hours of WTF-ery. \$\endgroup\$ – radarbob Nov 5 '16 at 17:44
2
\$\begingroup\$

In addition to your comparison...

Serialization

In case you need (or will need in the future) the class to be serializable, the first option is amazingly better than the other, because you don't need to worry about properties at all. All you need is to apply the [Serializable] attribute to the class.

[Serializable]
public class Angle1
{
    private readonly decimal angle;

    ...
}

While the second option requires you to create a method marked with the [OnDeserialized] attribute that duplicates the constructor's functionality.

The "hybrid" (or "lazy") option is suitable for serialization too.


Immutability

I'd prefer to make my classes and structs as immutable as possible. If it makes sense for you too, I suggest to add the readonly modifier to all the fields and to make all the properties get-only, since this guarantees that the class is immutable.

public class Angle2
{
    private readonly decimal angle;

    public int Degree { get; }
    public int Minute { get; }
    public decimal Second { get; }

    ...
}
| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ the first option is amazingly better than the other, because you don't need to worry about properties at all - they both use properties but the first one calculates the values each time where the other pre-calculates the values. Both classes have publicly read-only properties. I don't understand how [Serializable] should discover this and treat them differently. Could you elaborate? However my gut tells me that you thought the second one uses fields? \$\endgroup\$ – t3chb0t Nov 4 '16 at 20:50
  • \$\begingroup\$ @t3chb0t No, I don't think the second option uses fields. When the second one is deserialized, the only member that can be deserialized automatically is the field angle, and all the properties remain their default values. So you need to implement deserialization yourself to fill the properties. And that deserialization method will duplicate the constructor. \$\endgroup\$ – Dmitry Nov 4 '16 at 21:01
  • \$\begingroup\$ Ok, makes sense. I've never used this attribute before. It's of no use anyway ;-] \$\endgroup\$ – t3chb0t Nov 4 '16 at 21:03
1
\$\begingroup\$

You didn't tell us whether performance matters or not and how you plan to use it. This is actually the only thing that might favour precalculated values. If you don't use this class in any critical routine then either one is ok and a matter of taste.

In some contexts you might even consider to use a struct instead of a class.

| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ If I had a particular need, or priority, regarding memory, or performance, it would be easier to decide between the two. I considered a struct with the first design, because it is only storing one decimal value. \$\endgroup\$ – JonathanPeel Nov 5 '16 at 6:55

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.