12

Wikipedia says

"software entities (classes, modules, functions, etc.) should be open for extension, but closed for modification"

The word functions caught my eyes, and I now wonder if we can assume that creating an overload for a method can be regarded as an example of the Open/closed principle or not?

Let me explain an example. Consider that you have a method in your service layer, which is used in almost 1000 places. The method gets userId and determines whether user is admin or not:

bool IsAdmin(userId)

Now consider that somewhere it's necessary to determine if the user is admin or not, based on username, not userId. If we change the signature of the above-mentioned method, then we've broken code in 1000 places (functions should be closed to modification). Thus we can create an overload to get the username, find the userId based on username, and the original method:

public bool IsAdmin(string username)
{
    int userId = UserManager.GetUser(username).Id;
    return IsAdmin(userId);
}

This way, we've extended our function through creating an overload for it (functions should be open to extension).

Is it an Open/closed principle example?

Kilian Foth
  • 107,706
  • 45
  • 295
  • 310
Saeed Neamati
  • 18,142
  • 23
  • 87
  • 125

5 Answers5

5

I would personally interpret the wiki statement thusly:

  • for a class: feel free to inherit the class and override or extend its functionality, but don't hack the original class thereby changing what it does.
  • for a module (maybe a library for instance): feel free to write a new module/library that wraps the original, merges functions into easier to use versions or extends the original with extra features, but don't change the original module.
  • for a function (ie a static function, not a class method): your example is reasonable to me; reuse the original IsAdmin(int) function inside the new IsAdmin(string). the original does not change, the new func extends its functionality.

the shot in the foot however is that if your code is using the 'cUserInfo' class wherein IsAdmin(int) resides, you are essentially breaking the rule and changing the class. the rule would sadly only be kept if you made a new class cUserWithNameInfo : public cUserInfo class and put your IsAdmin(string) override there. If I owned the code base I'd never obey the rule. I'd say bollocks and just make the change you suggest.

Sass
  • 252
  • 2
  • 5
3

First off, your example is unfortunately contrived. You'd never do that in the real world, because it causes an unnecessary double-get. Or, worse, because userid and username might become the same type at some point. Rather than

public bool IsAdmin(int userid)
{
    User user = UserManager.GetUser(userid);
    return user.IsAdmin();
}

public bool IsAdmin(string username)
{
    int userId = UserManager.GetUser(username).Id;
    return IsAdmin(userId);
}

You should really extend that class.

public bool IsAdmin(int userid)
{
    User user = UserManager.GetUserById(userid);
    return user.IsAdmin();
}

public bool IsUsernameAdmin(string username)
{
    User userId = UserManager.GetUserByName(username);
    return user.IsAdmin();
}

You might further refactor and change the first method name to IsUserIdAdmin, for consistency, but it's not necessary. The point is that, in adding the new method and guaranteeing no calling code is broken, you have found your class to be extensible. Or in other words, open to extension.

And here's the thing with the open-closed principle. You never really know how well your code conforms until you try to extend part of it and find yourself having to modify instead (with the increased risk that comes with that). But, with experience, you learn to predict.

As Uncle Bob says (emphasis mine):

Since closure cannot be complete, it must be strategic. That is, the designer must choose the kinds of changes against which to close his design. This takes a certain amount of prescience derived from experience. The experienced designer knows the users and the industry well enough to judge the probability of different kinds of changes. He then makes sure that the open-closed principle is invoked for the most probable changes.

pdr
  • 53,387
  • 14
  • 137
  • 224
  • Thanks for your good explanation. But having 1000 get round-trips or one round-trip is not the main point here. Thus let's forget about performance at the moment. However, what I've done was also to extend the class, because I've added another method to it, though with the same name, but with different input parameters. But I really appreciated your quote from *uncle Bob*. +1. ;) – Saeed Neamati May 09 '12 at 11:08
  • @SaeedNeamati: The point I was making is that it doesn't matter if you are adding a method which uses an existing method or adding a method which is entirely independent, *either way* your class was open to extension. But, if you had to modify the existing method interface to achieve that, then you are not closed to modification. – pdr May 09 '12 at 11:18
2

Modules that conform to the open-closed principle have two primary attributes.

  1. They are “Open For Extension”. This means that the behavior of the module can be extended. That we can make the module behave in new and different ways as the requirements of the application change, or to meet the needs of new applications.
  2. They are “Closed for Modification”. The source code of such a module is inviolate. No one is allowed to make source code changes to it.
  1. By overloading your method you are extending functionality of existing module, thus meeting new needs of your application

  2. You are not making any changes to an existing method, so you are not violating second rule.

I would be interested to hear what others have to say regarding not allowing to change original method. Yes, you can put appropriate access modifiers and enforce encapsulation, but what else can be done?

Ref: http://www.objectmentor.com/resources/articles/ocp.pdf

CodeART
  • 3,992
  • 1
  • 20
  • 23
1

Open-Closed Principle is a goal, an ideal case, not always a reality. Particularly when looking to refactor old code, the first step is often heavy modification in order to make the OCP a possibility. The root of the principle is that working code already works, and changing possibly introduces bugs. Therefore the best scenario is not to change existing code, only to add new code.

But let's say you had a function called BigContrivedMethod(int1, int2, string1). BigContrivedMethod does three things: thing1, thing2, and thing3. At this point, reusing BCM is probably tough, because it does too much. Refactoring it (if possible) into ContrivedFunction1(int), ContrivedFunction2(int), and ContrivedFunction3(string) gives you three smaller, more focused methods that you can combine more easily.

And that is the key to OCP in regards to methods/functions: composition. You "extend" functions by calling them from other functions.

Remember that OCP is part of 5 other principles, the SOLID guidelines. That first one is key, Single Responsibility. If everything in your code base only did the one specific thing it had to do, you'd never need to modify code. You'd only need to add new code or combine old code together in new ways. Since real code rarely meets that guideline, you often have to modify it to get SRP before you can get OCP.

CodexArcanum
  • 3,421
  • 21
  • 23
0

You are following the open-closed principle if you are altering your program's behavior by writing new code rather than altering old code.

Since it's pretty well impossible to write code that is open to every possible change (and you don't want to because you enter analysis paralysis) you write code that responds to all the different behaviors you are currently working on by extension rather than modification.

When you run into something you need to change, instead of simply altering something to allow the new behavior, you figure out what the point of change is, restructure your program without altering its behavior so that you can then change that behavior by writing NEW code.

So, how this applies to your case:

If you are adding new functions to your class then your class is not open/closed but the clients of the function you're overloading are.

If you are simply adding new functions that work ON your class, then both it and the clients of your function are open/closed.

Edward Strange
  • 9,172
  • 2
  • 36
  • 48