Code Standard - is it better to have a getter/setter even though they are never used?

Question

The IDE has suggested to add a getter/setter to a private field. It is never used, and reaching the field is only from within the class.

what is the preferred coding style? keeping the never used methods?

Im asking specifically about java/kotlin but this is a general question

Answer 1

There are a few distinctions that you need to know about to answer this question yourself - as it depends on a ton of things; far too much to ask for and for you to write down:

For this entire answer it's important to think about the distinction between layers of code. These layers can be a bit hard to think about if the project you're imagining when thinking about layers is something small and written just by yourself. So don't do that - think about, say, Microsoft Word as a product. It's written by many people, over many years - entire departments and dev teams. It's somewhat modular (there's the "Mail Merge" system that doesn't interact, at all, with the 'show available fonts' dropdown).

What's the whole private fields, public getters/setters all about in the first place?

Fields are highly inflexible constructs. If you 'expose' them (make it public), then there is no granularity available to you. The only knobs you can twiddle with is:

• You can make a field unchangable for everybody - you can't change it, nor can anybody else. (To do this, mark it final).

That's it. You can't do anything else 'to' it. You can't have more fine grained control about access (such as allowing code 'nearby' to change it, but not code further out), you can't run some code as field writes/read happen either. Perhaps you need more granularity. Keep in mind that we're trying to wrote code that will survive 10 years in an environment with 100 programmers, most of whom won't last the entire 10 years, in many different teams. So, imagine you wanted to:

• Make it a field that everybody gets to read, but only 'your' code (that is, the programmers working on this particular corner of the codebase who are aware of this particular corner's rules and needs) should get to change.
• Make it a field that everybody gets to read, and write, but, if its not 'your' code doing the writing, a log line should be emitted.
• Make it a field that nobody gets to write (not even you - it is initialized at object creation, that's it, makes it easier to reason abou, that's why we 'handcuff' ourselves: When you need to maintain code for 10 years, limiting certain things off and having a compiler that enforces these is quite useful), and 'outsiders' can read, but you want to tweak the read a bit, for example, substitute 'the current date' when the value is blank.

And so on.

Even more importantly, is time: Sometimes you start out just wanting to expose a field to everybody right now, but later on you realize: Oh, wait, we need to emit a log line. Or: Oops, we need to return the current date if the value is blank.

If you just make a field public, you:

• Do not have any of that granularity.
• Even if you're okay with that now, you can't later on update your code and add stuff that needs this granularity; not without turning the field into a getter/setter pair, and that is not backwards compatible: You need to send a mail to those 100 developers or start refactoring their code which is a huge undertaking.

Hence, even if you don't see any point or purpose in giving you the granularity powers right now, it's still advised to just make that field private and add getters and setters: That way if later on some currently not forseeable request comes in (such as: Log the writes to this field, please!), you can add that feature without having to ask all other 100 developers to pull the change and edit all their branches, which is a huge undertaking.

YAGNI

A maxim in the programming world is YAGNI - You aren't gonna need it.

YAGNI is a dangerous beast - it applies -solely- to semi-local endeavours.

The basic principle of YAGNI is: Code is a flowing concept, and you should never hesitate to make improvements, especially if you can't think of a way this would break any existing usage. Hence, given that your development processes should be set up such that adding stuff is easy, don't add stuff until you need it - after all, if you add stuff even if you don't currently need it, maybe you never need it and you're now just clogging up the code for no good reason. IF somebody needs it, they can trivially add it then.

The problem with YAGNI is that predicate: YAGNI is based on the notion that making a change is quick and painless.

Imagine this scenario: The Microsoft Office development crew decides to write their own font rendering system, because what windows delivers just looks bad on HiDPI screens. So, they spend a ton of time and research on this and with much fanfare release a new version. Everybody loves it.

The OS team comes aknocking and the MS Office team decides to 'hand over' the new font rendering engine to the OS team. In order to avoid having 2 teams spend the resources on maintaining it, the next version of MS Office is pegged to only run on a new version of the OS that includes the new pipeline, and thus, the MS Office team removes the font rendering engine from it - it's now the OS's job.

Whoops, any YAGNI is now quite a big problem: If there's something foreseeable and obvious the MSOffice team needed that they didn't add (or if the Windows OS team applied YAGNI to the API they expose to apps to do font rendering stuff), then the MS Office team needs to give a call to the Windows OS team that's in another country, working on other source control, and having entirely different versioning pipelines, and ask them for a change. It'll take 2 years before it's all said and done.

Linters/stylcheckers are tools, and fairly stupid ones at that

Any warning about style or suggestion about changes are just that - suggestions. These tools aren't perfect, and will absolutely suggest very silly things from time to time. You should never apply style advice until you understand why it is given and under what circumstances it should be followed, and you should feel free to tell linters/stylecheckers to buzz off if they are wrong.

Some dev shops put out absolutist rules ('you can NEVER check in code that fails our linter tool - we have git commit hooks that enfore this!'), but those shops are misguided: They seem to think that if only you rigorously apply enough style rules, that code will therefore be well written, bug free, and performant. This is obviously entirely false. You should absolutely help programmers (and might lightly enforce this even) to help themselves and avail themselves of the tools available to write better code, but you can't beat the bird to make it sing, so to speak.

Thus, be aware that sometimes the best thing to do about a style suggestion - is to ignore it.

Back to your question

So, now you know what I'm driving at when I ask these questions, which naturally lead you to answering your own question:

• Is the field even meant to be exposed in the first place? Anything you 'expose' is likely to be used by code that's relatively far removed from you (different team, different time, different context), and once you expose it, you have to continue to support it - any changes you make can't fundamentally change/remove what you exposed. So, perhaps just having a private field with no getters and setters is the best place to start:
• If you're sure it makes no sense to expose it, then don't. Just leave them as private fields, the code in this source file can edit them, and other code cannot even assume this field exists - they should know nothing about it.
• If you're sure it makes perfect sense to expose it; it is the very point of the class, then make a private field with public getter (and if you want, setter - do you intend for it to be mutable or not?) - even if you don't see any need to do special stuff in that getter. Java programmers expect to access properties from other source files via getters and setters and you keep the flexibility to change things later without breaking compatibility.
• If you're not sure, then think about YAGNI: Is this an API that is going to be exposed so far and wide it'll affect people who cannot easily modify the codebase? Then, sorry, you're going to have to think some more and make a decision. But most likely you're not writing that kind of code, and anybody who might want to access this thing could change the code fairly easily: It'll be you, or a colleague working in the same source tree. In which case, don't think about it too long - err on the side of caution and don't make getters and setters. If someone needs em later, well, let them make the call - with the benefit of that use case they now have, they'll be more likely to make a well informed decision than you can, without that benefit.