Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

First off, this is not my code. I'm curious what everyone thinks about changing the type of an object inside of a method then modifying the properties of the cast type. I wanted to use composition to build up a patient, but this is the recommend path. I'm probably wrong which is why I wanted 3rd party feedback. Or maybe we just have different coding styles...

    // Person/Patient Builder
public static void BuildUpPersonOrPatient(Person person, Dictionary<string, string> row)
        {
    person.FirstName = row["FirstName"];
    person.LastName = row["LastName"];

    Patient patient = person as Patient;

    if (patient != null)
    {
        patient.PatientID = row["PatientID"];
    }
}

for reference

public class Person
{
    public string FirstName { get; set; }
    public string LastName { get; set; }
}

public class Patient : Person
{
    public string PatientID { get; set; }
}

// //

share|improve this question
    
In C# you can't change the type of an object (an object only exist at runtime) you can test the type (and cast) as you do but you are not chaning the type –  Rune FS Jul 10 at 20:25

1 Answer 1

up vote 7 down vote accepted

It stead of having a static method the basically does what polymorphism could do for you use polymorphism.

public class Person
{
    public string FirstName { get; private set; }
    public string LastName { get; private set; }

    public virtual void InitializeFromDictionary(Dictionary<string, string> row)
    {
        FirstName = row["FirstName"];
        LastName = row["LastName"];
    }
}

public class Patient : Person
{
    public string PatientID { get; private set; }
    public override void InitializeFromDictionary(Dictionary<string, string> row)
    {
       base.InitializeFromDictionary(row)
       PatientID = row["PatientID"];
    }
}

and then where ever you call

BuildUpPersonOrPatient(p,row);

do this instead

p.InitialieFromDictionary(row);

I've added an access modifier to the setters because unless you have a reason to be able to change those fields from outside the object you should encapsulate the possibility of chaning the state of an object. (It's surprising how seldom you have a modeling reason for that and not just a "for conviniece" reason)

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.