HiveBrain v1.2.0
Get Started
← Back to all entries
patternswiftMinor

Message factory

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
factorymessagestackoverflow

Problem

Today I thought about Factory Design Pattern in Swift, and I do an implementation with Extension.

My post is about if anyone have any observation or issue about this implementation violate any convention about Factories? Can I do this way?

I did it because I thought the statics function in my object not smell good, and for who will do maintenance it will confuse, and because this I split to Extension.

let kToastMessageTurnType:String = "kToastMessageTurnType";
    let kWhoStartTheGameMessageType:String = "kWhoStartTheGameMessageType";

    class ToastMessage {

        private var gameState:GameState;

        private(set) var message:String = "";
        private(set) var isFoul:Bool = false;

        init(gameState:GameState, type:String, isFoul:Bool)
        {
            self.gameState = gameState;
            self.isFoul = isFoul;

            self.message = processMessage(type)
        }

        private func processMessage(type:String)->String
        {
            let textLocalized:TextLocalized;

            switch type
            {
            case GSFoulTypeBallEightPocketedInBreakShot:
                textLocalized = TextStartedIncorrectly(playersInfo: self.gameState);
            case GSFoulTypeBreakAtLeastFourBumpers:
                textLocalized = TextStartedIncorrectly(playersInfo: self.gameState);
             case kToastMessageTurnType:
                textLocalized = TextTurnMessage(playersInfo: self.gameState);
             case kWhoStartTheGameMessageType:
                textLocalized = TextWhoStartMessage(playersInfo: self.gameState);   
            default:
                return "";
            }

            return textLocalized.text
        }
    }


```
//Factory from ToastMessage
extension ToastMessage
{
class func getFoulToastMessage(type:String)->ToastMessage
{
return ToastMessage(gameState: GameState.shared, type: type, isFoul: true);
}

class func getNotifyTurnMessagetMes

Solution

I can't recommend these string constants at all. We should be using an enum. Even if your enum is still string backed (which it doesn't appear that that'd be necessary), it's still going to be better overall to use an enum.

As a start, the enum, means that users can't simply pass any random value in. The values they can select from are more clearly defined. Auto-complete will give them a hint at their choices.

It's also more efficient. Despite the enum being back by a string, the actual value you're passing around isn't a string at all. The string is only ever accessed if you call RawValue on the enum value. This means all your enum comparisons (and switch statements) are all more efficient.

But the most important thing about using an enum here is that it might completely eliminate the need for your factory methods extension.

GameState is apparently a singleton accessed via the .shared property. You haven't provided enough context for me to take a stab at the idea of this being a singleton (maybe/probably unnecessary), but if it is a singleton, why does it need to be passed around, rather than just accessing it directly where ever it's needed? That sort of defeats the point of a singleton.

Likewise, we're not provided enough information about isFoul: and what that's supposed to mean.

But the point is GameState can definitely be eliminated from the initializer. And if we turn the type from a String to an enum with limited options, then the factory extension just seems completely unnecessary, because the init method is simple enough.

Context

StackExchange Code Review Q#87006, answer score: 8

Revisions (0)

No revisions yet.