50

A while back I wrote this answer to a question on how to avoid having a getter and setter for every mutable variable. At the time, I had only a hard-to-verbalize gut feeling that this is a Bad Idea, but OP was explicitly asking how to do it. I searched here on why this might be a problem, and found this question, whose answers seem to take it as a given that using reflection unless absolutely necessary is bad practice.

So, can someone verbalize why it is a given that reflection should be avoided, specifically in the case of the generic getter and setter?

JimmyJames
  • 24,682
  • 2
  • 50
  • 92
HAEM
  • 503
  • 1
  • 5
  • 12
  • 13
    If you just want to reduce boilerplate, both Lombok and Groovy will generate getters and setters silently but safely. – chrylis -cautiouslyoptimistic- Oct 09 '17 at 19:01
  • 2
    How would you even consume these getters and setters in a static language like java? It seems like a non-starter to me. – Esben Skov Pedersen Oct 10 '17 at 06:32
  • @EsbenSkovPedersen You'd consume them much like a `Map`'s `get`and `put`, which is a pretty clunky way of doing things. – HAEM Oct 10 '17 at 07:00
  • 2
    IIRC, Lombok synthesizes getters/setters at compile time, as dictated by the appropriate annotation, so they: don't incur the performance penalty of reflection, can be statically analyzed/inlined, can be refactored (IntelliJ has a plugin for Lombok) – Alexander Oct 10 '17 at 19:40

4 Answers4

120

Downsides of reflection in general

Reflection is harder to understand than straight-line code.

In my experience, reflection is an "expert-level" feature in Java. I would argue that most programmers never use reflection actively (i.e. consuming libraries that use reflection doesn't count). That makes code using it harder to understand for these programmers.

Reflection code is inaccessible to static analysis

Suppose I have a getter getFoo in my class and I want to rename it to getBar. If I use no reflection, I can just search the code base for getFoo and will find every place that uses the getter so I can update it, and even if I miss one, the compiler will complain.

But if the place that uses the getter is something like callGetter("Foo") and callGetter does getClass().getMethod("get"+name).invoke(this), then the above method won't find it, and the compiler won't complain. Only when the code is actually executed will you get a NoSuchMethodException. And imagine the pain you're in if that exception (which is tracked) is swallowed by callGetter because "it's only used with hard-coded strings, it can't actually happen". (Nobody would do that, someone might argue? Except that the OP did exactly that in his SO answer. If the field is renamed, users of the generic setter would never notice, except for the extremely obscure bug of the setter silently doing nothing. Users of the getter might, if they're lucky, notice the console output of the ignored exception.)

Reflection code is not type-checked by the compiler

This is basically a big sub-point of the above. Reflection code is all about Object. Types are checked at runtime. Errors are discovered by unit tests, but only if you have coverage. ("It's just a getter, I don't need to test it.") Basically, you lose the advantage using Java over Python gained you in the first place.

Reflection code is unavailable for optimization

Maybe not in theory, but in practice, you won't find a JVM that inlines or creates an inline cache for Method.invoke. Normal method calls are available for such optimizations. That makes them a lot faster.

Reflection code is just slow in general

The dynamic method lookup and type checking necessary for reflection code is slower than normal method calls. If you turn that cheap one-line getter into a reflection beast, you might (I have not measured this) be looking at several orders of magnitude of slowdown.

Downside of generic getter/setter specifically

That's just a bad idea, because your class now has no encapsulation anymore. Every field it has is accessible. You might as well make them all public.

Sebastian Redl
  • 14,950
  • 7
  • 54
  • 51
  • 9
    You hit all the major points. Pretty much a perfect answer. I could quibble that getters and setter are marginally better than public fields but the bigger point is correct. – JimmyJames Oct 09 '17 at 15:17
  • 2
    It's also worth mentioning that getters/setters can easily be generated by an IDE as well. – stiemannkj1 Oct 09 '17 at 19:02
  • 26
    +1 Debugging reflection logic in a production-size project should be recognized as torture by the UN and illegalized. – errantlinguist Oct 09 '17 at 20:55
  • In fact, I beg the OP to delete their linked answer to keep these bad ideas out of the heads of programmers: I'm usually against censorship but this is playing with horrible, unmentionable evils which will truly drive you insane. – errantlinguist Oct 09 '17 at 21:08
  • 4
    "harder to understand for these programmers." - for these programmers it's significantly hard to understand, but it's *harder* to understand for everyone, no matter how proficient. – BartoszKP Oct 09 '17 at 21:20
  • 5
    "Reflection code is inaccessible to static analysis" - This. So important for enabling refactoring. And as static-analysis tools get more powerful (e.g. code search in the latest Team Foundation Server), writing code that enables them becomes even more valuable. – Dan J Oct 10 '17 at 04:57
  • @JimmyJames more than marginal, irrespective of type safety. Adding additional logic (like validation or other side-effects) to a setter is easy. Refactoring to a field being being computed on lookup with a getter is easy. Changing a public field to support those is a breaking change. This issue comes up all the time even in dynamic languages. – Jared Smith Oct 10 '17 at 12:02
  • 1
    @JaredSmith Your points are correct but I would call these techniques for coping with a bad design. In the long run you still end up in the situation where the cost of changes rivals or exceeds the cost of a total rewrite. This is why I consider this a marginal improvement. – JimmyJames Oct 10 '17 at 14:33
  • @JimmyJames bad design? Perhaps so, but concluding that a priori seems... excessive. Strong OO modeling is overkill for a great many instances. It can also, depending on the people involved, lead to bikeshedding over the model instead of solving the problem. Although to be fair to you, question is tagged Java and Java doesn't give many other options. – Jared Smith Oct 11 '17 at 13:53
  • @JaredSmith Exactly. Java (until 1.8) was sort of a one-trick pony. It's a good trick, though. Now with function references etc. the situation is a bit different. In other languages, the distinction between maps/dictionaries and objects are blurred and I use other techniques. – JimmyJames Oct 11 '17 at 14:10
  • Might also be worth mentioning that setters (and to a lesser degree getters) are a sign of bad OO design anyway. Having them at all is just a temptation to code poorly. – Bill K Dec 10 '18 at 17:07
12

Because pass-through getter/setters are an abomination that provide no value. They don't provide any encapsulation since users can get at the actual values via the properties. They are YAGNI because you're rarely if ever going to change the implementation of your field storage.

And because this is way less performant. In C# at least, a getter/setter will inline straight to a single CIL instruction. In your answer, you're replacing that with 3 function calls, which in turn need to load metadata to reflect over. I've generally used 10x as the rule of thumb for reflection costs, but when I searched for data, one of the first answers included this answer which ballparked it closer to 1000x.

(And it makes your code harder to debug, and it harms usability because there's more ways to misuse it, and it harms maintainability because you're now using magic strings rather than proper identifiers...)

Telastyn
  • 108,850
  • 29
  • 239
  • 365
  • 2
    Note that the question is tagged Java, which has such delightful crap like the Beans concept. A bean has no public fields, but it may expose fields via `getX()` and `setX()` methods that can be found via reflection. Beans aren't necessarily supposed to encapsulate their values, they might just be DTOs. So in Java, getters are often a necessary pain. C# properties are much, much nicer in every respect. – amon Oct 09 '17 at 15:03
  • 12
    C# properties are dressed-up getters/setters, though. But yes, the Beans concept is a disaster. – Sebastian Redl Oct 09 '17 at 15:07
  • Pretty sure Beans and their kin won't play nice with generic getters/setters with a name parameter... – Telastyn Oct 09 '17 at 15:11
  • 7
    @amon Right, C# took a terrible idea and made is easier to implement. – JimmyJames Oct 09 '17 at 15:15
  • 5
    @JimmyJames It's not a terrible idea, really; you can maintain ABI compatibility despite code changes. A problem many don't have, I suppose. – Casey Oct 09 '17 at 19:58
  • 1
    @Casey I'm not sure how the problems with procedural approaches relate to binary compatibility. You can maintain binary compatibility without breaking encapsulation. – JimmyJames Oct 09 '17 at 20:07
  • 1
    @JimmyJames I'm not sure I understand the question. If you decide you wanted a more complex getter than just returning the private variable, you can do that easily if you just use properties from the beginning. – Casey Oct 09 '17 at 20:16
  • 3
    @Casey The scope of this goes beyond a comment but building an interface from the internal details of the implementation is backwards. It leads to a procedural design. There are times when having a getX method is the right answer but it's pretty infrequent in well-designed code. The problem with getters and setters in Java is not the boilerplate code around them, it's that they are part of a terrible design approach. Making that easier is like making it easier to punch yourself in the face. – JimmyJames Oct 09 '17 at 20:25
  • @JimmyJames https://softwareengineering.stackexchange.com/q/358924/212639 – gaazkam Oct 10 '17 at 19:33
  • Could you show an example of C# inlining getter methods? Because from my (albeit short) testing, that does not seem to be the case at all. – Rob Oct 11 '17 at 03:33
8

In addition to the arguments already presented.

It doesn't provide the expected interface.

Users expect to call foo.getBar() and foo.setBar(value), not foo.get("bar") and foo.set("bar",value)

To provide the interface that users expect you need to write a seperate getter/setter for each property. Once you have done that there is no point in using reflection.

Peter Green
  • 2,125
  • 9
  • 15
-3

Of course is not a bad idea, it is just than in a normal java world is a bad idea(when you only wants a getter or setter). For example some frameworks(spring mvc) or librares already use it(jstl) in that way, some json o xml parsers are using it, if you want to use a DSL you are going to use it. Then it depends on your use case scenario.

Nothing is right or wrong as a fact.

as a fact
  • 21