Search code examples
c#exceptionconstructorsearch-engine

Best way to check if an instance was instantiated properly


so I have a question about checking if an object was instantiated using a particular constructor.I have a class called SearchWithTwoLevelCore that is part of a search engine. It has a constructor like this:

public SearchWithTwoLevelCache(ISearchCore s, ICurrentTimeProvider tp)
    {
        //Initialize the two levels.
        S=s;
        lvl2 = TimeBoundedQueryCache(s.AsQueryDataSource, tp, TimeSpan(24,0,0));
        lvl1 = SizeBoundedQueryCache(lvl2, 10);


    }

Where S, lvl1, and lvl2 are all private fields declared in the class that hold objects of other public classes. Then I have a public method I want to run that resides inside SearchWithTwoLevelClass, but first I want to check to see if the constructor used to make SearchWithTwoLevelCache was the one above otherwise the method wont run properly and throw some sort of exception if it wasn't. What is the best way to do this? Thanks so much in advance!


Solution

  • This is a good example of the importance of the Liskov Substitution Principle. I'm very fond of the image used in this answer.

    If your class has multiple constructors, and the success of your method depends on which constructor was called, then you have a number of choices.

    The main issue here is that your object state appears to be dependent upon which constructor is called; this is not the best design technique as it leads to a proliferation of state checks throughout the class, regardless of how you present your implementation.

    1) Refactor your objects into smaller objects

    The idea here is that your constructor should ONLY require the data the class needs to run. If you have multiple constructors each requiring a different amount of arguments and data, then it can be argued that this class actually represents several different objects.

    2) Enforce the data requirements on the method.

    public SearchWithTwoLevelCache(ISearchCore s, ICurrentTimeProvider tp)
    {
        //Initialize the two levels.
        S=s;
        lvl2 = TimeBoundedQueryCache(s.AsQueryDataSource, tp, TimeSpan(24,0,0));
        lvl1 = SizeBoundedQueryCache(lvl2, 10);
    }
    

    It could be argued that this state is not actually required by the class itself, as, if it is actually a pre-requisite for a function, then make it a requirement of the function itself. This removes ambiguity.

    Your class may therefore become:

    public SearchWithTwoLevelCache(ISearchCore s)
    {
      S = s;
    } 
    
    public Whatever PerformTwoLevelSearch(ICurrentTimeProvider tp) { }
    

    For example.

    The idea here is to only supply the data in the constructor that is absolutely required for the state of the entire class.

    You can, of course, implement checks at the method level and throw exceptions, but for people using your class, this can prove very frustrating. How did they know that to call function 1, they had to call constructor 2 and set some other variable? How would they know which interface to use? That's why making it a condition of the function is much simpler to both use, and maintain.

    It will also make extending this class difficult in future. If the success of a method relies on calling several other methods or a constructor first, refactoring and changing this logic then also forces all of your clients to change, too.