1

Suppose I have the following List to hold a list of fruits.

Example:

def fruits = ["Apple", "Orange", "Grapes"]
def fruitsBowl = ["Apple", "Grapes", "Orange"]

// Will print false
println(fruits.equals(fruitBowl))

Only after I call .sort() them will both collection be equal.

fruits.sort()
fruitBowl.sort()

// Will print true
println(fruits.equals(fruitBowl))

Suppose if I had a Book class, obviously a Book class will have a list of authors. For this example, the Authors class has implemented the Comparator.

class Book {

    private String title;
    private List<Author> authors;

    Book(title, authors){
        // code to initialize left out
        authors.sort()
    }

My main concern is the equals() method. If two books are compared, even if the book objects are the same, but list of authors is unsorted, it will return false.

If I were to call .sort() the same way I call sort on fruits and fruitsBowl on the collection holding the list of authors in the Book constructor, is it bad practice?

If it's bad practice, what should I do to ensure that the equals method works?

Update:

As per the comments suppose if I used a Set, it makes much more sense, because if the books are written by a list of authors, each name will only appear once, so is doing this also considered bad practice?

class Book {

    private String title;
    private Set authorsSet;

    Book(title, authors){
       authorsSet = authors.toSet() 
    }

A Set is a collection of unique elements and the equality test will work properly, regardless of order.

  • 2
    Possible duplicate of Legitimate "real work" in a constructor? – gnat Aug 28 '18 at 14:56
  • just resolving my deja-vu, but why do you want to do work in the constructor? you are obvs aware of the principle, is it just that you disagree and are looking for examples where it is the correct thing to do? – Ewan Aug 28 '18 at 16:06
  • @Ewan - I don't want to work in the constructor, I just want to understand how things were done, for example, I was thinking about a book log application, to keep tracking of reading progress. From that point it got the ball rolling on other aspects, such as my Book object, how are the authors stored? In the library the books are order my author last name, can I do the same thing? If I see a feature of a program I want to understand how to works under the hood. The only way I figured out to sort the authors was in the constructor, but is that work and therefore should be avoided? – user313955 Aug 28 '18 at 16:12
  • @Ewan - Those are the types of questions I like to ask when it comes to writing code. – user313955 Aug 28 '18 at 16:15
  • @Sveta whatbim getting at is, is your question really 'whats the best way to represent a book with multiple authors and enable sorting by author name in my app' or 'what kinds of work are allowed in a constructor' – Ewan Aug 28 '18 at 16:20
11

Is calling .sort() in the constructor a violation of the guideline that a constructor shouldn't do work?

Not if .sort() must be called in order to construct a valid object.

What is a valid object, you say? A valid object is one that fulfills the constraints you've imposed on it. What are those constraints? Whatever you say they are.

var sortedList = new SortedList(unsortedList);

Naturally, there are practical limits. Yesterday, someone asked a question about the Spring Framework AnnotationConfigApplicationContext class. Instantiating this class takes 15 seconds in his application, because it registers and makes available to the application a couple hundred forms. Well, 15 seconds is a pretty damn long time in computing terms, but I have applications on my computer right now that take that long to load.

This guy's problem is not the amount of work that the constructor is performing; it is that he's doing that work on every page load of a web application.

"Constructors should not do real work" is a red herring. Focus more on appropriate use.

  • Exactly what I was after: Not if .sort() must be called in order to construct a valid object. – user313955 Aug 28 '18 at 15:12
  • See update to question. – user313955 Aug 28 '18 at 15:41
  • 3
    I think you may be focusing too much on the idea of a "best practice." There is no such thing as a "best practice." There are only those practices that best meet your specific requirements. Find out what works best for your specific situation, and do that. – Robert Harvey Aug 28 '18 at 16:11
  • @RobertHarvey I both agree and disagree with your statement in the previous comment. There are best practices, and some people do focus too much on them. Worse then that I find people are misguided into thinking some practice is a best practice, when it's not really. In this case, I'm not sure that "Constructors should not do real work" is a best practice. That said I agree with your answer above. Especially that it's valid if it's required to make object valid. – zquintana Aug 28 '18 at 16:57
  • 3
    @zquintana A practice can be regarded as a best practice if it generally applies. Though good advice comes with a rationale, so you know when it doesn't apply; "best practices" are often marketed without such and are often in danger of being applied mindlessly everywhere regardless. – Deduplicator Aug 28 '18 at 20:23
6

The misunderstanding is that you don't have a List of authors (unless author order is important, such as in academia - but then you'd never even think of calling sort on that List), but rather you have a Set or Collection of authors.

Furthermore, the List contract specifies things that imply that it may be an unsorted list at any time. The .add() method adds to the end always.

As this is dealing with groovy, it is important to note the documentation on == and .equals() - http://groovy-lang.org/style-guide.html#_equals_and_code_code

Lists are equal if they contain the same items in the same order. - http://docs.groovy-lang.org/latest/html/groovy-jdk/java/util/List.html#equals(java.util.List)

Sets are equal if they contain the same items. http://docs.groovy-lang.org/latest/html/groovy-jdk/java/util/Set.html#equals(java.util.Set)

If you don't care about the order of the items in the Collection, don't use a List. If you are sorting the items in a List to get List equality, you don't care about the order.

-3

Yes it's bad practice.

sort alters the List you passed in, thanks to Java's confusing parameters by ref/value semantics.

You don't really want to get side effects from methods (or constructors)

In addition,

  1. It's good to avoid work in constructors, say I'm instantiating a million books to list by title.

  2. It's (probably) a view concern and doesn't need to be in the object

I would override the equality operator on Book to provide your custom comparison. Maybe just compare the ISBN number to avoid sorting the list. Or compare a sorted copy of the list.

Update in response to edit:

List.toSet() will remove duplicate authors. Just pass in a Set and, as a bonus, avoid work in the constructor!

  • Okay fine, but how would I ensure that my equals method remains valid, so that if I compare two books that are the same, with an unsorted list of authors it won't return false? – user313955 Aug 28 '18 at 15:15
  • edited, override equals for custom equality – Ewan Aug 28 '18 at 15:17
  • 1
    @sveta: To clarify the only good thing about this answer: you might be able to get a read on collection equality in some less expensive way than sorting a list. Having the two collections be hash tables or dictionaries would be one way of doing it. – Robert Harvey Aug 28 '18 at 15:19
  • @RH it's not really the expense which is the problem, what if i have a view which displays the authors in a different order, then I compare that book with a copy fresh out the of the DB? – Ewan Aug 28 '18 at 15:22
  • @sveta also, i have deja-vu, did you delete all your old questions? – Ewan Aug 28 '18 at 15:44

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy