How to avoid getters/setters in classes with many instance variables

Question

I'll try to keep it short.

I have classes with many instance variables (30+) and hence many getters/setters. The classes by themselves are simple, but because of the getters/setters the LOC just exploded (and there also was way too much code duplicity).

So I removed the attributes and stored them in a map, like this

public class MyTechnicalToolClassX
{

//...constructor

private Map<String, Object> data;

public Object getAttributeByKey(AttributeKey key)
{
    // ...doStuff, check data associated with key, etc
    // if (predicate == true) return otherData;
    return data.get(key.toString());
}

public void setAttributeByKey(AttributeKey key, Object value) throws IllegalArgumentException
{
    if(key.getType().isInstance(value))
    {
        data.put(key.toString(), value);
    }
    else
    {
        throw IllegalArgumentException("The passed value has the wrong type, we expect: "
        + key.getType().getName());
    }
}

public enum AttributeKey
{
    someKey1("key1", String.class),
    someKey2("key2", Date.class),
    //...
    someKeyN("keyN", SomeType.class);

    private String key;
    private Class valueType;

AttributeKey(String key, Class valueType)
{
    this.key = key;
    this.valueType = valueType;
}

@Override
public String toString()
{
    return key;
}

public Class getType()
{
    return valueType;
}

} // AttributeKey

} // MyTechnicalToolClassX

AttributeKey used to be just a string, but this way I can ensure type safety in the setter. Now my question, I removed the code duplicity within the class, but I have other classes that also have many attributes (because they represent some technical objects...), what is the best approach here? To give every class its own AttributeKey enum?

SOLUTION

I added some more thoughts. I have type safety now at compile time. Here is my new interface for the getter and setter.

public <Type, SetVal extends Type> void setAttributeByName(IAttribute<Key, Type> attribute, SetVal value);

public <Type> Type getAttributeByName(IAttribute<Key, Type> attribute);

Joshua Bloch calls this kind of concept typesafe heterogeneous container.

Answer 1

Forget what you learned in OOP school!

We've come a ways in 3 years. We have much better languages now. Swift, Rust, Kotlin, Go, etc. We understand the difference between data/value types and the code that manipulates it. Proper enterprise CLEAN architecture advocates for this in the Java realm. But Java just doesn't provide language-level support for such patterns. The end result is heavy use of (still wonderful) things like RxJava, and annotation processors that do code generation, etc. But it's really hard to be happy dragging around Java's legacy these days. Kotlin tends to solve Java's problems in a way Java can't at a very very minimal price: loss of strict source compatibility (it's not Groovy).

Original answer, with update at the bottom.

There are two answers.

One:

Useless Boilerplate!

If the class represents some large data object it sounds like most the member variables are just containers for data. When something like this is the case it's less important to strictly follow the OOP convention of information hiding. People often confuse the purpose of this convention and end up abusing it. It is merely to prevent programmers from having to deal with complicated and unnecessary inner workings of an object. Rather than mask an entire object, just mask the parts that shouldn't be messed with. In a case where you are simply mapping information from a database or are acting as a storage container, code like:

import java.util.Date;

public class Article {

    protected int id;

    protected String guid;
    protected String title;
    protected String link;
    protected Date pubDate;
    protected String category;
    protected String description;
    protected String body;
    protected String comments;
    
    protected Article (String articleTitle, String articleBody) {
        title = articleTitle;
        body = articleBody;
    }
    
    protected Article (String guid, String articleTitle, String articleLink,
            long publicationDate, String category, String description,
            String articleBody, String commentsLink) {
        this(articleTitle, articleBody);
        this.guid = guid;
        this.link = articleLink;
        this.pubDate = new Date(publicationDate);
        this.category = category;
        this.description = description;
        this.comments = commentsLink;
    }
    
    protected Article (int id, String guid, String articleTitle, String articleLink,
            long publicationDate, String category, String description,
            String articleBody, String commentsLink) {
        this(guid, articleTitle, articleLink, publicationDate, 
                category, description, articleBody, commentsLink);
        this.id = id;
    }

    protected int getId() {
        return id;
    }
    
    protected String getTitle() {
        return title;
    }

    protected String getGuid() {
        return guid;
    }

    protected String getLink() {
        return link;
    }

    protected String getComments() {
        return comments;
    }

    protected String getCategory() {
        return category;
    }

    protected String getDescription() {
        return description;
    }

    protected String getBody() {
        return body;
    }

    protected void setId(int id) {
        this.id = id;
    }

    protected void setGuid(String guid) {
        this.guid = guid;
    }

    protected void setTitle(String title) {
        this.title = title;
    }

    protected void setLink(String link) {
        this.link = link;
    }

    protected void setPubDate(Date pubDate) {
        this.pubDate = pubDate;
    }

    protected void setCategory(String category) {
        this.category = category;
    }

    protected void setDescription(String description) {
        this.description = description;
    }

    protected void setBody(String body) {
        this.body = body;
    }

    protected void setComments(String comments) {
        this.comments = comments;
    }

}

.. is utterly abominable.

In a case like this there is really no reason to go through all the extra work just to access members of the data object. Especially if you're just using them in a few external lines of code:

public OtherClass {

    private Article article;

    public OtherClass(Article data) {
        article = data;
    }
    
    public String getArticleContents() {

        return (new StringBuilder())
        .append(article.getTitle())
        .append(article.getCategory())
        .append(dateToString(article.getPubDate())
        .append(article.getBody())
        .toString();
    }
}

Just access the members directly and save yourself hundreds of lines of code (like you've suggested you were trying to do).

This leads to the second answer to this question..

Two:

Design Stank..

Your code may be rotting. Chef Ramsey would be ashamed. Here's why:

Clearly the above OtherClass is entirely useless because its functionality could (and should) be placed in the Article class instead of contained in some other, useless, unwanted , file-system-littering OtherClass. If you do that you can forget about even needing the getters and setters and OtherClass because things interfacing with it may only need the article contents rather than the title, body, etc., separately. In this approach the Article class hides everything from the outside world and only provides the information that's absolutely needed.

Since these are two perfectly viable answers to your problem, there must be a solut j on.

Use Clojure

Modeling Objects

Although you can use closures to model objects, there are even better approaches. The interesting thing is that in a language that treats functions as first class citizens, you can model the traditional object oriented paradigm entirely using maps -- as you began to figure out in your refactoring of the 30-plus-member-field class system you were given.

Compare this to the original Article + OtherClass approach:

(defn Article []
  (let [id (atom nil)
        guid  (atom nil)
        title  (atom nil)
        link (atom nil)
        pubdate (atom nil)
        category (atom nil)
        description (atom nil)
        body (atom nil)
        comments (atom nil)

        set (fn [g t l p cg d b cm]
              (do (reset! guid g)
                  (reset! title t)
                  (reset! link l)
                  (reset! pubdate p)
                  (reset! category cg)
                  (reset! description d)
                  (reset! body b)
                  (reset! commments cm)))
        get (fn [] [@guid
                    @link
                    @description
                    @comments
                    :content (content)])

        content #(str title category pubdate body)]
    {:get get, :set set}))

This above examble is a system that takes points from both answers and combines them into one that hides unneeded members, incorporates logical functionality (the function to get the 'content'), and uses a language that doesn't need incredible amounts of shiny boilerplate code..

Replacing Class/Object System

While this is a nice example of how to model an object in a functional language, it's not entirely idiomatic to Clojure and functional programming in general. For even easier ways to do structured data, look at Clojure StructMaps and Records.

2017 Update

Just use Kotlin. It's curing Java sickness across the board. It has first class language support for all these things and the compiler will even help you get rid of useless boilerplate. It's been around for over 7 years and at a stable release for since February 2016. If I'd have known about it originally, I probably would not have included Closure.