8

Recently, I was writing a class that I decided to be package-private (that is, no access modifier, or the default one). It has an inner class and some private helper methods, and one method intended to be used by a class in the same package. All of these class members are static. But then I had a thought: should this one method have the public access modifier or no access modifier, like the class containing it?

On one hand, since the class itself is package-private, it can only be accessed and used within its package, so there is no practical reason to make the method public. But at the same time, semantically, this method is intended to be a public feature of the class, that is, a feature of the class intended to be used externally, so it would make sense to modify its access as such.

For those that like to see code,

final class DummyRemover {

    private DummyRemover() {

    }

    public static int remove(Map<String, ClassNode> classMap) {
        return 0;
    }

    // ...
}

or,

final class DummyRemover {

    private DummyRemover() {

    }

    // Notice the modifier.
    static int remove(Map<String, ClassNode> classMap) {
        return 0;
    }

    // ...
}

What is the best choice here? Is there a rule of thumb for deciding what access modifiers to use in a case like this?

Martin Tuskevicius
  • 2,590
  • 4
  • 29
  • 46
  • 5
    Rule of thumb: Begin with the most restrictive modifier and change it (one level "up") only when you must. – Maroun Feb 22 '16 at 08:15
  • I don't really think this is a dupe of the C# question, among other things because `internal` in C# actually has quite different semantics from default access in Java. The answers are *similar* but I don't think this question is necessarily a true duplicate. – Daniel Pryden Feb 22 '16 at 08:17
  • @DanielPryden: That's because I *linked the wrong question*. **sigh** [This is the right one.](http://stackoverflow.com/questions/9302236/why-use-a-public-method-in-an-internal-class) (Just came back to reopen and saw someone had already done it.) The semantics are similar enough for the purposes of this question (and that one). – T.J. Crowder Feb 22 '16 at 08:18
  • One could consider this as a duplicate of http://stackoverflow.com/questions/19161439/public-members-in-package-private-class – Marco13 Feb 22 '16 at 11:23

3 Answers3

7

There are two reasons why a method should have a higher visibility than its enclosing class:

1. The enclosing class is a base class

... which is intended for extension by subclasses, which might eventually be public, and the method at hand is supposed to be publicly available. E.g.

abstract class Base {
    public void x() {}
}

public class Sub extends Base {
}

// Now, everyone can call:
new Sub().x();

usually, however, if this is your intention, you will declare your method x() in an interface anyway. Even if you don't declare x() in an interface, design-wise, it's probably better to keep the base method at the same visibility level as its class, and "open up" the method in the subclass, as that will communicate intent more clearly:

abstract class Base {
    void x() {}
}

public class Sub extends Base {
    /** public Javadoc here */
    @Override
    public void x() {
        super.x();
    }
}

I don't see any reason why this approach should be applied to static methods as in your case.

2. The method must be public

... because it implements a method from an interface. Unfortunately, Java doesn't offer package-private or private interface methods. So the following is usual, even without any subclasses, and even if the interface itself might be package-private (or even nested private):

final class Y implements X {
    @Override
    public void x() {}
}

This (unfortunately) also applies to static classes on interfaces, which can only be public:

interface I {
    /* implicitly public */ static void x() {}
}
Lukas Eder
  • 211,314
  • 129
  • 689
  • 1,509
3

There are two schools of thought on this:

One group prefers adding unnecessary modifiers (e.g. public in this case, private in private classes) as a form of "documentation in code". For example, this is the viewpoint Eric Lippert espouses in his answer on this related C# question.

The other group prefers to never add source code that has no functional effect: if public isn't doing anything then it doesn't belong in your source code.

I think I am now firmly in the second camp, but I understand the arguments from the first camp and I think it's not nearly as cut and dried as it might first appear. Reasonable people can disagree, which is why (like braces and whitespace) this is something you should decide once in a project style guide and then never debate again. :-)

Community
  • 1
  • 1
Daniel Pryden
  • 59,486
  • 16
  • 97
  • 135
0

I personally think that making the method public might be misleading to the reader. On the other hand, it supports extensibility (If the class should ever be made public, the change is easier).

Whenever I need to choose between readability and extensibility, I go for the YAGNI principle and choose readability.

See wiki for more about YAGNI: https://en.wikipedia.org/wiki/You_aren%27t_gonna_need_it

Yoav Gur
  • 1,366
  • 9
  • 15