25

I had a little debate going on with a coworker. Simply put, is there a good reason to hide/encapsulate functions that are pure?

By "pure" I mean the wikipedia definition:

  • Always returns the same results from the same input. (For the sake of this discussion Foo Create(){ return new Foo(); } is considered impure if Foo does not have value semantics.)
  • Does not use mutable state (except local variables) or I/O.
  • Does not produce side effects.
  • 26
    There could be an argument made not to make such a function member of a class at all. – Pieter B Sep 26 '14 at 14:55
  • 6
    @PieterB - indeed, though not all languages support that, and some languages allow module internal even for free functions. – Telastyn Sep 26 '14 at 15:02
  • 3
    What was your opinion? I don't think the purity of the function is related to whether it belongs in the public API. – Andres F. Sep 26 '14 at 15:19
  • @andresF. - the debate was actually around whether an unbound validation rule should be made public. I made the argument that since it was a pure function, there was little harm. For this particular instance, it aided testability and was likely to be reused. This question is more about how broadly that argument could/should apply. – Telastyn Sep 26 '14 at 15:53
  • 2
    @Telastyn If it's reusable but isn't part of the current class' responsibility, it should probably be in a separate class/module/whatever-your-language-has. Then being part of the new class/module's responsibility, it would necessarily be made public. Since you mention testing, I'll note that you don't necessarily have to mock out the method when you do that. As an "implementation detail," mocking it during tests gives little benefit. – jpmc26 Sep 26 '14 at 17:27
76

A pure function could still be an implementation detail. Although the function may cause no harm (from the point of view of not breaking important invariants/contracts), by exposing it both the author and the users of that class/module/package lose. The author loses because now he can't remove it even if the implementation changes and the function is no longer useful to him. The users lose because they have to sift through and ignore extra functions that aren't relevant to using the API in order to understand it.

  • 33
    +1. Public members of your class is its API. It's not a question of "Can it hurt me if it's public?", but rather "Should it be part of the API?" – Ordous Sep 26 '14 at 16:18
  • 1
    Only thing I would add is that if you start seeing similar functions popping up in other locations, refactor it into another class so multiple classes can make use of it. Or if that's a concern to begin with, define it in a separate class to begin with. – jpmc26 Sep 26 '14 at 17:25
  • 1
    I would seal the public classes that are not meant to be extended as well. – Den Sep 30 '14 at 15:14
  • +1 for pointing out that hiding or showing a function (or class) are primarily questions of protecting and maintaining an API, and not related to the type of code it is. – Marco Sep 30 '14 at 20:09
42

The question is backwards.

You don't seek for a reason to make a function non-public. It is an incorrect mindset to start with (in my opinion). The reasoning should go the other way.

In other words - don't ask "why would I make it private?". Ask: "why would I make it public?"

When in doubt, don't expose it. It's kind of like Ockham's razor - don't multiply entitites beyond necessity.

EDIT: Addressing counterarguments brought up by @Telastyn in comments (to avoid extended discussion there):

I've heard that over time, and even espoused it for quite some time, but in my experience, things tend to be too private.

Yes, it's a pain sometimes if a class is open for inheritance, but you can't override some private methods (whose behaviour you'd like to alter).

But protected would suffice - and it's still non-public.

It leads to a lot of code duplication and overhead to get at "things that shouldn't be public" yet are accessed indirectly anyways.

If it becomes problematic, then simply make it public! There's the necessity I was talking about :)

My point is that you shouldn't do it just in case (YAGNI and all).

Note that it's always easier to make a private function public than pulling it back to privacy. The latter is likely to break existing code.

  • I've heard that over time, and even espoused it for quite some time, but in my experience, things tend to be too private. It leads to a lot of code duplication and overhead to get at "things that shouldn't be public" yet are accessed indirectly anyways. – Telastyn Sep 26 '14 at 15:38
  • 3
    @Telastyn IMHO, that sounds like the application suffers from some design missteps; if you find yourself inclined to expose a method because you need to invoke it across discrete code paths, it's probably a sign that this method should be broken out into its own module that can be injected or included where appropriate, especially if it's a pure function. – leo Sep 26 '14 at 15:58
  • @leo - sorry, I don't follow. Consider a function like integer less than. This is a nice pure function that is exposed specifically because it can be then injected and included (or simply used) where appropriate. – Telastyn Sep 26 '14 at 16:23
  • 5
    @Telastyn In my opinion that's a symptom of the "everything is an object/class" philosophy combined with the fact that some OOP languages don't have composite data types other than classes. As soon as someone needs to pass two values at the same time they end up writing a class, and then every coworker and style checking software will tell you to make those fields private. – Doval Sep 26 '14 at 16:57
  • @Telastyn Right. What I mean is, if your 'integerLessThan' method started off as an encapsulated implementation detail of a public method, but you find that you need to invoke the private method in other places, that's probably a sign that the private method belongs in a different module/package/class so that it can be imported independently of the logic that invokes it. Simply publicizing the method as-is would mean the method is arbitrarily located in the first module where you found it useful. – leo Sep 26 '14 at 17:29
6

I don't think the decision to hide/encapsulate a function should depend on its purity. Just because a function is pure doesn't mean that externals need to know about it. Interestingly enough though if the function is pure and meant to be public maybe it doesn't even need to be an instance member of the interface at all maybe it is better suited as a static. But again all of this depends on the intent of the contract and in this case the logical grouping of functionality, not the purity of the function.

5

Classes should adhere to the Single Responsibility Principle. While a class may need to call on other functionality to achieve its goals, it should only expose functions that are part of its single responsibility.

Here is just one example of a case where visibility could cause a problem.

Consider a class that frobnicates widgets. Maybe as part of its frobnication code it needs some utility function that parses a string: perhaps it needs to transform the widget name in a way that standard string functions do not support.

Since this is a pure function (string comes in, transform it somehow, return a new string), it could be public or private without consequence. Or could it?

If you make it public, now your class has two responsibilities: frobnicating widgets, and transforming strings. This violates SRP, and can cause problems if other classes come to rely on the function. Since this is something you think is only used internal to the class, perhaps you change its interface or visibility. Now classes in other parts of the system are broken.

By keeping the function private, nobody ever has the opportunity to rely on code that is not part of the class's single responsibility.

  • 2
    I would argue that it should only contain functions that are part of its single responsibility, not just expose. – Telastyn Sep 26 '14 at 15:37
  • 1
    I think there is a gray area. Do you really need a separate StringTransformer class to encapsulate two or three lines of code that are only used in one place? I agree that once code is used in multiple places it is best to separate it into a new class with one responsibility, but there is a tradeoff. – user22815 Sep 26 '14 at 15:40
  • Certainly. Guidelines are guidelines, not rules. – Telastyn Sep 26 '14 at 15:45
  • @snowman in non java you just make a library of functions. – Pieter B Sep 26 '14 at 16:50
  • @PieterB it is not about Java v. anything else. To make it truly OO you would need a factory, some abstract classes, etc. – user22815 Sep 26 '14 at 17:10

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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