Password Validator – Code Review (OOP)

config.yml

This is an answer that is based on a post I made to code review.

The Original Problem

My Response

Some general comments on this approach.

OOP approach

Here’s why I don’t particularly like it. Let’s suppose that you had to change something: you want to add in another validation. That means you would have to modify the Password Validator class (one change) and you would also have to add in a corresponding method in the F# code (another change). In other words, things would be open for modification. Ideally if we want to extend something, we would want to do so without being forced to make changes to any existing classes. Forcing changes in two separate classes – I don’t like that idea.

Ideally if you wanted to add something you should be able to easily derive another class from a base class (or interface) – in this instance we are not editing any existing code. But in order to instantiate this new class then we would certainly have to modify any existing methods which instantiates classes of that particular type.

Perhaps this is best illustrated by example. Consider the following code:

Things are coming along quite nicely. Until I decide that I want to create another vehicle. I want to create a SUV class. I can easily do so:

I’ve added this class without modifying anything. Technically I would be editing the Factory method – but that’s unavoidable – for the most part, I can add things without being forced to edit any existing classes (apart from the factory method).

The approach the PasswordValidator class takes is not exactly open to extension yet closed to modification. It could easily be though.

Possible Ideas

Separation of Concerns.

Are the responsibilities kept separate? If you go for the above approach – if you wanted to make a change to the Minimum length class – you could do so – and the only class that would be affected would be the minimum length class (and the factory method, but let’s ignore that for a second). That’s a huge boon – you need to make changes basically in only one place.

Now let’s consider this method here:

If I decided to change the logic inside the static Password.longestRepeatedString method, then I would have to make a corresponding change to the logic in the above if statement – to make sure that all is kosher. In other words, a change in one place forces me to make sure there are no spill over effects in an entirely different class – the PasswordValidator class. In other words, I would say that the PasswordValidator and the static Password class are highly coupled. You can minimise this coupling by using the approach suggested above.

Other design patterns

You could possibly employ the decorator pattern, or some have suggested the chain of responsibility pattern. Or to keep it simple, a sub-classing approaching like the above should work just fine.

Hope this helps.

Written on September 27, 2017