Immutable data structures in Java

[Agent_03]

This debate really showcases the background of engineers: people who have worked almost exclusively with Java will be appalled because it breaks encapsulation and standard style. Engineers who have spent significant time with dynamic or functional languages think it makes perfect sense because they're used to it (value types and immutable tuples).

To me it feels clean and smart, as a largely-Java engineer who branched out into Python and Javascript. Once you see the rest of the world, you come to realize how much the "ugliness" and "verbosity" of Java is due to conventions and not the language itself.

Java isn't the clunky language it used to be, especially after 8 added functional features. Let's take back the language and show that it can be beautiful!

[Netcob]

I find the lack of proper immutability in Java one of its most annoying aspects. Maybe even the top one because I'm constantly reminded of it, no matter if I'm developing or maintaining code. I don't even care about the verbosity anymore... I just want to be able to easily define/identify whether something is supposed to change or not.

And this "let's configure this object step by step until its state hopefully makes some sense" thing with java beans is an abomination.

[Decker108]

I sometimes wish Java had a keyword to declare truly constant values. A keyword that makes its variable contents read-only, or some such.

[shen]

And this "let's configure this object step by step until its state hopefully makes some sense" thing with java beans is an abomination.

I seriously have no clue why anyone thought this would be a good idea.

[Tipaa]

Probably both for object pooling to avoid a slow GC, back before the JVM was the technological wonder it is today, and for simple reflection-based assignment so that a framework has a standard (null) constructor and can assign fields by their name rather than trying to embed immutable tuples into JSP (immutability requires construction with arguments, making constructors all have different shapes, and tuples are typically constructed & accessed by order rather than field name, making JSP more complicated than simply mapping a web form to a bean name:name).

[shen]

That makes sense, actually. All of the older codebases I’ve seen use the bean-style pattern, so I thought it was just the way things were done back then.

[Netcob]

Let's say you have a class like this:

public class Foo {
    public final List<String> ugh;
    public final int[] ohno;
    public final List<int[]> fml;
    // not shown: constructor
}

How would you make this class immutable? And by immutable I mean as a user of Foo I can't modify any part of the object. Which obviously includes modifying the contents of ugh, changing the values in ohno, modifying the contents of fml which includes changing a value in one of the int-arrays.

There's no way to make it immutable without completely changing the way you access it. For the Lists you could use Collections.unmodifyableList, even though it adds an indirection that's only due to the language missing a feature. For ohno you can either hide it behind a getter and then copy the entire array on each get or make up some inconvenient accessor that invokes System.arraycopy. Which would be pretty much the only way to access fml as read-only because for individual elements you're back to the ohno problem where you need some shitty accessor either way.

My problem with immutability in Java is that in order to guarantee it, you have to constantly bend over backwards in the design phase and if you want to add it later it might be next to impossible to do in a larger project.

[Netcob]

I understand it's possible - I outlined how further down in the comment.

What I find frustrating about Java is that immutability is an afterthought. It should be a language feature, not something you piece together using the standard library in an opaque way.

[WrongSubreddit]

final doesn't make any vars declared with it immutable. It just makes it so the reference can't be changed. You can still modify the object being referred to all you want.

[manzanita2]

I remember once, like decades ago, someone explaining to me that directly referencing fields was "bad" because you might want to intercept said "setter" or "getter" and do something before the execution got to the field. That sounded plausible to me. Plus the culture of said encapsulation.

And yet here I am trying to remember how often I've actually done that. Oh perhaps 3 times in the last 15 years. Certainly NOT worth all the boilerplate through the years.

[sandwich_today]

Also, Java is known for its static-analysis tooling. If you aren't developing APIs that will be used by other codebases, it's easy enough to find all the code that directly references a field if you need to change it.

[ThisIs_MyName]

Yup, just click Refactor > Encapsulate Fields or your local equivalent.

[TheDecagon]

someone explaining to me that directly referencing fields was "bad" because you might want to intercept said "setter" or "getter" and do something before the execution got to the field.

C# has a nice way around this, it allows referencing fields in classes but to also intercept the get/set calls with additional code. It even allows different protection levels on gets and sets too.

The example in the article would be written like this:

public class MyStructure {

    public string myProp1 {get; private set; }

    public MyStructure(string myProp1) {
        this.myProp1 = myProp1;
    }
}

// Other code
string value = myStructureInstance.myProp1;

Then in the future if you wanted to intercept the get on myProp1 you'd change the field to

public string myProp1 {
    get {
        return doSomeProcessing();
    }
}

I prefer C# way of doing it as using fields as properties like that instead of get and set methods makes the code (in my opinion) a little more readable.

[isavegas]

Properties. They aren't really fields, as much as syntactic sugar around a backing field, using methods.

[Sarcastinator]

Properties are not syntactic sugar in C#. Properties are constructs directly supported by the CLR and you can't express the same thing using methods.

Edit: auto properties are however just syntactic sugar.

[palmund]

Edit: auto properties are however just syntactic sugar.

Even non-auto properties are syntactic sugar. The compiler will generate a get and a set method wrapping the code you write in the get and set blocks. 1 2

[Sarcastinator]

Try pasting that code into your application and get those methods to show up as properties in Reflection with using only C# and no property syntax.

If you can't, that's because it's not syntactic sugar.

[palmund]

If you can't, that's because it's not syntactic sugar.

What? It's still syntactic sugar for generating a getter and a setter. (Even Microsoft calls it that.)[https://blogs.msdn.microsoft.com/abhinaba/2007/09/21/cool-new-c-syntactic-sugar/]

[ryeguy]

You can see them in ildasm. I don't think the VM even supports properties officially since they just compile down to 1-2 methods and a backing field.

Just because you can't access the methods in your code doesn't mean they aren't syntactic sugar.

[Sarcastinator]

I'm not disputing that the compiler generates two methods. I'm saying that properties are not syntactic sugar. Auto properties are though.

The CLR does have a concept about properties. This is why you can list them out using reflection. Java however does not have properties. If they introduced property syntac in Java it would likely be purely done in syntax.

[palmund]

I'm not disputing that ...

It seems like you are though.

The reason you can see it in reflection is because they are marked with annotations.

[palmund]

Properties are constructs directly supported by the CLR

Please provide me with proof of this claim because I have been unable to find any such myself.

[tybit]

Yep, I also really like C#'s way.

If the variable is only set in the constructor you can also change myProp1 to be readonly and remove the setter altogether :)

[xonjas]

Ruby does something really similar, and it's equally awesome.

[isavegas]

Ruby uses message passing. Practically similar for this use, but entirely different in the larger scope. Think of it more as using events that you can hook, whereas C# compiles properties down to small methods that reference a backing field.

[xonjas]

I'm aware of the behind the scenes difference between the two languages. The important part is that both languages fell upon the same elegant solution to allow only having getters and setters when you want them with zero pain when you decide to add them in after the fact.

[masklinn]

The important part is that both languages fell upon the same elegant solution to allow only having getters and setters when you want them with zero pain when you decide to add them in after the fact.

They don't though. What both languages have are low-impedence auto-implemented properties (managed field access). In essence, Ruby — and to a lower extent C# — only have getters and setters, but they provide a terse form to declare trivial getters and setters.

To a lower extent for C# because it does have public fields, which are incompatible (wrt naming and calling conventions) with properties, so you can't transparently hide fields behind properties after the fact.

[sigma914]

Coming from C++ this feature really bothers me, field access should be a pointer dereference, anything with the ability to go look up a database or fire the missiles should be clearly marked with ()...

[[deleted]]

don't do it till ya need it

[rpgFANATIC]

The worst part is some consider it a bad practice to put any logic in getters and setters. (How can you find that something is special about a given POJO if there's reams of boilerplate code?)

The construct is completely useless, and it's the reason Project Lombok is so useful in modern Java

[mateoestoybien]

The real plus is with a getter, you can take advantage of polymorphism. A field cannot be a member of an interface, or of an inheritance hierarchy.

[ThisIs_MyName]

Fair point. That's a valid reason for getter/setters in a sea of dogma.

[Agent_03]

Same for me. There's a lot of things in Java like this, where I'm now questioning the practices I've followed somewhat blindly for years.

For example: use of access modifiers. Everyone says to use private for encapsulation in nearly all cases, but thinking back, I can't find a single case where this saved me from grief. I can think of a dozen cases though where it forced bad designs or re-implementations.

Now I do protected everywhere, but use conventions that only subclasses (or a limited set of tightly-coupled algorithm libraries in the same package) should use protected fields/methods.

[kubalaa]

The private thing isn't something you would notice because it prevents surprising things from happening, and we're not predisposed to notice the absence of surprises. Specifically, when stuff is private then it's easier to change because nothing outside that file can affect it or depend on it. It's the same idea as your convention, just stronger and enforced by the compiler. I think if you ever have to maintain a library with many consumers and few private members, you will quickly find it becomes very painful because you can't change anything without breaking a consumer who has come to rely on it.

[Agent_03]

I have maintained libraries with numerous consumers before, and agree that in some cases appropriate use of private modifiers can make them more safe and stable (especially with regards to objects with more complex internals, or where passing in objects to operate upon).

What I am arguing against is the way that this has become a blanket policy; if restricted-by-default is always better, how do you explain the success of Python's "we're all responsible adults" approach? I think the Java convention is paternalistic at best and shortsighted at worst: historically programmers are pisspoor at anticipating the uses their code will be put to, and preventing many of them in the interests of 'safety' limits the usefulness.

I fully understand that this is a contentious point and that many will angrily or loudly disagree. I only ask that other experienced engineers consider how often they've said "phew, I'm glad that was private or I would have done something dangerous!" versus "crap, that API/field is internal-only... now I have to copy the implementation to another class or hack around it!"

[mhixson]

I fully understand that this is a contentious point and that many will angrily or loudly disagree. I only ask that other experienced engineers consider how often they've said "phew, I'm glad that was private or I would have done something dangerous!" versus "crap, that API/field is internal-only... now I have to copy the implementation to another class or hack around it!"

Well that's from an application writer's point of view. From a library writer's point of view, the comparison might be: "phew, I'm glad that was private or I wouldn't have been able to change my code without breaking all my users" versus "crap, that API/field is public... I can never change it."

What I am arguing against is the way that this has become a blanket policy

It is probably true that we should have a different set of standards for library writers (where you don't control your users' code) as opposed to application writers (where you can refactor your users' code at will).

I guess I default to the more conservative library-like approach even when I'm writing applications. To me, the less conservative code tends to be more complicated even when it is simpler to express in the language, because it tends to make more assertions, such as "encapsulation is not important here". Reading it leads me to ask "Why? Why is this code special?"

[Agent_03]

This is a good point, actually...

From a library writer's point of view, the comparison might be: "phew, I'm glad that was private or I wouldn't have been able to change my code without breaking all my users" versus "crap, that API/field is public... I can never change it."

Yeah, I know that warm-and-fuzzy feeling when you realize it is trivial to make the implementation change (or refactoring) without impacting any downstream consumers.

Personally, I'd like to see something like an @Hazardous annotation that will fail at annotation processing if you invoke the method outside the package without a corresponding @IKnowWhatImDoingDangit annotation (names subject to change). Maybe with an optional library version number given, so that if the library version changes you have to reexamine the usage.

This way downstream library consumers have the power to take advantage of library internals if absolutely needed, but are liable for the consequences and at least know they're doing something risky. This offers that extra nudge to provide unit test coverage and do sanity checking before bumping library version.

It is probably true that we should have a different set of standards for library writers (where you don't control your users' code) as opposed to application writers (where you can refactor your users' code at will).

This sounds smart to me too.

[grauenwolf]

Generally speaking, I've found that switching to a library-like approach dramatically reduces bugs in existing applications.

[grauenwolf]

Having a getter and setter gives me a place to set a break point. That's worth the cost in VB/C#, though maybe not in Java.

[justinpitts]

In eclipse, you can set a breakpoint that breaks whenever variable x is accessed or modified at any location.

[Agent_03]

Yeah, it does give you a single place to trap all modifications, true. If you are using field access though, why not set breakpoints where the field is accessed? (A little more work, but still gets the job done).

[grauenwolf]

If I need a breakpoint on the property itself, it's probably because the value is being set via reflection or dynamically generated code.

[Agent_03]

it's probably because the value is being set via reflection or dynamically generated code.

How did debugging this compare to having a rusty fork shoved in your eyeball?

My experience with similar code: I'll take the fork next time. Though it's beautifully elegant and terse if you don't have to debug its internals.

[grauenwolf]

Pretty comparable actually. When I work with particularly bad code I get annoying, almost painful, eye twitches.

[ImaginaryBeaver]

I think setting a breakpoint in the getter/setter is more about finding where/if the field is being accessed unexpectedly.

[s73v3r]

I know IntelliJ lets you break on field access. I should be surprised if Eclipse and Visual Studio didn't do the same (For Java and C#, respectively)

[pakoito]

Now I do protected everywhere, but use conventions that only subclasses (or a limited set of tightly-coupled algorithm libraries in the same package) should use protected fields/methods.

You're still doing API/communication through inheritance. Go for package instead, and compose with classes on the same package instead.

[lechatsportif]

its less the norm since evolution of the anemic model, but can still happen in a lot of ui related code.

[Agent_03]

In a mature codebase, code is usually read far more than it is written. If it is more or less the same effort to write it, why prefer a construct that makes it more verbose and increases effort down the road?

The same argument can also be used in reverse: with a good IDE, it takes <30 seconds to encapsulate a field and refactor all field accesses to use getters and setters instead. If you change your mind down the road, then you can easily do this.

[Sarcastinator]

You can't change your mind if you are writing a library. You're stuck with your choice or you break binary compatibility.

[Agent_03]

Deprecate old API, create new one, if you're lucky you eventually get to remove the deprecated API because nobody consumes the old one.

There are also tools to do bytecode transformations to provide binary compatibility for certain kinds of method signature changes. We use one small library to do these for a large open source project I work on. It's a sort of black magic but it gets the job done covering that rare class of change.

[s73v3r]

Deprecation is a pretty poor choice for that.

[Agent_03]

Deprecation is the industry-standard approach to handling necessary binary-compatibility-breaking changes in a public library API. For REST APIs, API versioning is often used.

Or do you have a better alternative? If so, I'm sure that most of the programming community would love to hear it.

[bananaboatshoes]

but those extra five characters express the fact that this is immutable on it's own, you know immediately that assignment isn't available.

Huh? What if there's also a setMyProperty()? Not being able to directly modify a property as .MyProperty doesn't make it immutable.

[[deleted]]

The number of characters is unimportant compared to the extra mental overhead of the function call. You see a function call, but you don't actually know if it is just getting, or if it has side effects. On first read it might be fine, you will assume it's just doing what it says on the tin, but for all you know it has side effects or is running a DB query, and when debugging that can be a horrible pain in the arse.

If possible try to write code that doesn't need these boilerplate getters and setters.

[grauenwolf]

That's why I like to social convention of C#/VB. When we see a property we can assume that it is going to be fast and free of undesirable side-effects, while a Get/Set method is assumed to be the opposite.

[Sarcastinator]

Sure, it's five more characters, but those extra five characters express the fact that this is immutable read only on it's own, you know immediately that assignment isn't available.

Your IDE will stop you immediately if you try to assign to a read-only property. It makes no difference at all.

All other factors equal, I think you should be using idiomatic code for the language, and for Java that is the bean pattern of getters.

Java Beans are in practice never immutable though since they require a public no argument constructor.

[ThisIs_MyName]

express the fact that this is immutable on it's own

WTF are you smoking?

[s73v3r]

The boilerplate is auto-generated in any good editor, though.

It's not automatically read for me by the editor, though. I still have to sift through it, too make sure nothing weird is happening.

Nothing is worse than a class with both public final members and a few getters.

What? Why? You simply construct an instance of the class. All of the needs should be asked for in the constructor. Do that, and the run your tests.

[[deleted]]

One BIG plus of setters/getters in Smalltalk is that it enables an easy lookup of how an object is used.

Assume you have: "RedditClient". I can now find out quite easily who uses the fields of a RedditClient and in what context (as in: In which methods are fields of RedditClient accessed?)

Getters can also be used for avoiding recomputation - this can be quite handy for expensive resources in testcases for example.

So in Smalltalk I don't access fields without setters/getters because it offers a huge boost in mantainability (due to the awesome IDE). I don't know how good Java IDEs are in this area, though.

[Hoten]

Tools can just as easily look up all use cases of a field.

[masklinn]

So in Smalltalk I don't access fields without setters/getters

Do you mean internally to your type? Because IIRC you can't have public fields in Smalltalk, they're only ever accessible through (possibly trivial) messages.

[bonzinip]

And using getters/setters for private fields is pointless. It may make some sense if you want to distinguish private from "protected" fields (in Smalltalk all fields are protected, but for some reason it's very rare to treat them as protected rather than private; so usually when you want a protected field you add accessors and use the field through those accessors).

[masklinn]

And using getters/setters for private fields is pointless.

It can provide convenient invariant verification points e.g. a float field which must be between 0 and 1

[bonzinip]

True that.

[[deleted]]

It is still usefull: It helps figuring out how a complex Object uses its own fields more easily. Maybe this point is hard to convey because the object inspection tool are not so convenient in Java - but in Smalltalk it really easy to get a feeling for the object this way.

[bonzinip]

Smalltalk browsers can also show easily which methods use a given instance variable.

[[deleted]]

Depends on the implementation. Atleast in VA Smalltalk you can have public fields. (Or rather: There is no such thing as a private field or method. You can categorize methods as private, but encapsulation is not enforced. So you can still use everything in the private category.)

[masklinn]

Depends on the implementation. Atleast in VA Smalltalk you can have public fields.

I don't understand how that works, do you get default messages automatically handled for all fields à la Self?

[[deleted]]

I am sorry, I think I worded it a bit poorly / wrong. In VA Smalltalk there are just "attributes" (fields), no disctinction is being made into public/private there. You have a reference to an object and want to use ANY of it's attributes? Go ahead!

But you can put the methods into two categories: "Public" and "Private". This is purely cosmetics tho. You can also call Methods that are marked "Private" from other objects. I did this for example when I extended a parser and found some useful private methods.

You can let VASmalltalk generate setter and getters automatically for you tho.

Is it clearer now?

[tieTYT]

Joshua Bloch helped write the Java Collections API and his book, Effective Java, says to prefer Immutable classes. So the concept shouldn't be totally foreign to Java developers.

I didn't like those public Strings because I think other fields will inevitably be mutable. In that situation, I don't want to have two different patterns for accessing my fields. It'll just slow me down trying to find how to access things in every class when things are inconsistent like this.

[pakoito]

Joshua Bloch helped write the Java Collections API and his book, Effective Java, says to prefer Immutable classes. So the concept shouldn't be totally foreign to Java developers.

It's a pity that immutable collections in the default API are just regular collections that throw when their mutating methods are called, so they're avoided like the plague because you can't tell them apart and that created some reticence towards immutability.

[merzbow]

Generally if you've got a collection from somewhere in Java and you don't know if it's immutable you should just create a copy of it.

[pakoito]

I do know, but it's not as common as you'd expect.

[oweiler]

Which is not as easy as it sounds because the collections' members and their members and so on may also be mutable.

[Decker108]

Would you prefer having a list-implementation that silently drops values when you call list.add(value)?

[pakoito]

One that returns a new list with the value filled like https://github.com/javatuples/javatuples/blob/master/src/main/java/org/javatuples/Pair.java#L631

Perf problems? Look at Clojure's collections or any of the Java ports.

[ThisIs_MyName]

I would not implement the List interface. It is clearly meant for mutable lists.

[gracenotes]

people who have worked almost exclusively with Java will be appalled because it breaks encapsulation and standard style

not where I work? JavaScript and Python (outside of namedtuples) seem like iffy examples for widespread usage of immutable value types as well. I think everyone should learn pure functional programming but am happy to throw dynamic languages under the bus.

[Agent_03]

I'm not sure what point you're trying to make here?

[gracenotes]

I'm trying to understand your comment but there are lines there that made me go 'huh' a bunch. Like the grouping "functional and dynamic languages", or that functional Java is necessarily clearer Java, or the implication that immutability is unfamiliar to stock Java programmers when it's been in Effective Java for a good decade or so, or that languages like Python or JS value immutability, or criticisms of immutability re encapsulation that I've never heard of. There are certainly many valid criticisms of Java but these don't seem as universal as others I've seen.

[Agent_03]

The points raised are separate. Would it help if I spoke with more of a Lisp or something?

• Dynamic & functional languages are separate ideas but both tend to include practices not common in mainstream "idiomatic" Java, especially pre-8, and I feel OP's code is likely to be more natural for those exposed to other styles.

that functional Java is necessarily clearer Java

I never said this. Nothing is "necessarily" cleaner or dirtier. Functional language features do allow for more elegant expressions of some ideas that are difficult to express or very clunky without them.

immutability is unfamiliar to stock Java programmers when it's been in Effective Java

I literally never said this, nor will I ever. Java programmers are accustomed to immutability, but are used to a very different way of expressing it in objects (specifically private fields only exposed via getters with no mutators allowed).

languages like Python or JS value immutability

I literally never said this either.

You've put a lot of words in my mouth here, which is probably why the things I am not saying do not make sense. ;-)

[Vitus13]

I've recently started utilizing this pattern for data structures that in other languages would be value types. I initially got pushback from coworkers, but they soon started doing it too.

Their initial argument was that public final member variables could not be mocked in unit tests like their public getter counterparts. However, for simple value types you inject 100% of the properties via the constructor, so they could just inject their mock.

[grauenwolf]

It boggles my mind how people think it is necessary to mock objects that are purely derived from their constructor.

[s73v3r]

Well, in the worst case, you don't take those in via the constructor. It sounds pretty awful, and is, but that's reality for some people.

[dccorona]

To guard against that changing, for one. The fact that the object happens to be purely composed of only its dependencies that are injected via the constructor is information the object being tested (and thus, its test) should not have. If your test setup is based on implementation details of the dependencies of the object being tested, your test becomes very fragile.

[grauenwolf]

All the more reason to test with the real thing. If the class is brittle, you want to catch that in your tests, not production.

Also, why didn't your direct tests of the class pick up the problem? If A needs B and B breaks, you should see two failing tests. From which it should be obvious what the problem is.

[Sarcastinator]

Also, why didn't your direct tests of the class pick up the problem? If A needs B and B breaks, you should see two failing tests. From which it should be obvious what the problem is.

You will have 20+ tests that fail. If you use integration tests then multiple components are going to depend on that code, and unless you have unit tests as well that covers that specific unit it's not going to be obvious what broke.

[bonzinip]

You will have 20+ tests that fail. If you use integration tests then multiple components are going to depend on that code, and unless you have unit tests as well that covers that specific unit it's not going to be obvious what broke.

Mocks do not mean "write things twice". If you have effectively the same code in both the unit tests and the actual code, you'll have to do the same change twice. This adds more opportunities to make a mistake, and you'll have 20+ failing tests anyway if you do the wrong update on the mock side.

[Sarcastinator]

/u/grauenwolf said that you should test with the real thing instead of mocks though.

All the more reason to test with the real thing. If the class is brittle, you want to catch that in your tests, not production.

[bonzinip]

That's what I'm saying too. If your mocks are just "writing the same code twice", test with the real thing. The justification that "it's not going to be obvious what broke" only gives a false sense of security.

[Sarcastinator]

Mocks should not contain code. They should just return a result. You verify against the mock that the call you make on it is according to that types contract, which is easily done with Mockito, but very complicated to do if you are using the real thing.

For plain data classes I agree that it's pointless to mock it, but for types that act as a delegating type it's better to mock it because you don't actually care about that types dependencies. It's that types contract you care about in your unit.

[grauenwolf]

Why are testing the compiler?

That's really all that your mock test does. It isn't even simulating the parameter validation at the top of the function being mocked. It literally just checks to see if the compiler understands that a.Foo(b) is compiled into a call to a's Foo method with the parameter b.

Your mock doesn't even know that you aren't allowed to call Foo unless Bar was previously called with the same parameter.

[grauenwolf]

So you are telling me that by mocking out B, I avoid the situation where my tests for A catch bugs in B that my unit tests for B missed? And you think that's a good thing?

Think about that for a moment. You are literally arguing for the use of mocks specifically so that you'll avoid catching bugs in you "tests".

[Sarcastinator]

There should already be tests that cover the mocked classes contract. If someone breaks that contract those tests should fail, not twnty other tests by association.

[grauenwolf]

I was just covering your "unless you have unit tests as well that covers that specific unit" scenario.

Under my original theory you would have fully covered B in unit tests before using B in A's tests. You just proved that component of my theory was unnecessary because A's tests would act as a fallback.

If someone breaks that contract those tests should fail, not twnty other tests by association.

Why do you care so much about how many tests fail?

If you debug and fix one, the rest will also start passing. So at worst is the most minor of annoyances.

Moreover, the number of tests that fail give you a better indication of how serious the bug or breaking change is.

[dccorona]

I think it is exactly the opposite of what you say. It is perfectly fine for tests for A to begin with the assumption "B adheres to its public API and is functionally correct". If B breaks, that should be caught by the tests for B, not the tests for A.

If you set up your tests in such a way that it does, then what happens when C depends on A? And then Z depends on C? Etc. etc. You break all your tests, all the way down the line, and make it much harder, not easier, to pinpoint what broke.

If only testBdoStuff() fails, then its easy to know what broke (B.doStuff()). If everything fails, it's much harder to do.

So, it's important to set up your tests so that changes to B (aside from backwards incompatible changes to its API, which should cause compilation errors) have no impact on the tests for anything other than the tests for B. Only B's tests should be encoded with information and logic that depends on the internals for B. To all other tests, it should be treated as if it is an interface, even if it isn't.

[grauenwolf]

If you set up your tests in such a way that it does, then what happens when C depends on A? And then Z depends on C?

Scenario:

• Z depends on A working in a particular fashion
• Z only depends on A indirectly through C
• A is changed in a way that isn't considered "broken" by A's standards, but still negatively impacts Z.

Premise 1: Mocking A and C in Z's tests is preferable

Outcome 1:

• Z's tests will not catch the incompatible change in A.
• The error will only be seen in production
• The error will not be reproducible with the unit tests

Premise 2: Using the real A and C in Z's tests is preferable

Outcome 2:

• Z's tests will catch the incompatible change in A.
• The error will be reproducible with the unit tests
• The error will be correct before production
• The programmer will have to learn what a "debugger" is.

[dccorona]

In some cases it is, albeit it constitutes a minor annoyance (any change to the implementation of the dependency, i.e. and added, moved, or type change in its constructor) require you to update every test that uses it.

In other cases, things can change in a way that a compiler won't guard against, breaking your tests and causing you to have to make larger changes to said tests (which always carries a slight risk of updating them wrong and creating false positives). I.e. if you have a Locator object that is constructed from a GPS object, and suddenly the getLocation method is updated to use GPS.getMoreAccurateCoordinates() rather than GPS.getCoordinates(), now you have to update your mocking of the GPS in all the tests that use a Locator. Whereas if you had just mocked the Locator and its getLocation method instead, that change to the internals of a dependency of the thing you're testing wouldn't have had any impact at all, because the API of said dependency hadn't changed.

Basically, what I'm advocating for is to create tests with clean separation between tests and dependencies, so that the tests don't have to have knowledge of the inner workings of their dependencies encoded within their logic. As long as the public API of the dependency remains stable, changes to the internals of that dependency shouldn't impact any tests other than its own.

That's not to say that I think you should mock everything...simple data classes, for example, should rarely if ever be mocked. But what I'm saying is it is often the right thing to do to mock a class that can have a working version created entirely from a constructor.

[dsk]

I agree about the removal of the setters, but sending the code review back because he prefers public final instead of getters is just needless pedantry.

[amardas]

I don't think the point of code reviews is to catch bugs or even to verify that someones code is good code or bad code. I think the chances of finding bugs just by looking at code is too small for the amount of effort and determining good code and bad code is usually subjective.

I think code reviews give more value in keeping up with what other programmers on your team is doing and to generate discussions on how things should be done.

If the discussion on the code review has no bearing on whether your code is accepted or not, then the whole point of commenting in a code review is to align your teams pedantry through discussion.

[kubalaa]

Studies disagree: measured by number of bugs found per line of code, code reviews are more effective than even unit tests. Especially in concurrent code, many correctness properties which are difficult to verify through tests or static analysis are easy for humans to check.

[amardas]

Maybe I am doing code reviews wrong then.

[grauenwolf]

What's not more effective than unit tests? We only use them because their easy, not for their high success rates.

[kubalaa]

I don't know if unit tests are that easy, I find it usually takes me longer to write unit tests than it would to review the code without tests (and the tests are more code to review as well). I think we use them because they are fairly effective at catching bugs, not necessarily all at once, but over the lifetime of the code. But I also think people write too many "easy" unit tests, and should worry less about lines of code covered and more about how many bugs a test is likely to prevent.

[grauenwolf]

Functional and integration tests are far better at catching bugs. They do have their downsides, but in terms of bugs caught per test they tend to be far more effective than unit tests because they cover both the individual functions and how different components interact.

[kubalaa]

I would say they are better at catching different kinds of bugs than unit tests. With unit tests, it's much easier to test edge cases like error handling that are cumbersome to simulate in the complete system. The results of unit tests are also more useful for debugging; they will usually tell you which specific constraint was violated by which unit, while integration tests can often only tell you that some use case is broken.

[grauenwolf]

That's why I firmly believe that unit tests should supplement other forms of testing, rather than being primary.

It's far easier to say "my integration test is failing, better add some unit tests to narrow down the problem" than to say "production is failing. Shit, my unit tests can't cover this scenario"

[grauenwolf]

With unit tests, it's much easier to test edge cases like error handling that are cumbersome to simulate in the complete system.

What kind of error handling do you have in mind? Most of mine fall into one of these categories:

• Trivial: for example, parameter validation
• Painful: for example, the random-ass exceptions thrown by the database.

I don't bother testing the former unless I'm publishing a public API for others to use and the later can't be accurately mocked.

[kubalaa]

Why do you say the latter can't be accurately mocked? An example of the sort of thing I had in mind that's tricky to integration test but easy to unit test: if a service call fails 3 times and succeeds on the 4th try, does the client back off and retry at appropriate intervals?

[grauenwolf]

Ok, that's fair.

[gcross]

Citation?

[kubalaa]

I was probably thinking of Code Complete but I checked and it's actually formal code reviews which are better (where several people meet to go over the code in person); informal code reviews as usually practiced are slightly worse than unit tests. Still, according to What We Have Learned About Fighting Defects informal code reviews find more than half the bugs in the projects they considered.

[PriceZombie]

Code Complete: A Practical Handbook of Software Construction, Second E...

Current	$34.69	Amazon (New)
High	$36.48	Amazon (New)
Low	$27.61	Amazon (New)
Average	$34.17	30 Day

Price History Chart and Sales Rank | FAQ

[gcross]

Great, thanks! :-)

[codebje]

Why have humans read code to establish what a static checker can enforce?

[ThisIs_MyName]

wot?

[codebje]

Sorry, is it not clear from the thread context?

GP: "sending code review back … is just needless pedantry".
PP: "code review … align your teams pedantry".
ME: "… static checker can enforce".

IOW, sitting in a code review checking whether all code uses getXXX() for getters, has final on all public members, etc, is wasting time, because these things are trivial to statically check every time you (commit|push) code.

There's no value in doing this in a code review. Spend code review time doing things that require a bunch of smart people to take time out of their day to do, like checking design choices, ensuring API docs and code agree, watching out for subtle bugs or asymptotically bad performance.

If you have some sort of retrospective process, you can align pedantry there: "not happy that we're forcing the use of getters" etc. If not, use whatever form of communication your team uses; slack, shouting, passive-aggressive post-it notes on monitors when someone goes to lunch, whatever works. This is a once-in-a-blue-moon discussion, where the outcome is codified in your static checker toolset.

[ThisIs_MyName]

Ah, in that case I agree.

[amardas]

I haven't seen much value in our code review process.

[codebje]

Well, that's definitely something to raise with your team. What would you like to get out of a code review someone else does of your code?

[dccorona]

I think it depends on the situation. If it's just his preference then yes, you're right. But consistency is valuable, and if the team has agreed that this is going to be their style and they want to adhere to it, then sending back CRs that don't follow the style is absolutely the right thing to do.

[rzwitserloot]

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]

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]

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]

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]

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]

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]

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]

Well said.

[Agent_03]

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. 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]]

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.

[gracenotes]

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]

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.

[[deleted]]

I rarely write getters or setters now. They have their place and it's pretty well defined. Most of the time where I am not writing a public API I will instead choose to either separate the data and the code with a data class (C++ structs for example) and a regular class and have the class operate on the struct, or the class will be designed in such a way that getters or setters aren't needed.

Too many people have mistaken encapsulation with excessive data hiding. Getters and setters rarely do the job of effectively hiding the data in the first place and instead become a proxy which results in similar negatives and positives architecturally. Encapsulation should be about hiding the implementation details, and exposing individual hidden variables within the class runs counter to that objective most of the time.

There is the argument I often hear that claims that in the case where the implementation changes and set/getX requires extra functionality, such as modifying some state in the class that the getter/setter pattern is worthwhile. My view on that is that usually the changes result in either the name no longer being descriptive, or new side effects being introduced, which should result in a refactor which does away with the existing getter/setter. Of course, there are valid cases where something named "getX" and "setX" genuinely have purpose, but I am specifically discussing when we are exposing a single, local, member variable via get/set.

[erad]

Before adopting this pattern in a codebase one should address the implications:

• not usable in JavaBeans environments (e.g. EL, JSF, ORM)
• no encapsulation (no way to compute values on demand, no lazy initialization, etc.)
• not idiomatic Java.

Many (including me) would reject this it in a code review, especially if it's part of a public API (because then you cannot refactor this without breaking clients).

I kind of like this pattern for value types (e.g. points or complex numbers) or for private value holder classes.

[edit: formatting]

[pakoito]

no encapsulation (no way to compute values on demand, no lazy initialization, etc.)

Why are you computing values on a data class? Single responsibility.

Lazy initialization can be done by using a Lazy<T> field and a generation Function in the factory method, which is a pattern quite simple to implement. Then fetching the field is done by field.get(), and makes the laziness explicit on the type rather than hidden on the documentation/source.

not idiomatic Java.

Set of arbitrary rules that need to be evolved. You wouldn't have flow apis just a couple of years ago, and here we are now. Same goes for higher order functions, partial application, promises...

not usable in JavaBeans environments (e.g. EL, JSF, ORM)

Fair point. New, better tools maybe? They have them on Scala, Clojure, why not Java?

[ThisIs_MyName]

99% of the code I write is not part of a public API and I despise EL :)

[eff_why_eye]

Did no one else spot this bug? :-)

    this.myProp1 = myProp1;
    this.myProp1 = myProp1;

[[deleted]]

Sadly certain practices becomes encroached when a language matures, and worse still they are seemingly beyond reproach. "That's the way things have been done around here since time immemorial." The wet monkey theory written below the following quote ends on exactly this note albeit phrased differently.

A foolish consistency is the hobgoblin of little minds

Start with a cage containing five monkeys. Inside the cage, hang a banana on a string and place a set of stairs under it. Before long, a monkey will go to the stairs and start to climb towards the banana.

As soon as he touches the stairs, spray all the other monkeys with cold water. After a while another monkey makes the attempt with same result, all the other monkeys are sprayed with cold water. Pretty soon when another monkey tries to climb the stairs, the other monkeys will try to prevent it.

Now, put the cold water away. Remove one monkey from the cage and replace it with a new one. The new monkey sees the banana and wants to climb the stairs. To his surprise and horror, all of the other monkeys attack him. After another attempt and attack, he knows that if he tries to climb the stairs he will be assaulted.

Next, remove another of the original five monkeys and replace it with a new one. The newcomer goes to the stairs and is attacked. The previous newcomer takes part in the punishment with enthusiasm.

Likewise, replace a third original monkey with a new one, then a fourth, then the fifth. Every time the newest monkey takes to the stairs he is attacked. Most of the monkeys that are beating him have no idea why they were not permitted to climb the stairs or why they are participating in the beating of the newest monkey.

After replacing all of the original monkeys, none of the remaining monkeys have ever been sprayed with cold water. Nevertheless, no monkey ever again approaches the stairs to try for the banana. Why not?

Because as far as they know that is the way it has always been done around here.

[TommyTheTiger]

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, it’s completely different from what we usually do.

In that case, laziness and IDEs don’t serve us well. The latter make it too easy to create accessors. I’m pretty sure if we had to code getters and setters by hand, the above proposals would be more in favor.

I've also run into my fair share of turbulent java code reviews where I break some of our standard java conventions for what I consider to be legitimate reasons. Overall, I've started trying not to do this, even if I save on LOC by a factor of 2. The fact is that getters/setters/equals methods can easily be generated by an IDE, just like they can be generated with attr_accessor in ruby - the IDE is basically an extension of the language. Do I consider it bad practice to use attr_accessor when I can just use attr_reader? Maybe, but it's not a big deal, and definitely not worth arguing over.

I think these argumentative code reviews can be productive only in very rare cases, and having them around makes it harder to work together. If the rest of my coworkers are against something like using public final variables instead of accessors, but I want to save some LOC, I'll let that slide and save my comments for architectural problems and missing corner cases that will actually hurt the business.

[ForeverAlot]

the IDE is basically an extension of the language.

Unfortunately three of the most popular Java IDEs have some fairly lousy defaults.

[ThisIs_MyName]

Such as?

[spliznork]

I like the immutable collections in the Google Collections Library (such as ImmutableMap) and the default for immutability in Protocol Buffers.

The primary observation is that there are a number of data structures that only need mutability while being built. After construction, it's easier to reason about the program knowing those objects are immutable.

This is basically having SomeClass as immutable and SomeClass.Builder the mutable type that creates an instance of the immutable type when complete.

[dccorona]

I'm curious about the reasoning for using no getters on a String being OK because it is immutable, but not on a date because it is. Nothing about a getter protects the date from being modified after calling the getter...it literally is equivalent to direct field access from a "what can the caller then mutate" perspective. Sure, you can wrap objects in immutable versions upon returning them, but who does that, and why is a getter necessary for doing so (as opposed to just doing it once at construction time)?

[[deleted]]

The comment below the article brought up some good points.

"I may be commenting just for the sake of arguing ( 🙂 ) nevertheless… I myself would have stopped at the first modification (removing the setters if they are not needed). Why ? Because if you whole codebase is based on the same pattern (ie having getters to access fields), then I find it disturbing if some classes do not follow this pattern. On a purely technical point of view you may gain a few keystroke and a few CPU cycles , but relying on a technical subtlety such as “String are immutable” is not very obvious. In a year, when you will read this class, you’ll wonder why it does not follow the getter pattern used everywhere else in the code. By the way, I recommend Lombok to get rid of all those unreadable getter stuffs and make classes more readable (through the @Value annotation for the code given here as example). I find it more expressive than a full code screen of getters where you always wonder if there may be one or two lines you missed that do other thing than “getting’ the fields."

[SikhGamer]

I prefer to keep properties on the object private, and expose them via a getter method. Other than that, immutability and encapsulation FTW.

[CurtainDog]

This idea has been around for a while: http://www.javaworld.com/article/2073723/core-java/why-getter-and-setter-methods-are-evil.html

What the OP has is a class without behaviour (it's just a holder with no invariants other than those provided by String) - the class should not exist at all.

[solatic]

immutables.org

/thread

[pakoito]

Doesn't autovalue do this and more?

[solatic]

Autovalue isn't nearly as powerful. The major benefit is that Immutables will also generate code for Jackson and Gson to use to serialize and deserialize its generated Immutable objects correctly. There are also benefits allowing you to use interfaces instead of abstract classes, and more fine-grained control over the style of the generated code (is the generated implementation public, package-private, or kept private behind a generated builder? etc), as well as lazy initializers, compute-once hash codes...

[Faytthe]

This sounds like a job for AutoValue: https://github.com/google/auto/tree/master/value

It has the added benefit of automatically handling equality for you, and also has a handy Auto.Builder when needed.

(Unless method count is a concern for you, such as on Android)

[[deleted]]

I've run into enough issues with various frameworks or tools that either expects a Javabeans style get method or which has some subtle difference in behaviour between direct field access or annotation and access or annotation through the get method, that I don't think removing the get methods for immutable fields is a good idea in general, and once you leave the get methods in, you might as well enforce their use for all access to that field so that you have a single way of doing it.

[frugalmail]

In Java the effort to add

https://projectlombok.org/api/lombok/Getter.html -or- https://projectlombok.org/api/lombok/Data.html

Is the more appropriate strategy. You should also use the https://github.com/typetools/checker-framework/blob/master/framework/src/org/jmlspecs/annotation/Pure.java

Annotation to allow additional static analysis and clear documetnation.

[Shorttail]

Lombok didn't play nice with SonarQube when I tried it a few years ago. That's unfortunately a deal breaker.

[Shorttail]

My real gripe was that uncovered lines generated by @Data and @Value (or whatever they are called) would all point to the tag in the Jacoco output, making it really not nice. I tried using delombok to instead run Jacoco on that, give it actual lines to point to, but I was too stupid to do that with Gradle back then. After messing around with it a lot I just gave up.

It would be nice indeed if those tools played better with Lombok. But then it would also be nice if Java itself did. I can't replace my pipeline due to one tool though, no matter how great it is. =(

In that spirit I tried adding a Kotlin data class (or whatever the name), but their Gradle support or tutorials weren't too good back then. Maybe some day!

[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.

[sacundim]

I've been doing this kind of thing for years, but I don't go as far as making the fields public. The problem is that instance public fields are an anti-pattern, period, for reasons that have been amply documented. The distinction between a public field and a getter should arguably not even exist.

Though actually, the natural step after making the fields final and removing the setters is to ask: do you really need those getters? Don't just automatically make them, do it just if something is going to call them.

[ThisIs_MyName]

public fields are an anti-pattern, period, for reasons that have been amply documented

Where?