gistfile1.swift

import Foundation

/*
  Lets suppose you have a class called "Action" which has to represent an user action in a drawing app. There are a fixed number of different actions: Delete, Add, Modify. Some of those action types have different properties. For example: 

    If the user adds something, the add action contains "what" the user has added and the "content" of the new object.

    If the user modifies something, the modify action contains "what" has been edited, the "previousContent" and the "newContent".

  So there are basically at least three different approaches how you can model that. Which one is the best?
*/


//======= OPTION 1: A single action class ========

// Have a single class that contains the union of the different properties needed by the different action types. The properties which are found in every action type (only "what" is specified as non-optional whereas every other property has to be an optional type.
class Action1 {
  enum Type {
    case Delete
    case Add
    case Modify
  }
  init(type: Type, what: String, previousContent: String?, newContent: String?) {
    self.type = type
    self.what = what
    self.previousContent = previousContent
    self.newContent = newContent
  }
  var type: Type
  var what: String
  var previousContent: String?
  var newContent: String?
}

// == Usage example:
func doSomethingWith1(action1: Action1) {
  switch action1.type {
    case .Delete: println("delete")
    case .Add: println("add")
    case .Modify: println("modify")
    if let previousContent = action1.previousContent {
      // bla bla
      }
  }
}

// Drawbacks: If you want to access a specific attribute you have to use if let.


//======= OPTION 2: Base class + sub classes ========

// Have a base class with the common stuff (non-optionals)
// and subclasses with the action-type-specific stuff.
// This avoids optionals.

class Action2 {
  var what: String
  init(what: String) { self.what = what }
}

class DeleteAction : Action2 { }

class AddAction : Action2 {
  var content: String
  init(what: String, content: String) {
    self.content = content
    super.init(what: what)
  }
}

class ModifyAction : Action2 {
  var previousContent: String
  var newContent: String
  init(what: String, previousContent: String, newContent: String) {
    self.previousContent = previousContent
    self.newContent = newContent
    super.init(what: what)
  }
}

// == Usage example:
func doSomethingWith2(action2: Action2) {
  if let action = action2 as? DeleteAction {
    println("delete")
  }
  if let action = action2 as? AddAction {
    println("add")
  }
  if let action = action2 as? ModifyAction {
    println("modify")
  }
}

// Remark: Avoids optionals, needs casting every time you work with an "abstract" action, you are not forced to handle every action type (you are not forced to cast to every available action class)


//======= OPTION 2: Enums ========

// Use an enum with associated description objects. The description objects contain the information needed.

enum Action3 {
  case Delete(DeleteActionDescription)
  case Add(AddActionDescription)
  case Modify(ModifyActionDescription)
}
class ActionDescription {
  var what: String
  init(what: String) { self.what = what }
}
class DeleteActionDescription : ActionDescription {}
class AddActionDescription : ActionDescription {
  var content: String
  init(what: String, content: String) {
    self.content = content
    super.init(what: what)
  }
}
class ModifyActionDescription : ActionDescription {
  var previousContent: String
  var newContent: String
  init(what: String, previousContent: String, newContent: String) {
    self.previousContent = previousContent
    self.newContent = newContent
    super.init(what: what)
  }
}

// == Usage example:
func doSomethingWith3(action3: Action3) {
  switch action3 {
  case .Delete(let descr): println("delete")
  case .Add(let descr): println("add")
  case .Modify(let descr): println("modify")
  }
}

// Remarks: Avoids optionals, without additional work you have to use switch all the time, no casting required

// Which approach is best?

Option 1

Pretty bad because you have the properties for all types associated with each type even if they are optional. As you've already written you'd have to use if let for accessing those properties all the time.

Option 2

In my opinion this is the cleanest OOP approach - except for the casting part (as?). In this particular case I would use the visitor pattern. Here is how it might look like.

protocol ActionVisitor {
    func visit(deleteAction: DeleteAction)
    func visit(addAction: AddAction)
    //...
}

class Action {
    func acceptVisitor(visitor: ActionVisitor) {
        fatalError("implement in subclass") // because Swift doesn't support abstract methods... (yet?)
    }
}

class DeleteAction: Action {
    override func acceptVisitor(visitor: ActionVisitor) {
        visitor.visit(self)
    }
}

class DoSomethingVisitor: ActionVisitor {
    func visit(deleteAction: DeleteAction) {
        println("delete")
    }
    func visit(addAction: AddAction) {
        println("add")
    }
    // ...
}

The extensibility of this approach is a lot better than of the first one. By using the visitor pattern you are forced to explicitly handle all classes which are visited by the visitor.

Option 3

I think this is a nice and pragmatic solution to this problem. You have clear separation of the actions and their properties and you can extend the enumeration with new cases anytime.

Summary

Depending on the actual use case (especially depending on the responsibility of the actions) I would go either with the second option changed to use the visitor pattern or with the third option.

What I do not like about the Visitor-Pattern:

1. For every situation where I have to "visit" an action I have to write a new class that implements ActionVisitor.
2. Those subclasses always have to implement all methods: visit(Delete;Add;Modify). This may be a good thing but it adds a lot of "clutter".
3. My Subclasses are not able to capture their environment: Thus I have to write custom initializers for those classes becuase most of the time they will have to use some other data to do their job.

What do you think about the thing from below? It is basically the enum-stuff with a couple of additional functions/variables that take a closure. Usage example at the end.

//======= OPTION 2: Enums ========

// Use an enum with associated description objects. The description objects contain the information needed.

public enum Action3 {
  case Delete(DeleteActionDescription)
  case Add(AddActionDescription)
  case Modify(ModifyActionDescription)
  init() {
    self = Action3.Delete(DeleteActionDescription(what: "hello"))
  }
  public func consumeDelete(consumer: (description: DeleteActionDescription)->Void) {
    if let deleteDescription = deleteDescription { consumer(description: deleteDescription) }
  }
  public var deleteDescription: DeleteActionDescription? {
    switch self {
      case .Delete(let description): return description
      default: return nil
    }
  }
  public func consumeModify(consumer: (description: ModifyActionDescription)->Void) {
    if let modifyDescription = modifyDescription { consumer(description: modifyDescription) }
  }
  public var modifyDescription: ModifyActionDescription? {
    switch self {
    case .Modify(let description): return description
    default: return nil
    }
  }
  public func consumeAdd(consumer: (description: AddActionDescription)->Void) {
    if let addDescription = addDescription { consumer(description: addDescription) }
  }
  public var addDescription: AddActionDescription? {
    switch self {
    case .Add(let description): return description
    default: return nil
    }
  }
}
public class ActionDescription {
  var what: String
  init(what: String) { self.what = what }
}
public class DeleteActionDescription : ActionDescription {}
public class AddActionDescription : ActionDescription {
  var content: String
  init(what: String, content: String) {
    self.content = content
    super.init(what: what)
  }
}
public class ModifyActionDescription : ActionDescription {
  var previousContent: String
  var newContent: String
  init(what: String, previousContent: String, newContent: String) {
    self.previousContent = previousContent
    self.newContent = newContent
    super.init(what: what)
  }
}

// == Usage example:
func doSomethingWith3(action3: Action3) {

  action3.consumeDelete { description in
    println("delete")
  }
  action3.consumeAdd { description in
    println("add")
  }
  action3.consumeModify { description in
    println("modify")
  }

  // opposed to:
  switch action3 {
  case .Delete(let descr): println("delete")
  case .Add(let descr): println("add")
  case .Modify(let descr): println("modify")
  }
}

Kind of a lot of code just to get rid of the if let statements. For me this is way harder to read that the first option.

Yeah. The first option is the most pragmatic solution. That is certain. And you could see optionals as a "feature" and not something that has to be worked around. But I really dislike if let because it makes early return so hard.

Visitor Pattern

I do not like about the Visitor-Pattern

As I said, it really depends on the use case. I can understand that the implementation of the visitor pattern might be an overkill for what you are trying to achieve.

are not able to capture their environment: Thus I have to write custom initializers for those classes

Which is imho perfectly fine.

Option 4

I don't like it for the same reason I don't like the first option. All properties for individual action types (addDescription, deleteDescription and modifyDescription) are associated with each action as they can be accessed via the interface. And the fact that you can call consumeModify() and access deleteDescription on an action of type .Add is imho not well designed code.

Finding the Optimal Solution

I think the most important question to ask is whether you know the type of the action when you are trying to do something with it?

If YES, than I would propose using the class hierarchy from the second option. An example would be a view which would inform its delegate about performed actions. Instead of having one delegate method for all the action types, you can introduce a delegate method for each action type.

protocol ViewDelegate {

    // instead of this
    func view(view: View, didPerformAction action: Action)

    // do this
    func view(view: View, didPerformAddAction action: AddAction)
    func view(view: View, didPerformMoveAction action: MoveAction)

}

If NO, than your fourth option would not work either. In this case I'd still prefer the third option and using the switch/case mechanism for consuming the actions.