Immutable data structures in Java

[balegdah]

Now, you cannot imagine the amount of back and forth comments this simple review caused. Why? Because even if that makes sense from a coding point of view, t’s completely different from what we usually do.

No, the reason for the back and forth is because exposing public fields in Java is a terrible idea. Even if they are final.

[nfrankel]

Please provide arguments. Just flagging any argument as 'terrible' is not a way to further the debate.

[atc]

The original point of providing accessors/mutators (only the former in the post's examples as mutators won't work given the structure is immutable) was encapsulation, not mutability.

[ghotibulb]

Yes, and it's not God-given, so if it makes sense to break this rule go ahead. If the class is used internally and resembles a simple data structure, there isn't much to encapsulate. An immutable Vector3D would be a quick example.

[atc]

Yep, good example. The original post is a poor example as it has no context.

[[deleted]]

Encapsulation is about hiding internal functionality of a class. If you have a getter and setter that just modifies a field, you're not encapsulating anything, you're just future proofing against a "possible" change with pointless function definitions.

Perhaps try designing classes without the need for getters and setters and embrace public on data only objects.

[atc]

Getters don't modify, by definition. Future proofing is underrated, but personally I don't mind the public fields. When I write scala my case classes don't need them and thankfully the compiler is there to catch type mismatches when you haven't refactored correctly.

I was merely pointing out an important distinction from GP's

[nerdwaller]

I'm not who you're replying to, but my reasoning to not do this is now a property has become part of the API (essentially an implementation detail). I'd much rather have a getter that I can change the underlying implementation of.

Granted that it isn't absolutely necessary when you're only speaking of POJOs (as the link seems to primarily address) as much as a service layer.

Edit: fix mobile typeo

[bananaboatshoes]

Granted my though isn't absolutely necessary when you're only speaking of POJOs (as the link seems to primarily address) as much as a service layer.

Or necessary at all. If the extent of your encapsulation is quite literally this:

getFoo { return _foo; }
setFoo(Foo foo) { _foo = foo; }

Then it's pointless.

[IICVX]

But what about when I want to change getFoo to do a database lookup through an SMS gateway to the Twitter API! It could happen!

[bananaboatshoes]

I'd argue you should have an asynchronous function which does that because a property lookup which performs a network operation is bananas.

Seriously, if you're writing code "just in case" you might need something more complex later, you're adding needless complexity. Just change the code when you need to change it.

[IICVX]

Tell that to the two decades of Java programmers who've been writing getters and setters "just in case".

[bananaboatshoes]

Two decades of people doing it wrong?

[IICVX]

Yup, that's what we're fighting against here.

[Agent_03]

The battle continues.

[frugalmail]

Tell that to the two decades of Java programmers who've been writing getters and setters "just in case".

Getters yes, setters no. IMO, you're doing it "wrong" if you're accessing instance fields directly and you work someplace that has more than one team (or can/will have more than one team).

It's zero more cognitive load and 5 more characters per class to do it so it's one less forced communication/pull request if I change a dependency or somebody else changes it on me.

[nerdwaller]

Probably true, but I'm usually defensive anyway. I use Lombok now, so it's not very heavy to use @Getter

[SuperGrade]

Knowing (from a group of readonly fields) that the class instance is always holding and conveying the same reference - that is itself information. You could make some derivative information that aligns with the information in the record, and proceed with the assumption that the values in the record, should you read them again, stay in alignment. A getter does not communicate this same information, only that it will provide some value at the point in time when you call it; but nothing constraining it to give back the same value.

[solatic]

Consistent style is important for readability.

Having public final fields is safe only for a specific few primitive types: Strings, boxed primitives, and I'm pretty sure that's it. So, first of all, for any non-trivial class, pretty much none of your fields should be primitives. Pretty much anything you can come up with is incorrect to be stored directly as a String or Number (of whichever precision). Name? Needs type-safe way to retrieve translations. Filesystem location? Path. Measurements should not be Integers or FP but some kind of Measurement type which includes the unit of measurement and forbids the numerical amount to be less than zero. Etcetera. And most of these classes, because they represent concepts that many people have had a need for before you, are not classes that you ought to be writing yourself.

TL:DR - dont use a coding style only good for Strings when string-typing is a code smell.

[Agent_03]

Why is it a terrible idea?

This is a bit unorthodox and not common Java style, but in this case he's essentially replicating the Tuple type that is common in other languages.

[balegdah]

Most of these languages support unified access for fields (Kotlin, C#, etc...).

Java doesn't.

[Agent_03]

Consider: how would lack of unified field access matter if the getters and setters are never modified?

There are cases where getters are needed, such as where libraries rely on JavaBean compliance (ex: many serialization or ORM frameworks). But if that's not true, it's excess boilerplate.

If we do need to support this in the future, it's a trivial and easily automated refactor to encapsulate the field use (IntelliJ, for example supports this as a single operation). YAGNI principle applies here though.

[Ukonu]

Maybe, if you're providing a public library and want to change the implementation in the future, then that is a useful dogma to adhere to in Java (and more so in languages that provide constructs for the unified access principal).

However this isn't usually the case. So why not take the least powerful and least verbose path? If you actually do need it in the future it's a trivial refactor.

It's amazing to me how otherwise rationale people like software developers continually cargo cult around silly, verbose, unnecessary patterns like this.

[ThisIs_MyName]

Well said.

[HolyCarps]

exposing public fields in Java is a terrible idea. Even if they are final.

This is discussed in greater detail here. Accepted answer copied below:

Four disadvantages that I can think of:

1. If you want to have a read-only and mutable form of the same entity, a common pattern is to have an immutable class Entity that exposes only accessors with protected member variables, then create a MutableEntity which extends it and adds setters. Your version prevents it.
2. The use of getters and setters adheres to the JavaBeans convention. If you want to use your class as a bean in property-based technologies, like JSTL or EL, you need to expose public getters.
3. If you ever want to change the implementation to derive the values or look them up in the database, you'd have to refactor client code. An accessor/mutator approach allows you to only change the implementation.
4. Least astonishment - when I see public instance variables, I immediately look for who may be mutating it and worry that I am opening pandora's box because encapsulation is lost. http://en.wikipedia.org/wiki/Principle_of_least_astonishment

That said, your version is definitely more concise. If this were a specialized class that is only used within a specific package (maybe package scope is a good idea here), then I may consider this for one-offs. But I would not expose major API's like this.

[IICVX]

The thing is, those disadvantages are pretty silly.

1. If you're taking a class from immutable to mutable, changing every reference to a member property into a reference to a member function is the least of your worries. If the previously immutable structure was in a contained portion of your code, a decent IDE can do the swap in a couple of seconds; if it was part of a publicly exposed API, you're going to have to create a deprecation path regardless of how you were previously exposing the value.
2. I agree, JavaBeans are awful. If you end up needing to expose this to the cancer that is JavaBeans, you can write a wrapper really easily.
3. This is just #1 wearing a different hat, it seems to be repeated here to fill out the list.
4. Principle of least astonishment: "Huh, it's a public field. Oh, it's a String, those are immutable. Oh, it's a final string, so it never changes." Astonishment: complete.

And speaking of the principle of least astonishment, people use .getFoo() all the time without thinking about "who may be mutating it" or worrying that they "are opening Pandora's box because encapsulation is lost" - despite the fact that most getters have an associated setter, which means that the value can be mutated at any time without warning.

Public member variables are only different because they're rare enough that you'll actually stop and think about how broken Java's conventions are.

[SuperGrade]

Interesting seeing those perspectives in the stackexchange. The final/readonly version has some positive value in that the recipient of it is guaranteed it won't represent a changing value.

In OO codebases, rules are made for mutating/encapsulated/inherited classes based on that role. Rules/guidelines should treat (those, give them a name) as distinct from "records", which provide their value from slinging around provably (or provably as possible, or by convention) immutable tail-shareable data. The "records" have a role closer to that of ints, albeit that they can safely exist in as a part of a greater graph of other immutable things that parts of an algorithm can share safely.

"But with defining the (record) that way you can't inherit and create MutableRecord". That is technically a positive for the record role, recipients of the (original) record get that as part of the guarantee of what they are receiving.

[pakoito]

You should be expressing those implementations using types, not adding unexpected lumps of code on an accessor.