How to reorganize this code in C #?

Question

I am working on legacy code and I have seen a lot of code like this:

public class Person { public Person(PersonData data) { this.Name = data.Name; this.Gender = data.Gender ; } public String Name { get; private set;} public String Gender { get; private set;} } public class PersonData { public String Name; public String Gender; } public static Person ReadPerson(Reader reader) { PersonData data = new PersonData; data.Name = reader.ReadString(); data.Gender = reader.ReadString(); Person p = new Person(data); return p; } 

The PersonData class exists to set private fields in the Person class in its constructor. In addition, the PersonData class introduces redundant code, because now you can see the name and gender in the Person and PersonData classes.

In my defense, this type of design does not scale: now I have a new Age field for reading, I have to add the Age property in two different places.

Is this the right design choice (given that I have a lot of code like this in legacy code)?
How can I reorganize this?

EDIT:

These two classes are a simplified version of real code. Therefore, please forgive using a string instead of an enumeration for gender.

In real code, PersonData has more than 10 fields, so the class is Person.

Answer 1

Using a parameter object is a valid approach when using Constructor Injection , and you begin to get a large number of parameters in your constructor, but this is not necessary when there are fewer such parameters than yours.

Here is a suggestion:

 public class Person { public Person(string name, string sex) { _name = name; _sex = sex; } public string Name { get {return _name; }} public string Sex { get {return _sex; }} private readonly string _name, _sex; } 

This makes the class immutable .

Answer 2

If these were some external objects (unlike data transfer objects, which, it seems, in your case), you can freely use interfaces to create them, this will not reduce the number of classes, but will allow you to build the object in a more attractive way , with better control over what is required and what is optional.

See messages tagged with fluent-interface if interested. That is, the Conditional Builder Interface that supports free access :

 var person = PersonBuilder .CreatePerson() .Named(reader.ReadString()) .Sex(reader.ReadString()) .Build() 

Answer 3

One way instead

 public String Name { get; private set;} public String Sex { get; private set;} 

to infer a property of type PersonData

 public class Person { public PersonData PersonData { get; } } 

You can also see the output of Person from PersonData .

Answer 4

Get PersonData of PersonData and feed Reader into the constructor:

 public sealed class Person { public Person(Reader reader) { this.Name = reader.ReadString(); this.Sex = reader.ReadString(); } public string Name { get; private set; } public string Sex { get; private set; } } 

Answer 5

In general, I would return to the real (or business) system that you are modeling. If the class corresponds to something in this world, then everything is in order. If the class is a pure artifact of the programming system and also seems unnecessary, I would drop it. Using the "data" class can also hide various problems associated with using an explicit parameter. For example, when you add "age", how do you find that all cases are found? If you add it as a constructor parameter, you will get an error for each missing case.