3

I work on refactoring an Java application based on a CAST audit. One of the criterion says that

To respect OO encapsulation concepts, private fields should always be accessed through accessors

So far, so good. However, the audit highlights all private fields in the class that have no mutators, regardless how they are being used.

In my case, these fields are only accessed from inside the class.

I could provide private accessors for all of them, but for readability purposes, I do not think it is a good idea. Furthermore, I do not see how providing private getters for private attributes enhances OO encapsulation.

I could also provide public mutators for all fields. I have seen it done in many applications. Come to think of it, most IDEs provide a functionnality that automatically generates public mutators for all of the class attributes...

But I wonder, is it not against the very principle of encapsulation to provide public accessors for private attributes that are only meant to be used inside the class?

EDIT : my question is not about the use of accessors inside their own class. It is about the relevance of providing public accessors for all private fields in the class, regardless they are used outside the class or not.

The Once-ler
  • 163
  • 7
  • 1
    Possible duplicate of [Should the methods of a class call its own getters and setters?](https://softwareengineering.stackexchange.com/questions/181567/should-the-methods-of-a-class-call-its-own-getters-and-setters) – gnat Apr 17 '19 at 09:17
  • 4
    Your suspicions are correct: it's actually an anti-pattern to provide public mutators for all fields (or even most fields). Unfortunately this practice was historically required by some frameworks/libs (e.g. some persistence frameworks), but it's a historical artifact that goes against OOP practices. – Andres F. Apr 17 '19 at 17:40
  • @AndresF. well, your comment is the exact answer to my question. :) Would you consider posting it as an answer ? – The Once-ler Apr 18 '19 at 07:28

4 Answers4

11

So I did some googling. Here is the rule you quote:

To respect OO encapsulation concepts, private fields should always be accessed through accessors

https://www.appmarq.com/public/changeability,4576,Provide-accessors-to-Private-Fields

Its remediation:

Write a getter and setter to each private field

And a reference. Presumably for justification:

http://www.oracle.com/technetwork/java/javase/documentation/spec-136004.html JavaBeans(TM) Specification 1.01 Final Release - paragraph 8.3 Design Patterns for Properties p 55

However, if you check the reference it says nothing of the sort. It's all about how properties are implemented.

That, coupled with the awful Appmarq site which prevents text selection, leads me to believe that the entire CAST audit is basically worthless.

The idea that every private field should be backing a property is ridiculous. Presumably what they actually mean is: "Use properties rather than public fields" or maybe "Beware of stateful objects!"

Robert Harvey
  • 198,589
  • 55
  • 464
  • 673
Ewan
  • 70,664
  • 5
  • 76
  • 161
  • I had not checked the reference (shame on me), but you are right, the content of Section 8.3 is not really about the violated rule. This criterion is quite unclear, because the description of the violation says _Accessors are identified using the following java bean naming conventions: "set" and "get" followed by the name of the field with the first char in uppercase (case sensitive). For Boolean fields, the getter can also be named "is" followed by the name of the field._ I think in the end, it mixes proper naming and relevance of the use of public accessors, which are different issues. – The Once-ler Apr 17 '19 at 14:53
  • 7
    @Pikuni: And Java Beans are pretty much a violation of every accepted object-oriented convention for good class design anyway. – Robert Harvey Apr 18 '19 at 18:37
  • @RobertHarvey But as they are called *Enterprise* Java Beans, they are at least buzzword-compliant. – Deduplicator Apr 18 '19 at 22:45
7

No, you don't seem to have missed anything. That audit sounds misguided, among other things it implies immutable objects, like String, are "against OO encapsulation concepts".

Caleth
  • 10,519
  • 2
  • 23
  • 35
2

While there are few hard and fast rules in software design and development, there's a school of thought that mutators/getters/setters for every attribute of a class are an anti-pattern or at least not a good practice for OOP.

At least in some measure, the widespread use of getters/setters seems to have been heavily influenced by JavaBeans, which are not particularly object oriented, or at least not a general case of object orientation. Some people went as far as to claim that getters/setters were "evil" (in 2003!) or "considered harmful".

I wouldn't go as far, but I do think many getters/setters tend to weaken or sidestep guidelines such as Tell, Don't Ask and encapsulation. If an attribute is an implementation detail, an internal of the class, why should it be exposed through its interface?

Getter/setters are in some cases also a historical "artifact" of some persistence frameworks that used to require them (or at least encourage their use) to persist/load objects to and from the datastore. In that case, a technical detail ended up encouraging bad OO design.

With all this in mind, I think one thing can be said: no, a class should NOT provide public mutators for all its private fields. It might be necessary in some cases, but as a general principle it's not correct.

Andres F.
  • 5,119
  • 2
  • 29
  • 41
0

For an alternative point of view, some argue that there should be no getters or setters. At all. IMO, this is a little too strong, but they make excellent points that getters and settings have significant drawbacks and should be avoided.

In any case, the CAST audit, if it really means to add accessors for all private fields, is crap. I suspect that it really means "if you wish to allow access to a field, make an accessor method", but not sure.

Getters/Setters Evil Period

Why getter and setter methods are evil

user949300
  • 8,679
  • 2
  • 26
  • 35