C# Problem with collection of custom classes

Question

In my application I have robots that the user can give any number of different tasks to do. Each task is a class derived from the abstract class Task:

public abstract class Task
{
    string description = "default description";
    ...
}

public class Walk : Task
{
    string description = "Do walk";
    ...
}

public class Talk : Task
{
    string description = "Do talk";
    ...
}

So each robot has a collection of these tasks. The collection is defined like this:

public class TaskCollection
{
    List<Task> _collection = new List<Task>();

    public void AddTask(string taskName)
    {
        _collection.Add( (Task)Activator.CreateInstance(null, taskName).Unwrap() );
    }

    public int Count()
    {
        return _collection.Count;
    }

    public Task GetElement( int i )
    {
        return _collection[i];
    }
}

So to test it out I create a TaskCollection and add some tasks like this:

public TaskCollection tasks = new TaskCollection();
tasks.AddTask( "Walk" );
tasks.AddTask( "Talk" );

And then this code to show all the tasks that have been added:

for( int i = 0; i < tasks.Count(); i++ )
{
    print( tasks.GetElement(i).description );
}

For each task it prints out "default description" rather than the descriptions assigned in the inherited classes. Is this because i'm casting the class instance as a Task? If i remove that cast I get this error:

The best overloaded method match for `System.Collections.Generic.List.Add(Task)' has some invalid arguments.

Hopefully its clear what I'm trying to do here. Can anybody see where I'm going wrong?

Answer 1

Following is not right:

public class Walk : Task
{
    string description = "Do walk";
    ...
}

You are hiding Task's description member by declaring a new member named description in it's child class. You must be getting warnings stating this.

Instead, use a constructor like following:

public class Walk : Task
{
    public Walk()
    {
        description = "Do walk";
    }
}

and also make description protected in Task class.

protected string description = "...";

On a side note, if you put following code in your loop, you will get correct type names:

print( tasks.GetElement(i).GetType().FullName );

Following is a good article explaining what is going on there (Member hiding):

Polymorphism, Method Hiding and Overriding in C#

Answer 2

Use the following approach:

public abstract class Task
{
    private const string description = "default description";
    public virtual string Description { get { return description; } }
}

public class Walk : Task
{
    private const string description = "Do walk";
    public override string Description { get { return description; } }
}

Answer 3

You're using fields, rather than properties. Among other issues, fields cannot be polymorphic. Polymorphism is the effect you're seeking, where a member is defined at a high level and its implementation is provided (or overridden) at a lower level.

What you want are properties. Since this is an exercise in OO design, I'll keep things as you have it:

public abstract class Task
{
    public virtual string Description
    {
        get { return "default description"; }
    }
    ...
}

public class Walk : Task
{
    public overrides string Description
    { 
        get { return "Do walk"; }
    }
    ...
}

public class Talk : Task
{
    public overrides string Description
    { 
        get { return "Do talk"; }
    }
    ...
}

What you accomplished with the fields was simply providing a member of the same name, which hides the higher-level member from the outward-facing API of the lower-level class. If you were to reference the instance as Walk or Talk (through casting, for example), then you would see the appropriate description. But when you reference it as a general Task, the more-defined members are unknown.

Bear in mind that I'm answering the question you asked, which is why it wasn't working and how to make it work. This is not the design I would have chosen for this particular task, since you aren't providing different functionality in the derived classes (for this property), just different data. Given that, I would have created a constructor argument that takes the description, then returns it in the base class.

public abstract class Task
{
    public string Description { get; private set; }

    protected Task(string description)
    {
        this.Description = description;
    }
}

Then the derived classes would look like:

public class Walk : Task
{
    public Walk() : base("Do walk") { }
}

(etc.)

Answer 4

You're declaring extra variables in each subclass, but then you're always printing the description variable from task. The subclass variables only hide the variable in Task.

There are two (or more) options here.

First, you could use polymorphism:

public abstract class Task
{
    public virtual string Description { get { return "default description"; } }
}

public class Walk : Task
{
    public virtual string Description { get { return "Do walk"; } }
}

This makes a lot of sense if in reality instead of "description" you're actually wanting some behaviour which will change in each concrete implementation.

The other option is to keep a single description in Task, but pass the value for it into the constructor:

public abstract class Task
{
    private readonly string description;

    protected Task(string description)
    {
        this.description = description;
    }
}

public class Walk : Task
{
    public Walk() : base("Do walk")
    {
    }
}

Now you could set the field directly from the subclass if you made it accessible, but I would strongly advise you to keep it private. If you really want to "push" new values, make it a property like this:

public class Task
{
    public string Description { get; protected set; }
}

That allows subclasses to set the description, but from the outside it can only be fetched.

Answer 5

You shouldn't need to do this

public abstract class Task
{
    string description = "default description";
    ...
}

public class Walk : Task
{
    string description = "Do walk";
    ...
}

It should look like this

public class Walk : Task
{
   public Walk
   {
      description = "Do walk";
   }
}

That should set the field in the base class, right now you aren't actually doing that.