3

I was researching on getters/setters, and the general idea is that they are evil and should be avoided. You should let the object do the work, and produce the result.

Reading Material:

Why getter and setter methods are evil

Are getters and setters poor design? Contradictory advice seen

Why use getters and setters?

With all that in mind, suppose I have a Book class that looked like this:

publc final Book{

    private final String title;
    private final List<Authors> listofAuthors;
    private final int numberofpages;

    //constructor not included in example.

    //getters for title, listofAuthors, and numberofpages

    public int numberofpagesleft(int pagesRead){
        //code to determine number of pages left
    }

    public boolean isWrittenBy(Author author){
        //code to determine if book contains author
    }

}

If I had a UI(ex. JavaFX, webpage, etc..) and wanted my class to be flexible, would including the getters for title, listofAuthors and numberofpages for display purposes break encapsulation?

Example:

//library is an List<Book> collection, and author is an object of the Author class.
for (Book book : library){
    if(book.isWrittenBy(author){
      System.out.println("Books in your collection written by " + author.toString() + " include:" );
      System.out.println(book.getTitle());
    }
}

or

for (Book book : library){
    //returns a Collections.unmodifiableList().
    if(book.getAuthors(){
        //code to build a set of authors in my library
    }
}

Questions:

  1. Is calling getTitle() or getAuthors() in the loop breaking encapsulation?

  2. If the answer to the above question is yes, how would I display the book if isWrittenBy() returns true? and How would I gather all the authors in my library?

  • Well it seems reasonable that one would want to know the title of a book, so by all means yes, include a getter. – assylias Oct 10 '17 at 17:14
  • @assylias - how about getAuthors()? Suppose I want to see all the authors in my library regardless of book title, would including a getter for authors to build a set, be okay? –  Oct 10 '17 at 17:17
  • 1
    "getters are evil"? Seriously? Quote you source, please. – AJNeufeld Oct 10 '17 at 17:55
  • @AJNeufeld - You can find several pages on SO, and a article on Java World here: https://www.javaworld.com/article/2073723/core-java/why-getter-and-setter-methods-are-evil.html –  Oct 10 '17 at 20:17
  • From the link: _"A fundamental precept of OO systems is that an object should not expose any of its implementation details. This way, you can change the implementation without changing the code that uses the object. It follows then that in OO systems you should avoid getter and setter functions since they mostly provide access to implementation details."_ Neither the title of a book, nor the authors of a book, is an "implementation detail" of the book; rather they are first class attributes of the book, and providing a getter to return those attributes is not evil; it should be a requirement. – AJNeufeld Oct 10 '17 at 20:25
  • @AJNeufeld - I don't fully understand what the term implementation detail means. Could you explain further? –  Oct 10 '17 at 20:35
  • `getAuthors()` should not return `listofAuthors`, unless it is an `unmodifiableList`. Otherwise, the caller could change the authors of a book. Returning it as a `List<>` might be an implementation detail. Is it really an ordered list of authors? Perhaps it should it be a `Set<>` of authors, since an author shouldn't appear more than once? Perhaps a `Collection<>` would be better, since then the implementation could change from `ArrayList<>` to `HashSet<>` without the caller being impacted. – AJNeufeld Oct 10 '17 at 20:36
  • @AJNeufeld - :D In my example code I said `unmodifiableList` –  Oct 10 '17 at 20:37
  • @AJNeufeld - My concern now, is if I return an `unmodifiableList` and build a `Set` outside of the Book object, so I could display a list of authors in the users library regardless of the Book. Does that break `encapsulation`? –  Oct 10 '17 at 20:43
  • Again, the authors of a book are not an implementation detail of the book. They are a detail of the book, and must be retrievable from the book. You want to hide __implementation details__, such as whether authors are stored as a `List` or a `Set`. Returning a `Collection` from `getAuthors()` would hide this detail, but again you'd want to wrap that as a `unmodifiableCollection()`. – AJNeufeld Oct 10 '17 at 20:52
  • @AJ - If you gather all the comments you made into a answers, I'd be more than happy to check it as the correct one. :D –  Oct 10 '17 at 20:54

3 Answers3

6

There is a school of thought that claims that an abundance of get/set methods shows a careless object-oriented design. You ask how you would display your "book" data if your Book object didn't have the appropriate getXXX methods. The answer, presumably, is you tell your book to display itself:

MyUserInterface interface=...;
...
book.renderToUserInterface (interface);

I think there is some force in the arguments again over-using get/set methods -- I would argue that a good design makes programmatic classes good models of the real-world things they represent, and real-world things generally aren't just repositories of discrete data items that can be set and queried. I think we do have a tendency to think of classes (in Java and other languages) as nothing more than pimped-up data structures, and they can be more than that.

However, the idea that get/set methods are "evil" just goes too far, in my view. Dogmatic avoidance of these methods is unlikely to be helpful, particularly when a lot of what goes on in most OO programs is really supporting infrastructure around the classes that model domain objects. Moreover, "making the object do the work" makes most sense when considering a complete OO model for an application. At the level of the design of an individual object, for use in a general purpose library, for example, I think we have to be a bit pragmatic.

I seem to recall that "make the object do the work" was a hot idea about ten years ago, but I'm not convinced that is such a big deal any more.

Kevin Boone
  • 4,092
  • 1
  • 11
  • 15
  • So my use of getters to grab the data from the object to display, is fine, correct? –  Oct 10 '17 at 20:18
  • 2
    Well, it wouldn't worry me. The question, from a "make the object do the work" perspective, is -- can you make the class participate more actively in the display process, rather than just being a passive store of information? Maybe you can, maybe you can't. IMO it's worth thinking about these things, but not getting too hung up on them. Just my $0.02, of course. – Kevin Boone Oct 10 '17 at 21:14
1

There is a difference between attributes of an object, and implementation details of an object.

  • A book has a title - that is not an implementation detail.
  • A book has authors - that is not an implementation detail.
  • How authors of a book are stored - this can be an implementation detail.

Getters are not evil, but you must use them carefully, since they can expose implementation details which restrict changes to your implementation.

class Book {
    private ArrayList<Author> authors = new ArrayList<>();

    public ArrayList<Author> getAuthors() { return authors; }
}

The above is bad, for two reasons. First, it returns a modifiable collection to the caller, allowing the caller to change the collection. Second, it promises the caller that the returned collection is an ArrayList.

Book book = ...;
ArrayList<Author> authors = book.getAuthors();

We fixing the first problem by wrapping the collection:

class Book {
    private ArrayList<Author> authors = new ArrayList<>();

    public List<Author> getAuthors() {
        return Collection.unmodifiableList(authors);
    }
}

Book book = ...;
List<Author> authors = book.getAuthors();

Now the caller can't modify the collection, but they are still promised the collection is a List. If we find we want to store our authors as a Set (because an author doesn't author a book more than once), we cannot simply change the internal storage.

class Book {
    private HashSet<Author> authors = new HashSet<>();

    public List<Author> getAuthors() {
        return Collection.unmodifiableList(authors); // ERROR - authors not a list
    }
}

We would have to collect the authors into a new List, or change the signature of the getAuthors(), but that would impact the callers who expected a List to be returned.

Instead, the code should simply return a Collection. This does not expose the implementation detail of the storage of authors.

class Book {
    public Collection<Author> getAuthors() {
        return Collection.unmodifiableCollection(authors);
    }
}

Note: it could be that the authors are ordered, with the primary author listed first. In that case, you may want to return a List to promise the caller that the data is, in fact, ordered. In that case, the List is not an implementation detail, but rather part of the interface contract.

So, does getTitle() break encapsulation? No. Absolutely not.

Does getAuthors() break encapsulation? Depends what it returns, and what you are promising. If it returns a Collection, then no. If it returns a List and you are not promising ordered results, then yes.

AJNeufeld
  • 8,526
  • 1
  • 25
  • 44
  • "Encapsulation" is about preventing access to things, while "getters" are about allowing access to things. Therefore `getTitle()` breaks encapsulation by design, as do all "getters". Now, you can still argue that it is ok to have getters in a specific case. I might even agree in some cases. But you can not argue that it does not break encapsulation. – Robert Bräutigam Oct 11 '17 at 08:23
  • @RobertBräutigam Are you certain `getTitle()` is breaking encapsulation by accessing internal detail? The class may not have a title field, but instead `SELECT title FROM BookTitles WHERE id = ?` when the method is called. That may not be the most efficient implementation, but it is not wrong. Or maybe the method does `return properties.get(“title”);`. You assume that there is a `title` field backing the `getTitle()` method, and you might be right. But you might be wrong, too. The implementation may freely change, so encapsulation is not broken; the method simply gets the book’s title. – AJNeufeld Oct 11 '17 at 13:51
  • @AJNeufeld I'm talking about what is colloquially referred to as "getter method" or "accessor method". The method that returns a value from an instance variable, like the `getAuthors()` method in your answer. Do you agree then, that those are breaking encapsulation? I'm not sure where you stand based on your comment. – Robert Bräutigam Oct 11 '17 at 14:09
  • @S.R. It's ok to say that you think this would work best in your case, or at least that you don't see a better design at the moment. That does not mean we should change the meaning of words, like encapsulation. That is something I object (ha!) to. Design is always a compromise, it's ok to acknowledge breaking some rules if you think it's the right way forward. – Robert Bräutigam Oct 11 '17 at 14:21
  • @S.R. I can't write a fully qualified answer in a comment unfortunately. Summary: Depending on how you interpret SRP it *may* break it. But, things like encapsulation, information hiding, cohesion, coupling are much more important than any interpretation of SRP. And all of those say that *all* related functions should be in the object itself, including presentation, at least to some degree. – Robert Bräutigam Oct 11 '17 at 14:55
  • @RobertBräutigam - Why not just post a full answer? –  Oct 11 '17 at 16:04
  • @S.R. I think KevinBoone below got it right, I would probably just repeat what he said. "Tell your book to display itself". – Robert Bräutigam Oct 11 '17 at 16:18
  • @RobertBräutigam - He says to have the book display itself if you don't have the getters, however, in my case it does have getters. My real problem is, I've only recently seen that type design, I'm not saying it's wrong. Also, when ever people say not to use getters/setters, they never present an answer of how to display in a GUI, they only talk about behavior, and having a method return the answer. –  Oct 11 '17 at 16:37
  • @RobertBräutigam - you gave a better answer here: https://stackoverflow.com/questions/43708069/encapsulation-and-getters I understand it, and it answers my question, but whenever people talk about getters/setters, they come to the conclusion it's evil, fine, but they never talk about what to do instead given certain situations –  Oct 11 '17 at 16:39
  • @RobertBräutigam - The articles I link to all claim getters/setters are evil, fine, BUT, none of them present a solution to, or even mention displaying information in a GUI –  Oct 11 '17 at 16:43
  • 1
    The problem here is we are arguing about OO principles with a book as the subject. Books don’t do anything; people do things to books, like read the book’s title. If we were talking about a car, one would not write `car.getEngine().start()` to start the car. Instead, one would write `car.start(ignition_key);` and the car would be responsible for starting the engine if it had one (it could be electric, after all). `getEngine()` exposes internal details of the car’s implementation, which only a mechanic would need to know; a driver would just start the car. – AJNeufeld Oct 11 '17 at 17:41
  • 1
    Continuing with the car; one still may want to call `car.getMake()`. The manufacturer of the car is a **visible** part of a car, along with the model and license plate. You cannot hide visible parts; doing so just makes the class unusable. Visible aspects must be part of the interface. The title and authors of a book must be visible. This does not break encapsulation: the title and authors are part of the book. They’re just visible parts by necessity. – AJNeufeld Oct 11 '17 at 17:49
  • @AJNeufeld - A well written answer that I fully agree with. This should be the go to answer when it comes to getters and encapsulation. I simply don't understand why some developers fully avoid getters and place UI code in the class. That's simply wrong, and it isn't a clean approach. The class shouldn't know or care about how it's being displayed. Upvote! –  Jun 08 '18 at 13:51
1

This is just auxiliary comment on some different, but related (and sometimes advanced) topics, and is not a direct answer to the question (which is already well answered in other answers). Perhaps the info here is not relevant for here or your question, if so, just ignore it :-)

On Immutability, i.e., Just Not Adding Setters

Getters are usually fine as the caller can't change the object, but unnecessary setters can make it more difficult to reason about an object, especially when writing concurrent code.

When you have setters, the source of the object's change can come from a wider area, another class and even another thread.

See Immutable Objects in the Java tutorials and a synchronized sample with setters which is just plain more complicated and potentially error prone vs an immutable version without setters.

The JavaFX Color class is an example of an immutable class in the JDK.

Generally, when I make a class, I usually try to put all of the inputs into constructor parameters, make the class final, not add setters and make all the members of the class final. You can't always do that, but when you can it seems to make programming easier to me.

Note, setters are often useful and necessary, so don't worry about adding them in if you need them, immutability is just a sometimes desirable attribute and not a rule.

On bridging the data model and the display

If you are really interested in different patterns and conventions of getting data from the object to display, then read Martin Fowler's GUI Architectures as it gives a very good overview of this.

Usually, you want a separation of concerns from the data and domain objects and the UI. For small apps, you don't need that separation and it is just more overhead. But, as soon as you start developing larger apps, it usually becomes a lot better organized if you separate the UI from the domain model completely. Usually, observables help to do this, often coupled with a design pattern like MVC, MVP or MVVM.

On JavaFX Properties and Binding to the Display

If using JavaFX, as you mention in your question, properties and binding provide an observable interface so a change in your domain object can be reflected automatically in your UI. You can define the properties directly in your data and domain classes. You can define your UI using something like FXML and an FXML Controller. In the controller you can bind properties of the UI (e.g. the text string of a Label) to properties in your data and domain objects. That way, if you internally change the value of a property in your domain, e.g. the number of books in the library, you can expose that number as a property which is automatically updated in the UI whenever a new book is added (without you having to explicitly tell the UI to update the label when the book is added).

For example in your LibraryController, you might have:

@FXML
Label numberOfBooksInLibraryLabel;
Library library = new Library();

...

numberOfBooksInLibraryLabel.textProperty().bind(
    library.books().sizeProperty().asString()
);     

On JavaFX Properties and Encapsulation

JavaFX has some conventions around JavaFX properties which are a bit more complex and subtle than plain Java without JavaFX. To help with encapsulation and JavaFX properties, consider using FXCollections.unmodifiableObservableList() and read only properties.

private final ReadOnlyDoubleWrapper size = new ReadOnlyDoubleWrapper();

public final double getSize() {
    return size.get();
}

public final ReadOnlyDoubleProperty sizeProperty() {
    return size.getReadOnlyProperty();
}
jewelsea
  • 150,031
  • 14
  • 366
  • 406