1

As a followup to this question and one I asked a while ago, suppose if I had an Author class structured like this:

class Book {

    private String title;
    private Set<Author> authorsSet;

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

I accept a set for two reasons:

  • It doesn't need to be ordered to test for equality
  • It ensures that duplicate objects of Author don't exist

If I needed a getter for the authors set, does it violate the principal of least astonishment to return a List of authors sorted by last name?

As stated in the second link, I want to return a List<Author> with the promise that the List<Authors> will be sorted.

11
  • Why not just name your getter GetAuthorsSorted() or somesuch? Aug 28, 2018 at 18:26
  • @RobertHarvey - I could, but I wanted to know if pass a Set and getting a List in return is okay.
    – user313955
    Aug 28, 2018 at 18:37
  • GetListOfAuthorsSorted() Aug 28, 2018 at 18:54
  • @Sveta: how would you name the getter and setter you have in mind?
    – Doc Brown
    Aug 28, 2018 at 19:46
  • it might not violate the principle, but its still not a great way to design the class
    – Ewan
    Aug 29, 2018 at 6:29

3 Answers 3

3

does it violate the principal of least astonishment to return a List of authors sorted by last name?

It does not violate astonishment, if you document that is what your method does! A list is indeed ordered, but the type systems don't capture the nature of the ordering, so that has to be done via documentation.

However, we should not always try to anticipate all possible ways an abstraction might be used (and encode that within the abstraction), and sometimes let the caller do the work as needed, when that is simple enough for them to do without needing to understand the internals of an abstraction.

In this case, if you provide only the capability to return authors as a set, that can be turned into a sorted list fairly easily by the caller without violating encapsulation of the abstraction here.  That way the consumer can sort locale specific or as otherwise needed (e.g. by how well each author is known, etc..) — and this class (Book) doesn't need to know about sorting for display or other purposes.


There's a very minor consideration about performance (I say minor at this point since we have no reason to believe this to be performance critical) that you either cache the sorted list or regenerate each time.  Both have trade offs, since at this level (Book) you don't have the context of knowing how often the results will be used/requested on the same instance.  The former wastes some held space in the case of un-repeated requests, and the latter wastes time and space in the case of repeated requests.  I would defer to the caller to figure out what they want to do here rather than mixing in this concern into your Book class.

1

does it violate the principal of least astonishment to return a List of authors sorted by last name?

Any practical implementation will astonish you! scroll down to read more!!

Maybe its not impossible to write a getter that works 'as expected' but naive implementations quickly fall over. I'm going to stick with the (less confusing) C#.

public class Book
{
    private HashSet<string> authors;
    public Book(HashSet<string> authors)
    {
        this.authors = authors;
    }

    private List<string> cachedAuthors;

    public List<string> Authors
    {
        get
        {
            //avoid sorting every time we get the value!
            if(cachedAuthors == null)
            {
                cachedAuthors = this.authors.ToList();
                cachedAuthors.Sort();
            }
            return this.cachedAuthors;
        }
        set
        {
            //make sure our set remains in sync!
            this.authors = new HashSet<string>(value.Distinct());

            cachedAuthors = this.authors.ToList();
            cachedAuthors.Sort();
        }
    }
}

tests:

var auth = new HashSet<string>(new[] { "h", "j", "l" });

var b = new Book(auth);
Console.WriteLine(string.Join(",", b.Authors)); //h,j,i as expected

b.Authors.Add("a");
Console.WriteLine(string.Join(",", b.Authors)); //h,j,i,a  whhhaaaa?!?!

b.Authors = new List<string>() { "z", "y", "x" };
Console.WriteLine(string.Join(",", b.Authors)); //x,y,z as expected

b.Authors = new List<string>() { "a", "a", "a" };
Console.WriteLine(string.Join(",", b.Authors)); //a   whhaaa??!?!?!?!?!

b.Authors.Add("a");
b.Authors.Add("a");
Console.WriteLine(string.Join(",", b.Authors)); //a,a,a whhhaaaa??!!!!!

So what do we expect from authors? that it always be alphabetically sorted? that there not be duplicates? should we be able to alter it?

If you want these things simply changing to a list isn't for you.

In the comments you say you want a UI to display a sorted list of authors. But you don't need or want your model to do that sorting for you. It is a UI Concern. you can simply order it as desired in the UI layer

Console.WriteLine(string.Join(",", b.Authors.OrderBy(i=>i.LastName).ThenBy(i=>i.Firstname));
0

As Ewan already mentioned implicitly it is more a violation of SRP than of POLA.

You really want Authors to be a class. You have demands for them that simple or generic types do not support and you try to make up for that by throwing in different types in different scenarios.

Create a class Authors, feed an instance of it to the Book constructor and offer a property Book.Authors that returns an Authors object. If you want it to be read-only, return a copy, conveniently provided by Authors' copy constructor. You can sort the names immediately as the Authors object is constructed, you can have an Add(Author) method, a Set property that returns your set of authors, throw an exception or ignore when an already included Author is added, et cetera. Implement IEnumerable and IEquatable on Authors, overload == and !=. You get all the flexibility and future proofness you can wish for.

2
  • In my example, Author is a class, but I didn't post the details of it, because I didn't think it was necessary. Groovy doesn't have IEquatable or IEnumerable, but I understand what you're saying.
    – user313955
    Aug 29, 2018 at 19:09
  • @Sveta Yes, in your example Author is a class but you do not have a class for Authors. Aug 29, 2018 at 19:33

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.