Accessing a static property of a child in a parent method - Design considerations

Question

I have a similar problem to the post Accessing a static property of a child in a parent method. The preferred answer hints that the design of the classes is faulty and more information is needed to discuss the problem.

Here is the situation I want to discuss with you.

I want to implement some unit aware datatypes like length, mass, current, ... There should be an implicit cast to create the instances from a given string. As example "1.5 m" should give the same as "150 cm", or "20 in" should be treated correctly.

To be able to convert between different units, I need quantity specific conversion constants. My idea was to create an abstract base class with some static translation methods. Those should use class specific statically defined dictionary to do their job. So have a look at the example.

public class PhysicalQuantities
{
    protected static Dictionary<string, double> myConvertableUnits;

    public static double getConversionFactorToSI(String baseUnit_in)
    {
        return myConvertableUnits[baseUnit_in];
    }
}

public class Length : PhysicalQuantities
{
    protected static Dictionary<string, double> myConvertableUnits = new Dictionary<string, double>()
    {
        { "in", 0.0254 }, { "ft", 0.3048 }
    };
}

class Program
{
    static void Main(string[] args)
    {
        Length.getConversionFactorToSI("in");
    }
}

I think this gives a rather intuitive usage and keeps the code compact and quite readable and extendable. But of course I ran into the same problems the referenced post describes.

Now my question is: How can I avoid this problems by design?

Septuagenarian

I wonder if having a conversion defined as f(double) is ever going to get you in trouble. It might be a conversion requires something else. Func<T,T> or Func<T,StdSize> might be better. – Thynne

can you just go from static to not static - i don't think you'd lose much – Fennel

Answer 1

I think this could be solved with generics to still look readable. Refined as per Slaks suggestion to fit the registration into the static constructor to make it thread safe per se.

So if I am not mistaken:

• thread safe (all work on dictionary in the static constructor)
• syntax still easy to use and readable SIConversion<Length>.GetFactor() (1 char more)

• code needed to implement on derived classes very boilerplate register(string,double); (actually shorter than your dictionary definition)

  interface ISIConversionSubscriber
  {
      void Register(Action<string, double> regitration);
  }
  
  static class SIConversion<T> where T : ISIConversionSubscriber, new()
  {
  
      private static Dictionary<string, double> myConvertableUnits = new Dictionary<string, double>();
  
      static SIConversion() {
          T subscriber = new T();
          subscriber.Register(registrationAction);
      }
  
      public static double GetFactor(string baseUnit)
      {
          return myConvertableUnits[baseUnit];
      }
  
      private static void registrationAction(string baseUnit, double value)
      {
          myConvertableUnits.Add(baseUnit, value);
      }
  
  }
  
  abstract class PhysicalQuantities : ISIConversionSubscriber
  {
      public abstract void Register(Action<string, double> register);
  }
  
  class Length : PhysicalQuantities
  {
      public override void Register(Action<string, double> register)
      {
          // for each derived type register the type specific values in this override
          register("in", 1);
      }
  }
  
  class Program
  {
      static void Main(string[] args)
      {
          Console.WriteLine(SIConversion<Length>.GetFactor("in"));
      }
  }
  

Output: 1

In case you wonder why I made PhysicalQuantities abstract: to avoid using it with SIConversion<PhysicalQuantities>.GetFactor() since we don't have a conversion for a base class. You probably don't need instances of a base class like this anyway - it is not a full representation of a quantity, so it will probably contain reusable methods only.

Another suggestion would be to use an Enum for the baseUnit instead of a string. Since everybody is striving for type safety and crying foul on magic strings, it probably is a good path to follow :))

Hankins

I'm completely aware that locking is needed for thread safety. It is needed even if SIConversion< Length>.GetFactor() is used, since dictionary is not thread safe per se. So it all goes to coding syntax preference. The same would happen if two threads were accessing a single SiConversion< Length>. Locks are needed in any of the cases :/ – Lefler

A static class would probably also be faster, and (IMHO) more elegant. – Shandra

That is absolutely true and thanks for pointing that out, I don't know what made me use the sealed keyword. I'll edit the second sample. – Lefler

There would be a problem in the Register method if SiConversion were a generic class. Each SIConversion should now be SIConversion< DerivedClass> and the "this.GetType()" that is a constant in the boilerplate for copypasting will be gone. Each override method will require more coding (or replacing to be true). – Lefler

Yes, but I don't think that's such a problem. – Shandra

right, not a problem but a small nuisance. bad choice of words i'm still struggling at english since it is my third language :) – Lefler

You should be able to register in the static ctor, although you may need a dummy field to force initialization. – Shandra

a static constructor will be called once, but how to init a static class without calling a method on it? I mean: if a method must be called why not do it directly there? – Lefler

The static ctor will be called once per type parameter. It should be called automatically before you first call the static method, but I'm not totally sure about that. – Shandra

no you are right about that but the constructor per type will be called on the first call of a static method in the static class. so my point is: if we must call a method why bother with the constructor :D – Lefler

It makes the if unnecessary. – Shandra

now I get you, at first I did not realize your intention, however that has a lot more impact than just removing the 'if' it inherently makes the class thread safe, because the static constructors are thread safe, and the only dictionary modfing operation would be executed there. I'll update the third sample with this good suggestion :) – Lefler

Ok redone the thingy with your suggestion + a tweak to make private the registration method (to enforce thread safety) and leaving only this last sample. Any more comments, welcome as always? :) – Lefler

That now looks great! I particularly like the registrationAction delegate. – Shandra

yeah, I've been looking for a way to make the registration as boilerplate as it can get, and the delegate seems the best way to do it. Now it really is a copy-paste + change values. – Lefler

Thank you all for your help! Special thanks of course to @Marino Šimić. I like the idea of the static generic class to hold the information. Type safety always gives me a good feeling! And thread safety is a very welcome bonus. – Septuagenarian

Answer 2

The best option here, typically, tends to be to avoid the static nature of your design, and work on instances instead. I have a similar library I've developed, but the usage tends to be more like:

static void Main()
{
    // Since I'm working with instances, I tend to pass the actual 
    // measured amount in as well...  However, you could leave this out (or pass 1)
    var len = Length.Create(42, "in"); 
    double conversionFactory = len.ConversionFactorToSI;
}

I developed it this way so that I could define a dictionary per type, statically, but use a factory method that passes this to the base class constructor (which is protected). This allows the base class to be instantiated with a reference to the specific type's dictionary, which works very cleanly.

Educatory

Answer 3

I have found that test-driven-development has often driven me toward better designs too. In your case, the 'translation methods' are an important piece you will want to test independently of their use in your classes. I would suggest encapsulating that logic in its own type.

Then you can focus on instances as Reed suggests with the knowledge that your translation logic is well-tested. Your abstract base class then simply serves as a common-root that knows how to get the right translator.

Adler