Factory pattern for image or shape marker [closed]

Question

I'm making a photo marker application and need to make a factory pattern for marker. I think it is not very flexible and overall not good.

Would you check my code and suggest what could be improved?

import Foundation


enum MarkerType: String {
    case Shape, Image
}

enum MarkerError: ErrorType {
    case ImageNoExist
    case ShapeNoExist
}

struct ImageMarker {
    static func make(type: String) -> UIImageView {
        var imageStr: String = ""
        switch type {
        case "X":
            imageStr = "close.png"
            break
        default: break
        }

        let image = UIImage(named: imageStr)
        let tintedImage = image?.imageWithRenderingMode(.AlwaysTemplate)
        let imageView = UIImageView(image: tintedImage)
        imageView.tintColor = UIColor.redColor()
        return imageView
    }
}

struct ShapeMarker {
    static func make(type: String) -> UIView {
        var shape: UIView = UIView()
        switch type {
        case "CIRCLE":
            //shape = CircleView()
            break
        default: break
        }

        return shape
    }
}

typealias Factory = (String) -> AnyObject

class MarkerHelper {
    class func factoryFor(type: MarkerType) -> Factory {
        switch type {
        case .Shape:
            return ShapeMarker.make
        case .Image:
            return ImageMarker.make
        }
    }
}

Answer 1

In looking at your code, what I see are a series of effectively global functions that "make" things based on a string.

The object that calls factoryFor must know what the string is in order to know which type of maker to return (after all, if the string is going to be "X", then the object that calls factoryFor has to know not to pass Shape.) Also, the caller of the factory function must know what the factory is making in order to correctly cast the AnyObject that is returned.

This leads me to wonder what the MakerHelper.factoryFor function's responsibility is? I mean, it's job just seems to be to return the make global function that the caller already knows it wants, so why not just have the caller use the make function directly?

It seems to me that you could have written:

import UIKit

func tintedImageViewNamed(name: String) -> UIImageView {
    let image = UIImage(named: name)
    let tintedImage = image?.imageWithRenderingMode(.AlwaysTemplate)
    let imageView = UIImageView(image: tintedImage)
    imageView.tintColor = UIColor.redColor()
    return imageView
}

func make(type: String) -> UIView? /* thanks nhgrif */ {
    switch type {
    case "CIRCLE":
        //return CircleView()
        return UIView()
    case "X":
        return tintedImageViewNamed("close.png")
    default:
        return nil
    }
}

The above accomplishes the same thing your code does, is no less safe and no more testable than what you have and it's a lot less complex (which means less chance of bugs.)

I'd love to suggest a better solution, but to do that, I would have to better understand what problem you are trying to solve...