paulsamuels / SBConstants

MIT License
311 stars 28 forks source link

Fix #28 - add xib support #29

Closed paulsamuels closed 9 years ago

paulsamuels commented 9 years ago

I'm not sure if this will cause any issue. I don't think it should be behind a feature switch as it seems like something that should just be default behaviour.

@tp does this resolve your issue?

tp commented 9 years ago

Thanks! The generated file looks good.

Just tried it with gem specific_install https://github.com/paulsamuels/SBConstants hotfix-28-add-xib-support, now I will switch some code to constants and give final feedback.

tp commented 9 years ago

Great, so this works :-)

But I always have to append .rawValue to the enum value. I guess that is a Swift issue though.. Wondering if there is a better way to represent the constants in code.

let cell = tableView.dequeueReusableCellWithIdentifier(TableViewCellreuseIdentifier.amenitiesListCell.rawValue) as! AmenitiesListCell
tonyarnold commented 9 years ago

@tp if the tool output structs with static properties instead of enums, the .rawValue would be unnecessary:

public enum SegueIdentifier : String {
    case PSBMasterToDetail = "PSBMasterToDetail"
    case PSBMasterToSettings = "PSBMasterToSettings"
}

var ident = SegueIdentifier.PSBMasterToDetail.rawValue

Becomes:

public struct SegueIdentifier {
    static var PSBMasterToDetail = "PSBMasterToDetail"
    static var PSBMasterToSettings = "PSBMasterToSettings"
}

var ident = SegueIdentifier.PSBMasterToDetail

You can probably just update your local templates for now if that's a more attractive option.

orta commented 9 years ago

@tonyarnold I thought about that when adding swift support: https://github.com/paulsamuels/SBConstants/pull/26

tonyarnold commented 9 years ago

Great to see the discussion has been had — is there some method of shortening out the .rawValue? It's ugly as hell and the common use case is to use these in comparisons with strings.

orta commented 9 years ago

Yes, see this quote from @ashfurrow

We use this abstraction when invoking segues and this one when comparing them to avoid calling .rawValue.

tonyarnold commented 9 years ago

Nice. Definitely worth writing those up somewhere.

paulsamuels commented 9 years ago

@orta seems as this is just a PR have you got any objections to me undoing that merge as it makes the history ugly. A rebase would prob work nicer

orta commented 9 years ago

Sure, I have no problems with that

orta commented 9 years ago

This looks good to me then :+1: - merge when you're ready