8

Inheritance is a hammer. Eliminating code duplication is not a nail. – The Usele...

 3 years ago
source link: https://uselessdevblog.wordpress.com/2018/02/18/inheritance-is-a-hammer-eliminating-code-duplication-is-not-a-nail/
Go to the source link to view the article. You can view the picture content, updated content and better typesetting reading experience. If the link is broken, please click the button below to view the snapshot at that time.
neoserver,ios ssh client
Home Menu

Inheritance is a hammer. Eliminating code duplication is not a nail.

uselessdev in Uncategorized

February 18, 2018February 19, 2018

1,781 Words

It’s a daily occurrence when adding features to a system –
we currently have functionality X implemented, but now we also need functionality X’, which is just slightly different.
Re-implementing most of X with some modifications is code duplication, and this is badTM.
We need to keep our code DRY, which means that both X and X’ need to share some code.

For many, especially in certain communities (ruby), the first only solution they’d consider is:
“Let’s make X’ inherit from X”
“That way, we can change some of the functionality in X’ by overriding some methods / properties”.

This isn’t always the best solution.
Let’s look at the (over-simplified) example of this service, which handles the process of creating a new user record:

class UserCreator
def create_new_user(username, password)
existing_user = User.find_by(username: username)
return 'user already exists' if existing_user
return 'password not complex enough' unless password =~ SOME_UNGODLY_REGEX
user = User.new(username, password)
user.save
send_welcome_email(user)
end
end

And now we have a new requirement: we want to be able to create a user of type AdminUser as well. Also, the welcome email for an admin user is different.

Well, we can’t be repeating all of this code again, with the only difference being AdminUser instead of User, and some email wording, right?
Many times we’d find ourselves with this solution:

class UserCreator
def create_new_user(username, password)
existing_user = user_class.find_by(username: username)
return 'user already exists' if existing_user
return 'password not complex enough' unless password =~ SOME_UNGODLY_REGEX
user = user_class.new(username, password)
user.save
send_welcome_email(user)
end
private
def user_class
User
end
end

and then:

class AdminUserCreator < UserCreator
private
def user_class
AdminUser
end
def send_welcome_email(admin_user)
# some fancier email here
end
end

Simple enough, right?

This is where sirens should start flashing: AdminUserCreator has only private methods and no public interface?? What’s the justification for that?
Seeing this unusual situation alerts us that our design may be lacking.

Looking more critically at our solution, we can spot a few issues:

Single responsibility is broken

As we all remember, single responsibility means that an object has only one reason to change.
In our case, UserCreator should only change if we change the way we create users.
But, we just made quite a lot of changes to UserCreator to accommodate the way we create admin users.
Meaning: UserCreator has two reasons to change.

Encapsulation is broken

As we can clearly see above – UserCreator is accessing private members of AdminUserCreator .  One class interacting with another class’s private methods isn’t an ideal situation.
Worse – the other way around is also true – AdminUserCreator  can also access the private parts of UserCreator!

Which leads us to the next point:

Interfaces are broken

Take this exaggerated example: the welcome emails for the two types of users are completely different, but the email title is similar. What would that look like?

class UserCreator
# our public methods
private
def send_welcome_email(user)
title = get_title(user)
# more email generation...
end
def get_title(user)
friends_stats = get_friends_stats(user)
return "Welcome #{user.name}! you have joined   #{friends_stats.number_of_friends} of your friends who were active #{friends_stats.activity_last_day} times today"
end
end
class AdminUserCreator < UserCreator
private
def send_welcome_email(user)
title = get_title(user)
# admin email generation - something completely different...
end
def get_friends_stats(user)
# stats for admin users are calculated differently
end
end

Our code is DRY, and that's what's important, right?

Let's try, together, to follow the control flow of sending an email to an admin user:

1. Parent's create_new_user is called
2. Parent’s create_new_user calls Child’s send_welcome_email.
3. Child’s send_welcome_email calls Parent’s get_title.
4. Parent’s get_title calls Child’s get_friends_stats.

Confused? So are we.
It’s impossible to understand what a single method does without looking at both classes, and having to guess, at each method call, what actually is being executed.

Look at AdminUserCreator again – it only has some floating methods without any context that are impossible to understand (you can’t tell when or how get_friends_stats is called, for example, because it isn’t called in the class you’re looking at!)

(And just imagine what would happen if we added SuperUserCreator to the mix!)

The issue here stems from the fact that we have two classes that interact with each other, but it’s impossible to know when these interactions take place:

The interfaces between these two classes are inferred (based on whether a certain method has been overriden or not) rather than explicit (based on defining, at the call site, the object on which the method is executed).

Single responsibility is broken

Wait, didn’t we just talk about this..?
Well, it turns out, we can break this principle twice:

Since the child class has no choice but to inherit all the implementation (and therefore – responsibilities) of the parent class, if we add anything to it, it now has multiple responsibilities – its parent’s, and its own.

Imagine that, when an admin user is created, we want to take some more actions – send an email notification to other admin users, create a special log entry to document it, etc.

Because of the way the parent and child classes are all jumbled together, the child class now has more responsibilities than we would want, and we can’t easily extract them, because another class, and a completely different functionality, is involved.

Duplicating responsibilities also leads to –

Test code is broken

Two classes having the same functionality means that we have to have the same set of tests for both of them, to verify this functionality.
This causes either code duplication, or having to share test code (sharing test code in an easy to understand way is something that I find very difficult to do).
Additionally, the differences between the parent and child classes are in their private methods. That can be sometimes awkward to test.

What’s the alternative?

Prefer composition over inheritance. Meaning – compose the different responsibilities using a third, collaborating object that’ll take on the shared responsibilities, make it configurable as needed, and have both implementations use it.

In our example we can consider two such collaborators:

class UserEmailTitleGenerator
def get_title(user, friends_stats)
"Welcome #{user.name}! you have joined   #{friends_stats.number_of_friends} of your friends who were active #{friends_stats.activity_last_day} times today"
end
end
class UserRecordCreator
def create_user_record(username, password, user_class)
existing_user = user_class.find_by(username: username)
return 'user already exists' if existing_user
return 'password not complex enough' unless password =~ SOME_UNGODLY_REGEX
user = user_class.new(username, password)
user.save
return user
end
end

and now our original classes become trivial:

class UserCreator
def create_user(username, password)
user = UserRecordCreator.create_user_record(username, password, User)
send_welcome_email(user)
end
private
def send_welcome_email(user)
user_stats = get_regular_user_stats(user)
title = UserEmailTitleGenerator.get_title(user, user_stats)
# ...
end
end
class AdminUserCreator # look mom - no relation to the other class!
def create_user(username, password)
user = UserRecordCreator.create_user_record(username, password, AdminUser)
send_welcome_email(user)
end
private
def send_welcome_email(user)
user_stats = get_admin_stats(user)
title = UserEmailTitleGenerator.get_title(user, user_stats)
# ...
end
end

..And it’s that stupidly simple. Instead of configuring the two classes differently by overriding methods and attributes, we configure some lower-level services by providing different arguments.
(*in fact, the above example is so simplistic that it can easily be solved by a single class using inversion of control; I’ll leave that as an exercise to the reader)

This approach avoids the aforementioned disadvantages –

  1.  It keeps true to SRP – since the two Creator classes are completely unrelated, and can only change independently.
  2. It maintains very clear interfaces – for each method call, we know exactly what method is executed, and against which object, and with which parameters.
  3. Single responsibility is, again, maintained – because no class needs to assume responsibilities that are not strictly its own.
    In fact, this approach helps us spread the responsibilities in even finer-grained detail; we can have a class for just creating the user record, a class just for the email generation, etc..
  4. Since common responsibilities are now extracted to a completely separate class, they can be tested only on that class (i.e creation of the user record can be tested only for the UserRecordCreator class, and not for UserCreator or AdminUserCreator.)

There are no free lunches, of course, and these advantages carry with them a new disadvantage –
we’ve now broken up our functionality into many small pieces. These pieces may be too small, and having many small pieces can be very very confusing.

Summary (or: are you telling me that I should never use inheritance?)

I’m telling you that inheritance is one tool in your toolbelt, and it has its uses. But it isn’t the right tool for every job.

Like almost any design decision we make, it all comes down to tradeoffs. Each approach has its own pros and cons.
My thinking is that, even if inheritance doesn’t immediately show all of the cons listed above, starting down that path makes it hard for me to change course and start breaking things up, because by nature inheritance couples my classes quite tightly together.

That’s why I’d usually use inheritance only in these very specific cases:

  1. Is-a relationship: Like in the classic examples we’ve all seen in school – a car is a vehicle, because it has the same properties (move, make_noise), but some different implementation, or maybe some additional properties / methods.
    (meaning – it needs to meet Liskov’s substitution principle – any code that expects an instance of parent can equally use an instance of child).
    Note that this narrow definition is almost always restricted to domain classes only;
    While it’s easy to imagine that a triangle is a shape, it’s hard to say that “monthly report controller” is a “report controller”.

    te2m2uftkg701

  2. Well-known design patterns: Several design patterns utilize inheritance. I’m very comfortable using inheritance in these cases because it’s very clear what’s going on and how the different classes interact (because the patterns are well defined)

But, in most other cases – I’d look elsewhere than inheritance.


About Joyk


Aggregate valuable and interesting links.
Joyk means Joy of geeK