Design a class with interface

Question

Updated

What is the best way to setup this class? i.e. call the SetupInstruction() method? I want to make this method virtual so a class that overrides can have its own implementation. In the overridden class I will set up other objects/properties.

public abstract class InstructionInfo : IInstruction
{
    protected readonly IUserSession UserSession;
    protected readonly IReadOnlySession ReadOnlySession;

    protected InstructionInfo(int instructionID)
    {
        InstructionID = instructionID;
    }

    protected virtual void SetupInstruction()
    {
        _myInstruction = null; //call db and set up...
    }

    #region interface members

    private Instruction _myInstruction;
    public Instruction MyInstruction
    {
        get
        {
            **SetupInstruction();**
            return _myInstruction;
        }
        private set { _myInstruction = value; }
    }

    #endregion
}

I think you might be better served on http://codereview.stackexchange.com — Brian Driscoll, Jan 18 '12 at 20:34
What's your reason for not setting it up in the constructor? — Jon Hanna, Jan 18 '12 at 20:43
@JonHanna Resharper was complaining about setting up a virtual member call inside constructor... I dont have a clue about what the side effects could be... — Haroon, Jan 18 '12 at 20:44
@JonHanna: I hope my question is valid :p Now how do I avoid, what are the ways around this? Maybe I should have included that inside my question above... — Haroon, Jan 18 '12 at 20:49
No, it was my bad in missing that you'd mentioned that you were calling into a virtual. — Jon Hanna, Jan 18 '12 at 21:11

score 1 · Answer 1 · answered Jan 18 '12 at 20:38

1

It all depends how you intend on using the code. If you want the setup process to be internal, then you may want to call that method from within the constructor of the base type. Setup can then be customized by derived types through overriding the method. Similarly, if you want to run some common setup tasks with a step where custom setup can occur, you could create a setup similar to this:

protected InstructionInfo(...)
{
    // ...

    SetupInstruction();
}

private void SetupInstruction()
{
    // Common setup
    // ...

    // Custom setup
    SetupInstructionCore();
}

// Either an optionally overriddable method
protected virtual void SetupInstructionCore()
{
}

// Or a required override
protected abstract void SetupInstructionCore();

answered Jan 18 '12 at 20:38

Samuel Slade

8,405
6
33
55

I want the Setup of this class to be either set inside this very class, or allow a class to inherit from this class and setup this class, it seems as though the optional override method you provided might be what I need. I was just trying to figure out where I would set up the class, it seems as though you are suggesting the constructor... – Haroon Jan 18 '12 at 20:43
@Haroon If the setup is required to occur as soon as the object is create, then yes. It also allows for the base class to control when setup occurs, rather than waiting for some external caller. – Samuel Slade Jan 18 '12 at 20:51
that is exactly what I want, I want the class to be set up asap, the same rule applies for the inherited class, it must be setup then it is good to be used everywhere else, I could add another method to my interface called Setup() and perform all the operations inside there? Would that make more sence? More logical? Leave it as abstract and force the implementation? – Haroon Jan 18 '12 at 20:56
@Haroon Having a method on the interface would allow for external callers to call it, which by the sounds of it isn't necessarily what you want. I think calling an overriddable method from the base type's constructor is probably the most logical solution. – Samuel Slade Jan 18 '12 at 21:16

score 0 · Answer 2 · answered Jan 18 '12 at 20:37

0

I think I dont fully understand your question, but you should make the setter of MyInstruction protected, so the overriding class can set it in SetupInstruction().

You also should give the property a better name eg Instruction instead of MyInstruction.

answered Jan 18 '12 at 20:37

thumbmunkeys

20,606
8
62
110

I would have, but Instruction is the name of an entity object already, I am open to other names :) – Haroon Jan 18 '12 at 20:50

score 0 · Answer 3 · answered Jan 18 '12 at 20:44

0

May be I missed something, but just a sample:

// a Base abstract class 
public abstract class MyAbs
{
    public MyAbs() {        
         Init();
    }

    public virtual void Init(){
         "Do base".Dump();
    }
}

//derived
public class Derived : MyAbs
{
    public override void Init(){
        "Do child".Dump();
    }
}

code to run, a sample

Derived ab = new Derived();

Output? Based on OO principles is :

"Do child"

Hope this what you want.

answered Jan 18 '12 at 20:44

Tigran

61,654
8
86
123

Not sure why resharper is complaining about a virtual member call inside the constructor... What are the implications? – Haroon Jan 18 '12 at 20:48
@Haroon: a guees: considering that `ctor` is about to constructing the object calling the virtual method, let's to derived class to "disturb" initialization off the base class with it's own implementation. As the child's ovveride will be called first, and not the one of the base class. You can provide a `Factory` class which constructs the objects of base type, withe `Init()` call, avoiding in this way use it inside a constructor. – Tigran Jan 18 '12 at 20:51
@Haroon: a good explanation why is good to avoid calling virtual methods inside `ctor` can find here http://stackoverflow.com/questions/119506/virtual-member-call-in-a-constructor – Tigran Jan 18 '12 at 20:53
that is very interesting... Should I add a abstract method on my interface called setup() and force this to be run? – Haroon Jan 18 '12 at 21:00
@Haroon: how do you think it to force? With a `factory`? – Tigran Jan 18 '12 at 21:13

score 0 · Answer 4 · answered Jan 18 '12 at 20:49

0

Your InstructionInfo is an entity. The setup method should be moved to a repository class eg. InstructionInfoRepository.GetInstructionInfoByID() for example.

You can later inject the upcasted InstructionInfoRepository (e.g. IInstructionInfoRepository) into other components to decouple the database access & Instruction instantiation logic. You can also inject a mock IInstructionInfoRepository into those same components during unit testing to avoid external resource dependencies during unit testing. During integration testing, you can then test your concrete implementation of the IInstructionInfoRepository.

answered Jan 18 '12 at 20:49

Athens Holloway

2,183
3
17
26

Could do that, infact that is exactly what I did lol, then I realised how wrong I was, so I moved in another direction, my class implements an interface that is used in various places, my inherited class implements a slightly different interface alongside this one, 2 lots of functionality that was not so easy to get using a repository... – Haroon Jan 18 '12 at 20:52
can you not account for the various interface implementations in your repository. You can delegate the interface specific details to various strategies. I'm not familiar with the specific details but I typically lean toward separating data access and instantion responsibilities to either a repository and/or factory. If the interface implementations only vary by operation/method implementation then treat data access as a separate concern. – Athens Holloway Jan 18 '12 at 21:00

score 0 · Answer 5 · answered Jan 18 '12 at 21:11

For the most part, I'd just design a class to set-up those fields it needs itself in its own constructor, and let derived classes set up those that are just for them in theirs.

If I wanted the way a particular field was set to be overridden, then I'd make all use of it happen through a lazy-loaded property.

public abstract class MyClass
{
  private SomeType _someField = null; // Never touch this field in any member but
                                      // the SomeProp property
                                      // Maybe use Lazy<SomeType> to make this more obvious.
  protected SomeType CreateSomeType();
  public SomeType SomeProp
  {
    get
    {
      return _someField = _someField ?? CreateSomeType();
    }
  }
}

Now I don't think about "setting up" the object, I just act as if it always has SomeProp ready to go, and as long as I don't use it in the constructor, it'll work.

If that wouldn't work in a particular case, I'd consider the factory pattern or the repository pattern.

Design a class with interface

5 Answers5