Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

I am reimplementing some component and noticed that the original version has a Liskov Substitution violation. It's not all that critical, though I'd like to get rid of it in the new implementation . It is however not clear to me how to do this.

I have a component defining very simple value classes used by the system. This component has a DataValue interface and a dozen implementations such as NumberValue and GeoCoordinateValue.

The component I'm reworking is a storage layer meant for indexing data so it can easily be queried against. This component contains a set of classes that provide storage information for a particular DataValue implementing class. This information are things such as which fields a table needs to have to contain the DataValue in question, which field should be used for sorting, and what indexes should be placed. These classes all implement some DataValueHandler interface.

The LSP violation occurs for two particular methods in this DataValueHandler interface:

getWhereConditions( DataValue $dataValue )

getInsertValues( DataValue $dataValue )

The interface defines these methods take a DataValue. The implementations however expect the DataValue for which they provide information. For instance, the NumberValueHandler expects a NumberValue, and will throw an exception if it gets a GeoCoordinateValue.

So how can I get rid of this? (Before anyone suggests it: putting info of some specific storage backend in DataValue is not going to happen, as this would be worse design wise then the current LSP violation.)

share|improve this question
    
Are you looking for a solution in the language you are using? If so, which language is it? Also, are you actually using the DataValueHandler interface? –  Aaron Kurtzhals Feb 27 '13 at 14:16
1  
I've always assumed LSP applied to single dispatch, not behaviour of another class when passed different types - i.e. it would be a breach if NumberValueHandler did not fit the contract of DataValueHandler when passed a GeoCoordinateValue, but throwing an exception if the dynamic type doesn't match that expected is not ( unless DataValueHandler's contract prohibits throwing an exception ). I would probably replace with double dispatch though, unless there is another reason to have the handlers spread across different objects. –  Pete Kirkham Feb 27 '13 at 15:52
    
The language I'm using is PHP, though I'm perfectly happy with the solution specified in another language, as long as it can also be expressed (sanely) in PHP :) –  Jeroen De Dauw Mar 1 '13 at 15:14
    
@PeteKirkham If you pass a subtype of DataValue to getWhereConditions and you get an exception because it's not the correct subtype, is that not a clear violation of the LSP? Not sure how double dispatch would be helpful here - can you illustrate what you are thinking of? –  Jeroen De Dauw Mar 1 '13 at 15:18
    
Say you have two implementations TypeA and TypeB of interface I. For every call in the interface I, TypeA and TypeB behave in accordance to LSP. Someone then creates a free function which takes an I, tests whether its type is TypeA, and if it is throws an exception. That doesn't effect LSP with respect to TypeA or TypeB - LSP applies to the types which vary, not the behaviour of other code. If instead of a free function, it is a method of another object which throws the exception, this does not alter the LSP status of TypeA or TypeB... –  Pete Kirkham Mar 1 '13 at 15:40
show 2 more comments

2 Answers

up vote 3 down vote accepted

The specific DataValue class should be a parameter for the DataValueHandler constructor and stored as a member variable, and the get methods should not include a DataValue object as a parameter.

(Some java-ish pseudocode below to illustrate)

class NumberDataValueHandler implements DataValueHandler {
    private NumberDataValue myDataValue;

    NumberDataValueHandler (NumberDataValue newNumberDataValue) {
       myDataValue = newNumberDataValue;
    }

    String getWhereConditions () {
        myDataValue.someNumberSpecificMethod();
        // do some NumberDataValue specific operations...
    }

}
share|improve this answer
    
Thanks for your reply. What you suggested works for the problematic methods yes, though is not very sensible for the others. Those are often needed in situations where I don't have any particular instance of the DataValue in question. Guess I should split up the class into a part with info for a type of DataValue and a part for a particular instance of that type. –  Jeroen De Dauw Feb 27 '13 at 15:01
add comment

Do the DataValue classes all implement IDataValue or inherit from the same abstract DataValue class?

I'd expect all classes to have a common set of public methods, and for the GetWhereConditions only to act on these common methods (thus facilitating the substitution).

Otherwise they're just different classes, in which case you'd expect to see...

GetWhereConditions(NumberValue){...}
GetWhereConditions(GeoCoordinateValue){...}

...to reflect this.

share|improve this answer
    
DataValue is an interface implemented by all datavalues. –  Jeroen De Dauw Feb 27 '13 at 14:59
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.