1
\$\begingroup\$

I have an abstract object and I need to test whether it is an instance of one of the several concrete classes. Every concrete class has a Value property that is not inherited.

The following points need to be true:

  • Single cast per class
  • Short circuit when one test is successful
  • No unnecessary casts

This is the solution I came up with:

public string Value { get; set; }

code in the setter of ObserverDto:

_observerDto = value; // underlying field
var oldValue = Value;

// single cast before test, goto to emulate if - else if
var booleanObserverDto = ObserverDto as BooleanObserverDto;
if (booleanObserverDto != null)
{
    Value = booleanObserverDto.CurrentValue == true ? booleanObserverDto.TrueText :
            booleanObserverDto.CurrentValue == false ? booleanObserverDto.FalseText : booleanObserverDto.CurrentValue.ToString();
    goto castDone;
}

var intObserverDto = ObserverDto as IntObserverDto;
if (intObserverDto != null)
{
    Value = intObserverDto.CurrentValue.ToString();
    goto castDone;
}

var decimalObserverDto = ObserverDto as DecimalObserverDto;
if (decimalObserverDto != null)
{
    Value = Convert.ToSingle(decimalObserverDto.CurrentValue).ToString(CultureInfo.CurrentCulture);
    goto castDone;
}

var stringObserverDto = ObserverDto as StringObserverDto;
if (stringObserverDto != null)
    Value = stringObserverDto.CurrentValue;

castDone:;

if(string.IsNullOrEmpty(Value))
    Value = "NULL";

if(!Value.Equals(oldValue))
    UpdateObserver(Name, Value);

I don't know of any possibility to have a statement between if and else if, but it might be better than using goto. Is there any way I could do a cast before an else if?

I can't change anything about the class structure or make it more generic because these classes are from a library I need to use.

\$\endgroup\$
  • \$\begingroup\$ Ugh labels... the whole casting code is pretty duplicate, make a TryCast method for that and it will look so much cleaner. Without labels. \$\endgroup\$ – Viezevingertjes Apr 4 '17 at 11:42
  • 3
    \$\begingroup\$ It's not just the setter, the whole design is odd with its Value = "NULL" and all those observers. You're doing something terribly wrong. \$\endgroup\$ – t3chb0t Apr 4 '17 at 14:02
  • \$\begingroup\$ @t3chb0t I mentioned it being a setter, in case anyone was wondering what value was (it's a keyword). The application being developed shows 'observers' and their values, it is totally unrelated to the observer pattern. \$\endgroup\$ – oddRaven Apr 4 '17 at 14:10
  • 2
    \$\begingroup\$ Obviously we don't expect you to post the whole project, but this code could use some more context. I'm guessing, for example, that ObserverDto is a property whose getter returns _observerDto, and that Value is a property something like public string Value { get; private set; }, but without definite confirmation of those (and maybe other similar) points it's senseless to review this code because we can't tell what impact suggested refactors would have on correctness. \$\endgroup\$ – Peter Taylor Apr 4 '17 at 14:23
  • \$\begingroup\$ @PeterTaylor You are right, I did not think of that. I editted the question. \$\endgroup\$ – oddRaven Apr 4 '17 at 14:43
5
\$\begingroup\$

Assuming that you have a good reason for not having an overriden GetValue() method on each sub-class, an extension method for each class, say, GetValue() should solve it.

public static class ObserverExtensions
{
  private static readonly IDictionary<Type, Func<ObserverDto, string>> _lookup = new Dictionary<Type, Func<ObserverDto, string>>
  {
     {typeof(BooleanObserverDto), dto => ((BooleanObserverDto)dto).GetValue() },
     {typeof(IntObserverDto), dto => ((IntObserverDto)dto).GetValue() },
     {typeof(DecimalObserverDto), dto => ((DecimalObserverDto)dto).GetValue() },
     {typeof(StringObserverDto), dto => ((StringObserverDto)dto).GetValue() },
  };

  public static string GetValue(this ObserverDto observerDto)
  {
       return _lookup[observerDto.GetType()](observerDto);
  }

  public static string GetValue(this BooleanObserverDto booleanObserverDto)
  {
     return booleanObserverDto.CurrentValue == true ? booleanObserverDto.TrueText :
            booleanObserverDto.CurrentValue == false ? booleanObserverDto.FalseText : booleanObserverDto.CurrentValue.ToString();
  }

  public static string GetValue(this IntObserverDto intObserverDto)
  {
     return intObserverDto.CurrentValue.ToString();
  }

  public static string GetValue(this DecimalObserverDto decimalObserverDto)
  {
     return Convert.ToSingle(decimalObserverDto.CurrentValue).ToString(CultureInfo.CurrentCulture);
  }

  public static string GetValue(this StringObserverDto stringObserverDto)
  {
     return stringObserverDto.CurrentValue;
  }
}


//...
Value = ObserverDto.GetValue();

One nice thing about this solution is that we can add more classes (say, DateTimeObserverDto) without having to open up the set/update code. We simply add another extension method.

Edit: Correction

The extension methods only work if the references to the objects are of the concrete type. It doesn't work when passing in ObserverDto objects

// Passed
[TestMethod]
public void CheckConversions()
{
     var a = new IntObserverDto { CurrentValue = 17 };
     var b = new DecimalObserverDto { CurrentValue = 19.1m };
     var c = new BooleanObserverDto { CurrentValue = false };
     var d = new StringObserverDto { CurrentValue = "Hello World" };

     Assert.AreEqual("Hello World", d.GetValue());
     Assert.AreEqual("False", c.GetValue());
     Assert.AreEqual("19.1", b.GetValue());
     Assert.AreEqual("17", a.GetValue());
}

// Fails (doesn't compile) - no GetValue() for ObserverDto
[TestMethod]
public void CheckConversions()
{
     ObserverDto a = new IntObserverDto { CurrentValue = 17 };

     Assert.AreEqual("17", a.GetValue());
}

Best I can think of (sans adding a virtual method to ObserverDto) is use a lookup in the extension method (see above)

|improve this answer|||||
\$\endgroup\$
  • \$\begingroup\$ sorry about the last comment, though I had over written it but timed out and couldn't be edited :( . You are correct. In the test code I used, everything was var typed and I ended up with the concrete types and the extensions found the correct one. If passing in an ObserverDto, the correct GetValue() not found. Updated \$\endgroup\$ – AlanT Apr 4 '17 at 14:52

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.