Stop using noargsconstructors and setters (and builders)

noargs_setters_builders.md

Stop using noargsconstructors and setters (and builders)

TLDR summary

Noargsconstructors and setters are outdated, 90's style old school Java. They needlessly allow entire categories of defects that are easily avoided by using only allargsconstructors and no setters. Please stop writing code like that.

Longer version

How many times have you come across (or written) code like this

public class User {

    private String email;
    private String username;

    //either no constructor (= Java automatically adds default noargsconstructor)
    //OR explicit noargsconstructor public User (){}

    public String getEmail() {
        return email;
    }

    public void setEmail(String email) {
        this.email = email;
    }

    public String getUsername() {
        return username;
    }

    public void setUsername(String username) {
        this.username = username;
    }

    //maybe some builder too, who knows

}

or using Lombok or whatever

@Data

//or

@NoArgsConstructor
@AllArgsConstructor
@Getter
@Setter
@Builder

//or whatever other hodgepodge of annotations
public class User {
  private String email;
  private String username;
}

(NOTE the User example class is a "data" class, but that's besides the point - it doesn't matter if it's a data class or a Service class or a Resource class or a Controller class or DAO class or what have you)

You've probably come across this more times than you can count, even in fresh code, right?

Why it's bad

Briefly, it 1) needlessly allows instantiating objects in an invalid state, and 2) needlessly allows mutation.

Needlessly allows instantiating objects in an invalid state

Consider

User user = new User();

What does this even mean? This object doesn't even have its most fundamental attributes. What possible use could any part of any system have for such an object? Why even instantiate and pass such an object around, anywhere? It's completely nonsensical, let alone that it's begging to cause problems like NullPointerException etc. It doesn't matter if you immediately after set the attributes - just instantiate the object in a complete, valid state to begin with instead.

Needlessly allows mutation

Unless you have extremely good reason to make objects of a class mutable, by default, your classes should be written such that objects are immutable. This is especially true for this User example - once you get users from whatever is the source of truth for users, you MUST NOT MUTATE THEM, or even allow the POSSIBILITY of mutating them, in ANY way. Otherwise, your code could somehow end up changing (or removing) a user object's email or whatever before passing it on somewhere else. That would be extremely dangerous!

Even when you're dealing with less "important" objects, making them immutable is still beneficial. It keeps your classes shorter and less complex and it makes it a lot easier to debug and reason about the code when you know an object in a given state won't suddenly do a rug pull on you and change state while being passed through the system. It also enables safe parallelization, thread safety and any number of other more "technical" advantages.

Common arguments

These are just some of the most common ones I see, but I'm sure more could be thought of.

What about builders?

The most common style of builder makes use of and relies on noargsconstructors and setters. So if we want to use a builder, that makes it OK, right?

No! A builder doesn't make the problems just go away, nor does it justify allowing them.

Taking a step back, why do you need a builder? What's wrong with using the allargsconstructor? If your class is well designed, intantiating objects is not complicated.

User user = new User("foo@bar.com", "foobar");
// ^^^ this is not complicated, and there is no world
// in which adding a builder to do this is necessary

If you need a builder because instantiating objects is complicated because your class has a lot of attributes and can be in any number of different states depending on which attributes are set and which are not etc etc, that is a smell that your class is poorly designed - fix that first before adding a builder.

I happily admit builders do have one advantage even in well-designed classes: they name the attributes you're sending in, so it's less likely you'll send in the email as the username and vice versa. For that reason, I'm actually not entirely opposed to builders, but I will point out that 1) our IDE's kind of do this for us (but that doesn't help when we're in GitHub etc) and 2) overuse of classes like String for things like email and username etc is actually a problem all of it's own - primitive obsession, which is beyond the scope of this post, but a big source of errors all of its own. (briefly, rather than using String for things like email and username, make actual classes instead, such that new User(new Email("foo@bar.com"), new Username("foobar")))

Anyway, if you really, really need or want a builder, there are builder implementations that don't make use of or rely on noargsconstructors and setters - use them instead. (see eg immutables/immutables#438)

Jackson (or some other tool) requires it

Jackson or some other tool requiring a noargsconstructor doesn't justify adding one - there is almost always a way to enable the tool to use the allargs, here's an example of a class from one of our repos

@JsonIgnoreProperties(ignoreUnknown = true)
public class User {

    private final String userId;
    private final String username;
    //...

    @JsonCreator
    public User(
            @JsonProperty("userId") final String userId,
            @JsonProperty("username") final String username
            //...
    ) {
        this.userId = userId;
        this.username = username;
        //...
    }

    public String getUserId() {
        return userId;
    }

    public String getUsername() {
        return username;
    }
    //...
}

Code review will catch any resulting bugs

Maybe you carefully code review to make sure there are no bugs resulting from objects being in invalid states, or objects not mutating in undesirable ways. But... why not just not code in a way that the compiler will not allow them, and you won't have to (hopefully) catch them in code review? You're not being paid to be a compiler! Code review is supposed to focus on more important things, not trivial bugs resulting from needlessly error allowing implementation details. Code review can never be guaranteed to catch all resulting bugs, but the compiler can.

It's our team coding standards / It's how we've always done it

I don't mean to be uncharitable, but frankly, if your coding standards require you to, by default, allow instantiating objects in states that are demonstrably invalid, and allow mutating objects that must not be mutated, then your coding standards and how you’ve always done things are wrong, and you can and should change them.

What about Currying?

Sometimes, objects have to be instantiated in an incomplete stage at one point in the system so they can be passed around to other parts that add more values before reaching some end complete state. That's fine! But then, make that explicit - make the end methods accept only complete objects of a complete class, and make methods that deal with the objects before they're complete deal with objects that are clearly explicitly incomplete.

public class IncompleteUser {
    private String email;
    //doesn't have username yet
}

public class User {
    private String email;
    private String username;
}

Sometimes mutability is necessary, or more efficient etc than immutability

Sure! Despite what it may seem, I’m actually not making some fundamentalist argument that absolutely everything must always be immutable and mutability is never desirable or permissible. I'm simply saying, rather than by default always allowing instantiating objects in an invalid state and mutation, instead, by default, your classes should not allow mutation and instantiating objects in an invalid state.

This is just your opinion

No, it's not. People like Josh Bloch and Brian Goetz have been talking about this for decades, and there's a reason why not only Java but other languages like Kotlin, Rust etc have moved in this direction with data classes, records etc. Get with the times.

More on the topic

Endless articles, books etc bring up this very basic thing, here are just a few:

• Effective Java by Josh Bloch https://www.oracle.com/java/technologies/effectivejava.html
• Josh Bloch QA https://www.oracle.com/technical-resources/articles/javase/bloch-effective-08-qa.html
• Brian Goetz QA https://www.youtube.com/watch?v=ZyTH8uCziI4
• Another Brian Goetz QA https://blogs.oracle.com/javamagazine/post/what-are-they-buildingand-why-6-questions-for-the-top-java-architects
• Kotlin avoids entire categories of Java defects https://archive.md/kUA0f

What about builders?

Of course no code example of what did you mean

I've been kinda in rush, but what about this

public class Test
{
	public string Name { get; private set; } // only builder can set this
	
	private Test() {} // private ctor, so nobody creates instance in invalid state
		
	public class TestBuilder
	{
		private SomeDatabase _Db;
		
		private string Name { get; set; }
		
		public TestBuilder(SomeDatabase db)
		{
			_Db = db;
		}
		
		public TestBuilder SetName(string s)
		{
			Name = s;
		}
		
		public Result<Test> TryBuild()
		{
			if (_Db.Exists(Name)) // validation in database/3rd api/external system 
				return Result<Test>.Fail("already exists");
				
			var test = new Test
			{
				Name = this.Name
			};
			
			return Result<Test>.Ok(test);
		}
	}
}

Usage:

var buildResult = new TestBuilder(database)
				  .SetName("asd")
				  .TryBuild();

if (!buildResult.Success)
	return Result<yada_yada>.Fail(buildResult.ErrorMessage);

NotificationService.Blabla(buildResult.Data) // (our Test)

If you want to Edit an Test instance, then you create it via builder from scratch, so all validations are performed - immutability or something like that! :)

Of course you can/need to add some methods to builder to make it handy

@tester346 Not bad :) I like using Immutables (which is backed by the style of implementation referenced in the link in immutables/immutables#450)

The real problem of course is langs like Java and C# that enable these anti patterns in the first place. We would all be better off with sound type systems. Here's hoping we all live to see a language succeed in the mainstream that doesn't teach people to code like idiots. 😥

@mindplay-dk Yes, agreed, it would be much better if the languages simply didn't enable shooting yourself in the foot like this, especially not by default and as a matter of tradition, haha. It's very obvious reading Josh Bloch and Brian Goetz writing that they do regret how things were designed and are now trying to put the cat back in the bag. Tbh, I never expected people would still be coding like this in 2022, but they do, and it seems like it will never really go away. I for one would welcome a language that simply didn't have null nor mutation at all, not even Kotlin nulluable types or vars. (Haskell comes to mind but it's pretty hard to pick up)