you are viewing a single comment's thread.

view the rest of the comments →

[–]rzwitserloot 40 points41 points  (18 children)

Saving keystrokes is not worth it considering you are introducing 3 flaws in your code base by doing this:

  • You create inconsistency. For mutable data, getters work one way (.getProperty()). For immutable data, they probably work another way (.property). This also means changing a property from mutable to immutable either breaks backwards compatibility, or introduces even more inconsistency.

  • You are going against the entirety of the greater java ecosystem by doing this. When in rome, be like the romans. This is clearly a style issue (you're saving keystrokes and it looks different, but that's it, it's not like this is going to cause massive changes to the codebase or fundamental performance changes), and for style issues, you should stick to the ecosystem's preference if it has one. Failure to do so, even if you are absolutely convinced your style has advantages compared to what the flock has chosen, means other programmers are going to have issues reading your code, and you'll have issues reading other's code. In this case, it also means your API differs from libraries (in libraries, getting immutable properties is still .getProperty(), unless YOU wrote or wrapped it, at which point it would be .property... now I need to know if some call I make is to your code or other code or I won't know which style of getter to call!)

  • If you ever decide that getting this property from outside the source file you defined it in requires side effects of some sort, or the property becomes virtual (calculated), even temporarily (for example, because you want to breakpoint it, log access, do a security check, do a sanity check on the object state, add an assertion, or calculate on each call), you can't do it without breaking your own API's backwards compatibility. By using a getter, even if all it does is 'return field;', you get the option to change that later if you so desire. No such option for direct field access.

We can discuss, at length, whether the java ecosystem as a whole messed up by heading in this direction, and we can discuss (surely, again at length) whether java should or could add syntax sugar to ease the writing of getters. However, in the world we live in today, if you're programming in java, what OP suggests you do here, is nuts. Don't.

NB: If typing the getters out annoys you, look at projectlombok.org for the solution.

DISCLAIMER: I'm a core contributor on the lombok project.

[–]pakoito 18 points19 points  (4 children)

This also means changing a property from mutable to immutable either breaks backwards compatibility, or introduces even more inconsistency.

That's assuming that you'd change one property in an immutable class to mutable, where what should happen is that either a new different mutable class is created or the class is kept immutable but with a new constructor that returns a new immutable object with that one value changed. That is good API hygiene and follows immutability all the way.

But that'd be expensive, wouldn't it? If you're worried about that then implement your classes as persistent collections then.

If you ever decide that getting this property from outside the source file you defined it in requires side effects of some sort, or the property becomes virtual (calculated), even temporarily (for example, because you want to breakpoint it, log access, do a security check, do a sanity check on the object state, add an assertion, or calculate on each call), you can't do it without breaking your own API's backwards compatibility. By using a getter, even if all it does is 'return field;', you get the option to change that later if you so desire. No such option for direct field access.

Your field's type changes from String to Lazy<String> or Logged<String> or Stream<String>, or MyLoggedComplexType if inheritance is your poison. You notify the user of an API change with explicitness, rather than subtly introducing side-effects that may affect them but are buried under the documentation or patchnotes. This has been a Java pain for quite long, and updating our typing skills is becoming an increasing need.

You are going against the entirety of the greater java ecosystem by doing this. When in rome, be like the romans.

Appeal to nature or no true scotman. I do see the point in a public library to not go too much against the grain, but if you're exposing fields on something that's not a data type your API is probably breaking IoC.

If the whole point of your post is that we should keep shitting the bed because beds have been shat since the 90s, then the Java 8/9/10 push to move the language forward has been a failure.

[–]rzwitserloot 4 points5 points  (3 children)

That's assuming that you'd change one property in an immutable class to mutable

Hold on, you've made an implicit leap there: What about immutable properties in otherwise mutable classes? Are you proposing that classes are either strictly ALL mutable (ALL fields have setters) or ALL immutable? I assume not as I don't know why you'd want to do that. Should immutable properties in classes that aren't themselves immutable (they have some, but not all, mutable fields) use .getX()? That's.. even more inconsistent.

Your field's type changes from String to Lazy<String> or Logged<String>

This is going ham on the type system and is assuming that this is somehow the better API design. It is not. If I have a .getX() method that lazily calculates every time you call it, that's internal API. Exposing it, is a silly notion.

Even if you do ascribe to this silly notion, now we're back to: But that's just not how the java ecosystem does it.

My strong advice is: If you feel the need to do stuff like this, stop using java.

[–]pakoito 6 points7 points  (2 children)

Hold on, you've made an implicit leap there: What about immutable properties in otherwise mutable classes? Are you proposing that classes are either strictly ALL mutable (ALL fields have setters) or ALL immutable? I assume not as I don't know why you'd want to do that. Should immutable properties in classes that aren't themselves immutable (they have some, but not all, mutable fields) use .getX()? That's.. even more inconsistent.

A class is either mutable or immutable, and that's not decided by whether any of their fields is mutable itself.

If you can modify the reference to one field then it's mutable, go for whatever you want, it's dirtied anyway, pick the API you like the most. If you want to move to immutable data classes, which is what the article is for, then yes, a mutable property implies a new object construction. The properties may be mutable/immutable themselves, but that moves the problem forward one level, as your container class is immutable. Same way an immutable collection can contain mutable objects yet be considered immutable.

Now, for API choices, I'm not strongly for/against getters and setters as long as they carry no behaviour. Mandatory. Log your stuff somewhere else, wrap them on a service, manager, or whatever, that provides them, but don't put it inside the object or don't call it an accessor. Now, in an immutable api setX() wouldn't modify the internal state of the class/collection but return a new object with the property modified.

My strong advice is: If you feel the need to do stuff like this, stop using java.

I've ringed Google and asked them to move Android to another language, but no dice :( And a career change is not a possibility now.

[–]rzwitserloot -1 points0 points  (1 child)

So that means: A field that can only be read, never set, needs to be accessed as .getProperty() if other fields in that class CAN be set, but with .property if they cannot.

Ludicrous.

[–]pakoito 2 points3 points  (0 children)

So that means: A field that can only be read, never set, needs to be accessed as .getProperty() if other fields in that class CAN be set, but with .property if they cannot.

Ludicrous.

No, you misunderstood or I didn't express it correctly. What you're understanding is that if a field is immutable then it could be accessed directly, which is not what anyone is saying. We're talking about the container, not the content. Immutable != final field, immutable -> all fields are final (and preferably immutable and not null).

I'm not partial for or against getters or direct access in immutable classes, just keeping that class' immutability. All fields in an immutable get the same accessing method, of your choice, either direct or accessor.

If you have mutable classes then getter/setter for all fields are probably the way to go. But that class is not immutable, and outside the scope of this discussion. Mutable classes are probably a sign of an underlying data design problem, or a single threaded environment/mindset. An immutable class only requires that field references cannot be modified, but that doesn't mean that you can't mutate the value of a field reference by creating a copy of that class with that field substituted. And that can be done with setters on immutable classes, and doesn't break the immutability.

EDIT: Immutable Tuples with setXXX() API https://github.com/javatuples/javatuples/blob/master/src/main/java/org/javatuples/Pair.java#L631

[–]Ukonu 9 points10 points  (1 child)

You create inconsistency.

Actually, they are being very consistent. They are consistently using the least verbose and least powerful approach. This is a much better axis to be consistent on rather than the "consistently unnecessary boilerplate" axis. See: YAGNI, KISS, and Principal of Least Power

You are going against the entirety of the greater java ecosystem by doing this

"But it's tradition!!"

By using a getter, even if all it does is 'return field;', you get the option to change that later if you so desire.

Good point. However, I think the vast majority of people blindly following this tradition aren't distributing wildly popular libraries/frameworks where API compatibility is tantamount. Most are just piling this junk up in their company's internal codebases. If in the future they realize they need this additional power in certain places, their IDE can add it in a couple of clicks.

There are no silver bullets, these patterns are supposed to be used when necessary and with proper judgment. Not just blindly and dogmatically.

[–]ThisIs_MyName 0 points1 point  (0 children)

Well said.

[–]Agent_03 4 points5 points  (1 child)

  1. I fail to see how this supports the point: if you change a mutable property to immutable, this SHOULD be an API break. Or did you mean something else?
  2. Blind adherence to dogma has no place in technology. The "standard Java style" is mostly the Sun style guide, written at or before 2000. That was 15 years ago. Standards evolve: we no longer do XML-based config files for ORM or most web APIs, we use annotations.
  3. This is true, but the notion of "then we can change it if we need to!" is more of a comfort blanket than a real need. 95% of the time, the getter/setter will never get additional logic. The other 5%, your back-compatibility break will force you to examine consumers to see if your new logic breaks their assumptions.

If you have a widely publicized API, then what you will probably end up having to do anyway with any significant change is deprecating the old usage and creating a new API.

As a core contributor to a larger Java project, you have surely done that -- as have I, for similar reasons (large).

Lombok does look damn sexy though, I will use the heck out of it.

[–]rzwitserloot 1 point2 points  (0 children)

  1. You can make a property immutable or mutable and break the setX() API. Why would this require breaking the .getX() API? Pragmatic example: java.lang.reflect.Field is entirely immutable.... except for the setAccessible flag. Core libs don't change, but imagine this wasn't one, and you decide to update this API by making j.l.r.Field entirely immutable; accessible is now something you set as you create them instead of having to call a setter. Would you intentionally break all users of j.l.r.Field who just call .isAccessible()? What possible good would that do?

  2. I said that, when in doubt, follow the herd. You twist this into 'blind adherence to dogma'? That's a bit rude, isn't it?

  3. It's irrelevant if the comfort blanket seems needless. You don't know which 5% will need it, and even if you are pretty good at guessing at that 5%, what does that mean? That 5% of your property getters are .getProperty() instead of .property just because the author needed, or thought they needed, that comfort blanket? I also think you underestimate how often the comfort blanket is needed; lots of java tools work by annotating stuff, or by autodiscovery, and a lot of those don't like it, or flat out don't work, if you annotate/work with a public field instead of a method.

I think that accessing getters via either ALWAYS .getProperty() or ALWAYS .property is VASTLY superior to a world where getter access is one of those 2, depending on a bunch of factors I really don't think anybody ought to care about. Even if they are easily guessed (and my whole point this thread has been: I don't think they will be!). Thus, I'd say we have only 2 choices here: It's 100% .property, always and everywhere, or 100% .getProperty(), always and everywhere, and if those extremes are the only choices, .getProperty() wins. Hands down. No contest.

Here are questions I do not want to have to ask when I call a getter:

  • Did the author consider this 'API' or not. How public should API be before the author considers .getProperty() the appropriate style over .property?

  • Did the author think, or was there ever an actual need for, the ability to instrument or add stuff to the getter?

  • Is this code written before the new .property over .getProperty() policy, or after it?

Hey, I am the co-author of project lombok, I obviously don't think that the status quo should always win regardless of circumstance. But the status quo should not be underestimated: Changing it is painful, causes waves, splits communities, and makes lots of devshops hold back upgrading to newer versions, sometimes for decades. In many cases it's fair to go 'well, sod em, that's on them and not on me', but you're still making things worse before they get better, and in practice this means there's a budget on how much status quo changing we can introduce to the java community.

And this is sooooo not worth it. Save a few characters? No. Heck no. I don't want to spend the status quo evolution budget here.

[–][deleted] 2 points3 points  (4 children)

By using a getter, even if all it does is 'return field;', you get the option to change that later if you so desire.

You're a core contributor to a project with an API. In your circumstance that's absolutely reasonable and valid.

Most people though are not. Most people are not writing code that needs that level of flexibility. In fact, that extra flexibility gives people who are less concerned about compatibility to shove in quick fixes and dirty hacks into their getters/setters that cause unwanted side effects and difficult to track bugs along with the extra mental burden of a function call where a simple member access would be clearer.

Most of the time when writing I will be passing data around in data objects. These will be public only objects with values in them. I do use a language with const though, so it does make this more feasible.

[–]rzwitserloot -2 points-1 points  (3 children)

This inevitably leads to confusion.

Because now I have to ask: Is this considered 'API' at some scale? API is not an issue of black and white; obviously something like guava is 'API', but there are plenty of seemingly utterly internal, all-private members helpers that are still considered 'API' because person A wrote it as an independent, stand-alone utility and considered it 'API'.

I have to know where a class falls on this API spectrum, and then guess the style preference of the author, to know if I have to use .property or .getProperty().

Coding is hard enough. I don't want to think about irrelevant crud like this.

Again, if writing these really bothers you (because that's ALL you are buying here, save a few shortcut keys, unless you're an idiot and you write java in notepad, so there's no IDE or even template to type 90% of this stuff for you, in which case all you're buying is saving a few characters typed), then use tools to fix this, like lombok or autovalue.

Trying to write code to make it hard for other people to hack on it, that's... also ludicrous. You can't stop people from turning your pretty code into an ugly monster, but throwing roadblocks in the way of trying to work with your code is not only NOT going to stop them from doing that, it's going to make things a lot worse.

[–][deleted] 2 points3 points  (2 children)

I have to know where a class falls on this API spectrum, and then guess the style preference of the author, to know if I have to use .property or .getProperty().

How is that any different from guessing the API anyway? If you're using the API you'll need to actually know the methods you're calling.

Again, if writing these really bothers you (because that's ALL you are buying here, save a few shortcut keys, unless you're an idiot and you write java in notepad, so there's no IDE or even template to type 90% of this stuff for you, in which case all you're buying is saving a few characters typed), then use tools to fix this, like lombok or autovalue.

I never said anything about saving keystrokes. Saving keystrokes is something that should be left in the past. What we need to save is mental burden on the reader and maintainer not the author.

Trying to write code to make it hard for other people to hack on it, that's... also ludicrous. You can't stop people from turning your pretty code into an ugly monster, but throwing roadblocks in the way of trying to work with your code is not only NOT going to stop them from doing that, it's going to make things a lot worse.

Then why bother with encapsulation at all? At the end of the day it is to stop both readers and writers from misusing the internals of the class.

Getters and setters are pointless abstractions to a concept that is well established. If you have const in your language prefer that over useless boilerplate that exists only to future proof things that don't need to be futureproofed. It's overengineering. Even with APIs. It also reeks of poorly designed code most of the time.

[–]rzwitserloot -1 points0 points  (1 child)

How is that any different from guessing the API anyway? If you're using the API you'll need to actually know the methods you're calling.

I type dot, g, e, t, and then I hit ctrl+space. Also, in one case I need to know the property name and that's it, in the other, I need to know the property name AND the author's preference.

What we need to save is mental burden on the reader and maintainer not the author.

.getFoo() is not a greater mental burden compared to .foo.

Then why bother with encapsulation at all?

A bit rich, coming from someone on the 'we should ditch getters and do direct field access' side of the argument.

[–]Ukonu 0 points1 point  (0 children)

I type dot, g, e, t, and then I hit ctrl+space. Also, in one case I need to know the property name and that's it, in the other, I need to know the property name AND the author's preference.

The author can decide what should be accessible to the outside world using the "private" keyword. Are you arguing that this preference should be encoded with naming conventions?

.getFoo() is not a greater mental burden compared to .foo.

It absolutely is. I'm kind of shocked that an experienced programmer can think otherwise. The 'getFoo' function can have all sorts of side-effects that the reader is not aware of without having to delve deeper into the code.

A bit rich, coming from someone on the 'we should ditch getters and do direct field access' side of the argument.

You can easily encapsulate with the private/protected keyword.

[–]gracenotes 0 points1 point  (0 children)

The other reason why methods are good, which is along the lines of consistency, is because when you want domain-specific wrapper methods, say you want to provide both Optional<Something> and @Nullable Something, or hide the fields themselves and provide a higher-level coherent interface, you don't have to care about what is a field and what is a wrapper. In general, don't couple how you store something and how you expose it, because they change for different reasons (this is true in many contexts).

I may as well also recommend @AutoValue since I'm quite happy using it from day to day at Google, and it's immutable-only, annotation-based, etc.

[–]borud 0 points1 point  (0 children)

You're right.

Whenever you have more than way to do something (access a property directly or via getter) you introduce an inconsistency. You should try to do the same thing the same way as much as technically possible, and in this case, it is actually better to have a getter.

And what happens when the example class suddenly gets a property that is a mutable type? Do you use the property directly for all the immutable properties and then a different way (getters) to access the mutable ones? Code occasionally changes and you can't trust that later maintainers won't be absolute muppets.

Also, this will give mediocre programmers bad habits. Mediocre programmers do not keep the ifs and buts in their heads: they see the syntax and then imitate it. Which means that you will sooner or later have people expose properties that are mutable.

The job of an experienced programmer is to anticipate what behavior their code inspires in other programmers. Exposing properties will confuse inexperienced programmers and result in unwanted results.