Should members of an object be automatically set in the class?

Question

When you have a complex property, should you instantiate it or leave it to the user to instantiate it?

For example (C#)

A)

 class Xyz{
     List<String> Names {get; set;}
 }

When I try to use, I have to set it.

...
Xyz xyz = new Xyz();
xyz.Name = new List<String>();
xyz.Name.Add("foo");
...

Where as if I modify the code

B)

 class Xyz{
     public Xyz(){
         Names = new List<String>();
     }
     List<String> Names {get; }
 }

which in this case, I can make the List read-only.

Another scenario might arise, I suppose where you would intentionally not want to set it. For example in

C)

 class Xyz{
     String Name {get; set;}
 }

I would thing it bad practice to initialize.

Are there some rules of thumb for such scenarios?

Answer 1

This is my normal solution:

class XYZ 
{
   public XYZ () { Names = new List<string>(); }
   public List<string> Names { get; private set; }
}

(Note that it doesn't work with XmlSerialization, as you need getters and setters on all XmlSerialized properties.) (You can override this, but it seems like too much work for little effort).

As OregonGhost pointed out - you need to add [XmlArray] for this to work with XmlSerialization.

This still breaks the rules of encapsulation, as if you wanted to be entirely correct, you would have:

class XYZ 
{
   public XYZ () { AllNames = new List<string>(); }
   private List<string> AllNames { get; set; }
   public void AddName ( string name ) { AllNames.Add(name); }
   public IEnumerable<string> Names { get { return AllNames.AsReadOnly(); } }
}

As this goes against the design of almost all the rest of the .Net framework, I usually end up using the first solution.

However, this does have the added benefit that XYZ can track the changes to it's collection of names, and that XYZ is the only place where the collection of names can be modified.

I have implemented this for a few cases, but it causes too much friction with other programmers when I do it for everything.

Answer 2

It really depends.

If the class is supposed to operate correctly with a null property, you can leave it as un-initialized. Otherwise, you'd better initialize it in constructor.

And, if you don't want the property to be changed after construction, you can have it private

class Xyz
{
     public Xyz(string name)
     {
         this.Name = name;
     }
     String Name 
     {
          get; 
          private set;
     }
}

Answer 3

As a rule of thumb (meaning there will always be exceptions from that rule) set your member in the constructor. That's what the constructor is for.

Remember one of the ideas beind OO is abstraction. Requiring the 'user' (i.e. the programmer who wants to instantiate your object in his source code) to do that is a violation of the abstraction principle. It would be one more thing the user must think of before using your object, hence one more potential source for errors.

Answer 4

Yes, there are rules of thumb. Code analysis tools are a good start for this.

Some rules of thumb about your code in question:

Its bad practice to allow setters on collection properties. This is because its simple to treat empty collections just like full ones in code. Forcing people to do null checks on collections will get you beaten. Consider the following code snippet:

public bool IsValid(object input){
  foreach(var validator in this.Validators)
    if(!validator.IsValid(input)
      return false;
  return true;
}

This code works whether or not the collection of validators is empty or not. If you wish validation, add validators to the collection. If not, leave the collection empty. Simple. Allowing the collection property to be null results in this smelly code version of the above:

public bool IsValid(object input){
  if(this.Validators == null) 
    return false;
  foreach(var validator in this.Validators)
    if(!validator.IsValid(input)
      return false;
  return true;
}

More lines of code, less elegant.

Secondly, for reference types OTHER than collections, you must consider how the object behaves when determining if you want to set property values. Is there a single, obvious, default value for the property? Or does a null value for the property have a valid meaning?

In your example, you may wish to always check,in the setter, the Name value and set it to a default "(No name given)" when assigned a null. This may make it easier when binding this object against UI. Or, the name may be null because you REQUIRE a valid name, and you will be checking it and throwing an InvalidOperationException when the caller tries to perform an action on the object without first setting the name.

Like most things in programming, there are a bunch of different ways to do something, half of which are bad, and each way in the other half are good only in certain circumstances.

Answer 5

The purpose of a constructor is the construct the object. No further "setup" should be necessary. If there is some information which on the caller knows, then that should be passed to the constructor:

Wrong:

MyClass myc = new MyClass();
myc.SomeProp = 5;
myc.DoSomething();

Right:

MyClass myc = new MyClass(5);
myc.DoSomething();

This is especially true if SomeProp needs to be set for myc to be in a valid state.

Answer 6

If possible you should try to encapsulate your data. Exposing the object as a list requires the user to know a too many intimate details about the class. For example, they have to know whether or not they have to create the list from scratch to avoid a null reference exception.

public class Xyz
{
   private List<String> _names = new List<String>(); // could also set in constructor

   public IEnumerable<String> Names
   {
       get
       {
           return _names;
       }
   }

   public void AddName( string name )
   {
       _names.Add( name );
   }
}

Now it doesn't matter if you use a List, HashTable, Dictionary, etc.