friendly classes in c#

Question

actually i refactor some portion of code. what i want to do is to initialize an object "Task" with an object "TaskArgument". let s say "TaskArgument" is abstract and "Task" implements a method "OnEnterTask(TaskArgument args)" and is sealed (for some special behavior of the existing system, which is out of scope).

old code:

public sealed class Task : SomeSystemBaseTask {
  private int accessMe; 
  private int meToo;

  public void OnEnterTask(TaskArgument args) {
    if (args is SimpleTaskArgument) {
      accessMe = ((SimpleTaskArgument)args).uGotIt;
      meeToo = 0;
    } else if (args is ComplexTaskArgument) {
      accessMe = ((ComplexTaskArgument)args).uGotItValue * ((ComplexTaskArgument)args).multiplier;
      meToo = ((ComplexTaskArgument)args).multiplier - 1;
    }
  }
}

what would be the best practise avoid the typecheck? my first stupud thought was:

public abstract class TaskArgument {
    internal public abstract Initialize(Task args);
}

public class SimpleTaskArgument : TaskArgument {
    public int uGotIt = 10;

    internal public Initialize(Task task){
        task.accessMe = uGotIt;
    }
}

public class ComplexTaskArgument : TaskArgument {
    public int uGotItValue = 10;
    public int multiplier = 10;

    internal public Initialize(Task task){
        task.accessMe = uGotItValue*multiplier;
        task.meToo = multiplier - 1;
    }
}

public sealed class Task : SomeSystemBaseTask {
    public int accessMe;
    public int meToo;

    public void OnEnterTask(TaskArgument args){
        args.Initialize(this);
    }
}

but then my "accessMe" is public and the "Initialize" method works only with "Task". so i moved the typechecking to another place (in future). is there any best practise or good design idea.

..."internal public"... mmhhmm?

another crazy idea was an inner class, but i dont like those and it make such a simple case more complex or don't:

public abstract class TaskArgument {
    internal public abstract Initialize(ITaskWrapper wrapper);
}

public class SimpleTaskArgument : TaskArgument {
    ...
}

public class ComplexTaskArgument : TaskArgument {
    ...
}

public interface ITaskWrapper {
    public int AccessIt { set; get; } 
    ...  
}

public sealed class Task : SomeSystemBaseTask {
    private int accessMe;
    ...

    class TaskWrapper : ITaskWrapper {
        ...
    }

    public void OnEnterTask(TaskArgument args){
        args.Initialize(new TaskWrapper(this));
    }
}

where is the best place for initialization when it is based on the given Type of the "TaskArgument"?

kindly excuse my bad english knowledge

greetings mo

Answer 1

Use an interface.

public void OnEnterTask(TaskArgument args) { 
   if (args is SimpleTaskArgument) { 
      accessMe = ((SimpleTaskArgument)args).uGotIt; 
   } else if (args is ComplexTaskArgument) { 
      accessMe = ((ComplexTaskArgument)args).uGotItValue * ((ComplexTaskArgument)args).multiplier; 
   } 
} 

becomes

public void OnEnterTask(ITaskArgument args) { 
   accessMe = args.GetAccessMe();
} 

Then you have your classes implement ITaskArgument and implement the method for each class. In general, when you're doing something like this:

accessMe = ((ComplexTaskArgument)args).uGotItValue * ((ComplexTaskArgument)args).multiplier;

where you're accessing multiple properties on an object to perform a calculation, it usually makes sense to push that logic into the class itself.

Answer 2

Sounds like you want to put the logic associated with each sub-class of TaskArgument onto that class. You could add an abstract method to TaskArgument called Calculate that has the sub-class specific calculation. That would remove the need for your if statements completely:

public class Task {
  private int accessMe; 


public void OnEnterTask(TaskArgument args)
  {
    accessMe = args.Calculate();
  }
}

You would then put the multiplication or whatever is appropriate into each sub-class.

Answer 3

I would create a public interface, which only exposes the Intialize method. Do your calculations in your derived classes e.g.

public interface ITaskArgument
{
    void Initialize(Task task);
}

public abstract class TaskArgument : ITaskArgument
{
    protected int _value;
    public class TaskArgument(int value)
    {
        _value = value;
    }

    public abstract void Initialize(Task task);
}

public class SimpleTaskArgument : TaskArgument, ITaskArgument
{
    public SimpleTaskArgument(int value)
       : base (value)
    {
    }

    public override void Initialize(Task task)
    {
        task.AccessMe = _value;
    }
}

public class ComplexTaskArgument : TaskArgument, ITaskArgument
{
    private int _multiplier;

    public ComplexTaskArgument(int value, int multiplier)
       : base (value)
    {
         _multiplier = multiplier;
    }

    public override void Initialize(Task task)
    {
        task.AccessMe = _value * _multiplier;
    }
}

public class Task
{
    public Task()
    {
    }

    public int AccessMe { get; set; }

    public void OnEnterTask(ITaskArgument args)
    {                         
        args.Initialize(this);                         
    }  
}

example

SimpleTaskArgument simpleArgs = new SimpleTaskArgument(10);
ComplexTaskArgument complexArgs = new ComplexTaskArgument(10, 3);
Task task = new Task();
task.OnEnterTask(simpleArgs);
Console.WriteLine(task.AccessMe); // would display 10
task.OnEnterTask(complexArgs);
Console.WriteLine(task.AccessMe); // would display 30

Answer 4

OK, changed my answer a bit in light of the changing requirements appearing in the comments! (Sheesh, scope creep or what?!)

public class Task
{
    public int Variable1 { get; internal set; }
    public int Variable2 { get; internal set; }

    public void OnEnterTask(ITaskInitializer initializer)
    {
        initializer.Initialize(this);
    }
}

public interface ITaskInitializer
{
    void Initialize(Task task);
}

public class SimpleTaskInitializer : ITaskInitializer
{
    private int uGotIt = 10;

    public void Initialize(Task task)
    {
        task.Variable1 = uGotIt;
    }
}

public class ComplexTaskInitializer : ITaskInitializer
{
    private int uGotIt = 10;
    private int multiplier = 10;

    public void Initialize(Task task)
    {
        task.Variable1 = uGotIt;
        task.Variable2 = uGotIt * multiplier;
        // etc - initialize task however required.
    }
}

Answer 5

You could create overloads of Task as one option:

public class SimpleTask : Task
{
   public override void EnterTask(TaskArgument arg)
   {
      var s = (SimpleTaskArgument)arg;
   }
}

So each task type deals with an equivalent argument type. Or, you can move the logic to a TaskFactory with a static method that returns an int, and has the type checking argument there.

public static class TaskFactory
{
   public static int GetVal(TaskArgument arg)
   {
      if (args is SimpleTaskArgument) { 
        return ((SimpleTaskArgument)args).uGotIt; 
      } else if (args is ComplexTaskArgument) { 
        return ((ComplexTaskArgument)args).uGotItValue * ((ComplexTaskArgument)args).multiplier; 
      }
   }
}

Your interface implementation also would work; I wouldn't discount that... or define an abstract method within Taskargument, that each overrides to return the value.

HTH.