0

I have an interface with only one title field:

protocol Artist {
    var title: String { get }
}

(A) Should I pass the whole object as I did here:

class Album {
    func setArtist(_ artist: Artist) {
        /// Handle
    }
}

album.setArtist(artist)

(B) Or set that only field:

class Album {
    func setArtistTitle(_ artistTitle: String) {
        /// Handle
    }
}

album.setArtistTitle(artist.title)

What is the recommended way of software design? The (A) solution looks more safer as it is restricted to the type. But the latter (B) is much more flexible. There is no need to create a concrete object, e.g if I want to unit-test my class. The code is less coupled, what we are striving for

EDIT:
All of possible duplicates consider cases with >1 parameters. For me those cases are quite obvious, since multiple parameters form a single type. In my example there is one and only one primitive parameter. The idea of the interface is guarantee existence of title, and that's it. The dilemma is where to access the title

  • 1
  • How is B more flexible? Can you give an example operation that can be performed with B, but not with A? – D Drmmr Feb 17 at 13:15
  • @DDrmmr Unit test example album.setArtistTitle("foo"); expect(album.title).equal("foo"). And similar scenarios where we want to pass a title without creating a concrete object. For example setting a title from an input field. It is still possible to implement with (A) but requires more complicated code and dummy classes like class ConcreteArtist: Artist { }. At the same time, we cannot put away the interface, it is needed to make objects compatible – kasyanov-ms Feb 17 at 13:57
3

Use A. As you say yourself it is safer and the intention of the code is clearer.

I think you may have a misunderstanding regarding coupling. An Album is supposed to be coupled to an Artist. Using a plain string rather than an Artist instance does not reduce this coupling, it just makes the code harder to understand and less safe, since you could by mistake pass something which is not the name of an artist. You can say it is more "flexible" because you can pass arbitrary stuff to setArtist, not just Artists, but that is not the kind of flexibility you want!

When talk about reducing coupling, we only want to reduce unnecessary coupling. like coupling between semantically unrelated objects, or coupling from one object to implementation details of another object rather than just its public interface.

  • How should we store the value inside an Album? As Artist or String? In the first case it seems we could violate the Law of Demeter: someField.text = album.artist.title – kasyanov-ms Feb 17 at 14:13
  • @kasyanov-ms: Store it as an Album type for the same reasons. – JacquesB Feb 17 at 14:24
  • The Law of Demeter is really a separate issue, but you should understand the purpose of the law. It is not just about counting dots. The text field in your example needs to know the title of the artist of the album in both cases, so replacing the class with a primitive does not change anything conceptually even though it is one dot less. – JacquesB Feb 17 at 14:35
  • Fair point. The last thing I'm confused about is a one-field implementation class ConcreteArtist: Artist { let title: String }, which is used when we're forced to pass a String into the method (not a rare scenario). In my view, is a stinkiest code smell – kasyanov-ms Feb 17 at 14:55
  • 1
    The Law of Demeter is about not hard coding long dot paths unless you've been promised that they will always work. Cobble long paths together randomly and you turn a flexible code base into an immobile ball of mud. – candied_orange Feb 17 at 15:01

Your Answer

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

Not the answer you're looking for? Browse other questions tagged or ask your own question.