Open - closed principle issue

88 views Asked by At

Imagine an application where you have a class named Transaction representing a financial transaction.

These transactions can be classified based on the value of properties of Transaction class and some business rules.

An enum called TransactionType represents the possible value of such a classification: Type1Transaction, Type2Transaction, NotRelevant.

Type1Transactions and Type2Transactions will be reported because they are somewhat suspicious, NotRelevant transactions won't be reported.

This kind of classification is interesting only in order to report transaction and does not represent an intrinsic property of Transaction class (that's why Transaction class does not have a property representing this classification).

Well, at this point we want to write a class whose responsibility is determine the classification given a transaction, let's call it TransactionClassificator.

Its interface expose a method GetTransactionType() whose return value is TransactionType. The most obvious implementation is as follows:

public TransactionType GetTransactionType()
{
    if(IsType1Transaction())
        return TransactionType.Type1Transaction;

    if(IsType2Transaction())
        return TransactionType.Type2Transaction;

    return TransactionType.NotRelevant;
}

This method clearly violates OCP principle: it is NOT closed to modifications because it must be modified every time a new value for enum TransactionType will be introduced. I can't figure out how to use abstraction to remedy to OCP principle violation in this specific scenario.

Thanks for helping.

Enrico

1

There are 1 answers

2
Weltschmerz On BEST ANSWER

Use a TypeReporter (or some more suitable name). It's responsibility will be to determine the transaction type based on the data it gets from Transaction:

interface TypeReporter {

    /**
     * Methods required for transaction to feed the data required for the 
     * business rules to determine the transacion type
     */

    public TransactionType determineType();
}

Now the transaction can report its own type with TypeReporter's help:

interface Transaction {

    /** among other things **/

    public TransactionType reportType(TypeReporter reporter);

}

e.g.

class ConcreteTransaction implements Transaction {

    public TransactionType reportType(TypeReporter reporter) {
        reporter.hereIsThis(someProperty);
        reporter.andThat(someOtherProperty);

        return reporter.determineType();
    }

}

So Transaction delegates the type determination to another object. That way it is closed for modification if a new type or a new strategy for determining the type arises. If that happens you write a new TypeReporter.

Edit: To make the method for determining type more change resistant you can keep the conveniently named types like so:

public enum TransactionType {
    Type1Transaction,
    Type2Transaction,
    NotRelevant
}

where method that returns the type can be:

public TransactionType determineType() {
    //business logic to determine the type goes here
    //represent that type as a string (String determinedType)

    //now we check if that string is a valid enum value, if not returning NotRelevant
    try {
        return Choices.valueOf(determinedType + "Transaction");
    } catch (IllegalArgumentException ex) {  
        return TransactionType.NotRelevant;
    }
}

This way if the TransactionType becomes relevant you can just add it as a new TransactionType value and it will no longer return the NotRelevant.