Question

Let's say you've got a domain model class for a Song. Song's have a tempo attribute (an int) that should always be a positive number. Should this requirement be part of the domain model or externally (such as in a SongManager / business logic layer class)?

Let's say you've chosen to implement it like so:

class Song {
  private int tempo;
  // ...

  public void setTempo(int tempo) {
    if (tempo > 0) {
      this.tempo = tempo;
    } else {
      // what?
    }

  }

}

Would you replace // what? above with:

  1. Nothing. Given a Song instance s, s.setTempo(-10) would simply not modify the object's state.
  2. Set tempo to some minimum value, eg 1.
  3. Mark setTempo with a checked throws InvalidTempoException and throw it in the else clause. That way, a controller or other component is responsible for catching the invalid tempo value and making a decision about how to handle the exception.
  4. Throw a runtime InvalidTempoException.
  5. Extract the tempo attribute into a Tempo class with the "must be greater than 0" encapsulated there.
  6. Something else.

I ask because I've been exploring the common "layered architecture" approach lately and in particular the domain layer.

Was it helpful?

Solution

If a Song is not valid with a zero or negative tempo value, this certainly should be an invariant modelled within Song.

You should throw an exception if an invalid value is attempted - this makes certain you don't have a Song in invalid state.

OTHER TIPS

Song's have a tempo attribute (an int) that should always be a positive number

If this is a requirement for the proper initialization of the Song then you should throw a corresponding exception e.g. IllegalArgumentException

Should this requirement be part of the domain model or externally (such as in a SongManager / business logic layer class)?

Domain related logic should live in domain model. Calling something as Manager often is a sign that you just don't know where to put code and how to name it properly.

Nothing. Given a Song instance s, s.setTempo(-10) would simply not modify the object's state.

In case you need trying to set tempo and if you have little control over argument that is going to be passed, ignoring failure to set tempo can be acceptable. It's like with UDP protocol - it's fine if it fails. Only I would name it accordingly - something like s.trySetTempo(-10).

Set tempo to some minimum value, eg 1.

That is like extension of first case. Only I can't think of any business case when there would be a need to default song tempo to near zero.

Mark setTempo with a checked throws InvalidTempoException and throw it in the else clause. That way, a controller or other component is responsible for catching the invalid tempo value and making a decision about how to handle the exception. Throw a runtime InvalidTempoException.

Throwing an exception means that anything that tries to set tempo is responsible for proper input because there should never be expected exceptions. Usually you would also have to write canSetTempo (or something like isTempoChangeable) indicator method so outside world could hit it before trying real deal.

Extract the tempo attribute into a Tempo class with the "must be greater than 0" encapsulated there.

This depends on shape of your domain. It might be a good idea if anything else (like SoundSample class, MidiClip class) needs to use concept of tempo. Also - it might be good to encapsulate tempo if it is almost unrelated to your business and just polluting more interesting logic in Song class.


I personally like when domain model provides an answer if action is available but action itself throws an error. That allows to make better UX too. Fields and buttons that are responsible for changing tempo would just not appear/be disabled - an immediate feedback that something can't be done just yet.

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top