Is it considered bad software engineering practice to always use “public” methods? [duplicate]

Question

This question already has answers here:

Why not make everything private or public? [closed] (3 answers)

Closed 3 years ago.

I've always used public methods and recently been advised by one of my friends to always avoid defining methods as public, as far as possible, though I have worked in any commercial company I have never "really" understood the concept of why public methods are not "safe to use at times".

I have worked a lot with c# and please don't get me wrong I have used other access types such internal, private and protected and I know how they "work" but never really understood why you should define methods with those access levels, especially in production.

Answer 1

The methods, properties and constructors (i.e. members of a class) that you define using a public accessor determine the surface area that the users of your class will be allowed to interact with. Any method that you don't want to be part of this surface area should not be made public.

Reasons why some members might not be public:

1. They are implementation details; they implement behavior needed by the public members, but are not meant to be called from the outside directly (i.e. private).
2. They are meant to be accessed in a derived class, but not called directly. (i.e. protected).

Think of it in terms of your house. You don't let the tax man come into your house, find your checkbook and write himself a check. Rather, the tax man sends you a bill, you review the bill, write a check and send it to him. The sending of the bill and the check are both public acts. The acts of reviewing the bill and writing the check are implementation details performed in the privacy of your own home that the tax man doesn't need to know anything about.

Further Reading
Encapsulation

Answer 2

Short answer:

Declaring a method or field "public" translates to a promise:

Dear colleages,

I invite you to use this method or field wherever you find it appropriate.

I have documented everything you need to know about its usage, and as long as you don't violate this documentation, I take responsibility for every bug in this context.

I promise that I'll never change the behaviour of this method / interpretation of the data in this field, or I'll analyze all of your code using my method and change it appropriately.

Are you sure you want that (especially the "documentation" part)?

Answer 3

If I have a class:

class SomeThing
{
    public DoSomething()
    {
        DoThing1();
        DoThing2();
    }

    public DoThing1() ...
    public DoThing2() ...
}

Now I might intend everyone to call DoSomething only. But I've made DoThing1 and DoThing2 public too, so people might still use them. As a result, when I later rewrite DoSomething to use DoThing3 only, I'm stuck: I can't get rid of DoThing1 and DoThing2, even though I'm no longer using them.

If I'd marked them as private though, then no such problem would arise. I can safely delete them without breaking external code.

So a good rule of thumb is mark everything private unless it absolutely needs to be accessed outside the class. If it must be accessed, mark it internal if possible as it still remains "private" to that project and can be changed without breaking other projects. Only if it is needed as part of the public API of the project should you mark it public. This applies as much to classes, enums etc as it does to members of those classes etc.

Answer 4

Classes expose an interface (the English word, not the c# keyword), through which you use them.

Make interfaces easy to use correctly and hard to use incorrectly.

If your class has a function that should not be called from other classes, yet you allow other classes to do so, you make it easy to use the class incorrectly. This introduces bugs, maintenance costs and training costs. This isn't as bad if you're the sole developer - you already know the code and know which functions you can or cannot call. Unless you have to revisit your code 6 months later, at which point you can't remember.

Answer 5

If you're writing "large-scale" software (i.e. working with multiple teams, releasing new versions over time, have enough code that it's hard to keep it all in your head at once), then yes it's a bad engineering practice (see the other great answers).

If you're just writing one-off code (e.g. a single-file script to push data around with just 3 methods) then it's overkill.

Answer 6

The access specifiers also act as a kind of documentation/comment/description. When you read some source code, if you see something private you know immediately that the thing is only used internally. This helps improve code readability.