1

This might be a simple one, but my head is refusing to wrap around that, so an outside view is always useful in that case!

I need to design an object hierarchy to implement a Parameter Registration for a patient. This will take place on a certain date and collect a number of different parameters about a patient (bloodpressure, heartrate etc). The values of those Parameter Registrations can be of different types, such as strings, integers, floats or even guids (for lookup lists).

So we have:

public class ParameterRegistration
{
  public DateTime RegistrationDate { get; set; }
  public IList<ParameterRegistrationValue> ParameterRegistrationValues { get; set; }
}

public class ParameterRegistrationValue
{
  public Parameter Parameter { get; set; }
  public RegistrationValue RegistrationValue { get; set; }   // this needs to accomodate the different possible types of registrations!
}


 public class Parameter
  {
    // some general information about Parameters
  }

public class RegistrationValue<T>
{
  public RegistrationValue(T value)
  {
    Value = value;
  }

  public T Value { get; private set; }
}

UPDATE: Thanks to the suggestions, the model has now morphed to the following:

public class ParameterRegistration
{
  public DateTime RegistrationDate { get; set; }
  public IList<ParameterRegistrationValue> ParameterRegistrationValues { get; set; }
}

public abstract class ParameterRegistrationValue() 
{
  public static ParameterRegistrationValue CreateParameterRegistrationValue(ParameterType type)
{
    switch(type)
    {
        case ParameterType.Integer:
            return new ParameterRegistrationValue<Int32>();
        case ParameterType.String:
                return new ParameterRegistrationValue<String>();
        case ParameterType.Guid:
            return new ParameterRegistrationValue<Guid>();
        default: throw new ArgumentOutOfRangeException("Invalid ParameterType: " + type);
    }
}
  public Parameter Parameter { get; set; }
}

public class ParameterRegistrationValue<T> : ParameterRegistrationValue
{
  public T RegistrationValue {get; set; }
}

public enum ParameterType
{
  Integer,
  Guid,
  String
}

public class Parameter
{
  public string ParameterName { get; set; }
  public ParameterType ParameterType { get; set;}
}

which is indeed a bit simpler, but now I'm wondering, since the IList in ParameterRegistration points to the abstract ParameterRegistrationValue object, how will I be able to get the actual value out (since its stored on the sub-objects)?

Maybe the whole generic thing is indeed not quite the way to go after all :s

Sam
  • 1,514
  • 2
  • 14
  • 22
  • And what is the question and why are you using public fieds? – Lukasz Madon Jan 04 '11 at 09:50
  • You haven't mentioned what your problem is. – Will Vousden Jan 04 '11 at 09:52
  • What *are* you missing here? Does this not work? Where/how does it fail? – El Ronnoco Jan 04 '11 at 09:53
  • 1
    Do you want to have a strongly-typed collection of instances of different `RegistrationValues`-derived classes? – NOtherDev Jan 04 '11 at 09:53
  • Updated the example to use properties (was just shorthand) – Sam Jan 04 '11 at 09:53
  • The problem is, how can I model the different types of RegistrationValues, in other words, how can I go from a general ParameterRegistrationValue to a specific RegistrationValue (like A said, a strongly-typed collection of different RegistrationValues)? – Sam Jan 04 '11 at 09:54
  • A property of type `IList` with a public setter feels like bad design. And I'd consider removing most other public setters in these classes too. – CodesInChaos Jan 04 '11 at 10:10
  • CodeInChaos, what would you suggest as alternatives then? – Sam Jan 04 '11 at 10:14
  • For the list remove the setter/make it private and create an instance of a list in the constructor of `ParameterRegistration`. For the other properties it depends on whether you want immutability or not. But your classes look like good candidates for being immutable to me. – CodesInChaos Jan 04 '11 at 10:33

4 Answers4

4

If you don't know the final set of parameter and the corresponding type of each parameter then the generics probably won't help - use object as a parameter value type.

Furthermore iterating through the list of parameters will be a pain since you'll have to examine the type of each item in order to determine how to treat the value.

What are you trying to achieve with generics ? Yes, they are cool (and going for boxing/unboxing is probably not a best idea), but in some cases you might want to use object instead (for both simplicity and flexibility).

-- Pavel

volpav
  • 5,090
  • 19
  • 27
  • I do know the type of each parameter, this information will be stored in the database, so is available for retrieval. – Sam Jan 04 '11 at 10:10
  • Okay, in this case I would suggest to use a non-generic type as a collection type (a list of parameters) and the Factory pattern to retrieve specific parameter value type based on the actual parameter value. But still it doesn't worth it to use generics here, imho. – volpav Jan 04 '11 at 10:24
  • The more I tinker with it, the more I'm beginning to agree with you, generics are not exactly ideal either. Would you happen to have an example of the structure you would implement for this then? Thanks! – Sam Jan 04 '11 at 14:56
  • @Sam: I'd change the ParameterRegistration class to contain a property called ParameterRegistrationValues of type IList (instead of IList). The Parameter class will only contain the name of the parameter and the value (property of type object). – volpav Jan 04 '11 at 15:16
2

What you might want to introduce is an abstract base class for RegistrationValue<T> that is not generic, so that your ParameterRegistrationValue class can hold a non-generic reference, without needing knowledge of the type involved. Alternatively, it may be appropriate to make ParameterRegistrationValue generic also, and then add a non-generic base class for it instead (so that the list of values in ParameterRegistration can be of different types.

1st way:

public abstract class RegistrationValue
{
}

public class RegistrationValue<T> : RegistrationValue
{
  public RegistrationValue(T value)
  {
    Value = value;
  }

  public T Value { get; private set; }
}

And now your code should compile.


Once you have a non-generic base class, I'd also move any members of the generic class that don't depend on the generic type parameters up into this base class. There aren't any in this example, but if we were instead modifying ParameterRegistrationValue to be generic, I'd move Parameter up into the non-generic base class (because it doesn't depend on the type parameter for RegistrationValue)

Damien_The_Unbeliever
  • 234,701
  • 27
  • 340
  • 448
  • They would definitely need to be of different types, as a Parameter Registration can contain many different types of registrations. – Sam Jan 04 '11 at 10:04
  • Damien, when you say "make ParmaterRegistrationValue generic", do you mean instead of RegistrationValue or in addition to it? – Sam Jan 04 '11 at 10:57
  • @Sam - I was thinking in addition - that way its RegistrationValue property could be of a specific type. It depends on how high up the hierarchy it's useful to keep the generic type information, and at what level we definitely don't want it (such as when you're wanting to keep a list of such values, and they will be of different generic types) – Damien_The_Unbeliever Jan 04 '11 at 11:00
  • I understand what you're saying, but am not sure I understand the advantage it would confer in this case. – Sam Jan 04 '11 at 11:58
  • I've edited the question to update the model, I wonder if its really going to be possible to even get the information out, since you're always getting an abstract class out of the collection anyhow... – Sam Jan 04 '11 at 14:57
0

May be, you should use public RegistrationValue RegistrationValue, where T - is type, using in generic. For example, T - is String or other class or struct.

Or you should make class ParameterRegistrationValue as generic, to use generic argument in the field RegistrationValue.

Manushin Igor
  • 3,398
  • 1
  • 26
  • 40
0

I believe you want to have a collection of instances of different RegistrationValues-derived classes and be able to iterate it and for to have different type for each element. That's rather impossible.

You'll still need to cast each element to the type you know it is, because iterating the collection will return references to your base type (ParameterRegistrationValue - this one specified by IList type parameter). So it won't make any real difference from iterating over non-generic object list.

And if you can safely do that casting for each parameter (you know all the types), you probably don't need collection like this at all - it'll be better to have a class that encapsulates all the parameters in one type, so that you can call it with strong types, with IntelliSense etc. like this:

public class ParameterRegistration
{
  public DateTime RegistrationDate { get; set; }
  public PatientData PatientData { get; set; }
  public Guid Identifier { get; set; }
  // ...
}
NOtherDev
  • 9,542
  • 2
  • 36
  • 47
  • And what would you store in the PatientData object then? Thanks. – Sam Jan 04 '11 at 14:57
  • Well, I don't know your needs. Don't you have some kind of domain object that can represent all the data for your `Registration`, encapsulate all the possible `ParameterRegistrationValue`s? It may be a lot of data, but you can always skip data not applicable to given instance passing `null` if needed. – NOtherDev Jan 04 '11 at 19:02