Remove repeated code possible, is there a more elegant solution?

Question

I'm new to Swift...

I loop through an hand of cards (not shown below), and compare each card with a previously determined best option. If it has a better value, replace the struct variable "bestOption" with this card and the appropriate value (penaltyValue, which is arbitrary, can be -5, -1, 9, 199, doesn't matter for this question).

So for each card in my hand I call "compareReplaceBestOption" with that card.

The bestOption variable, a struct, is initially empty (NONE value) and will therefor be filled with the very first card and value. For each following card I compare (and possibly replace) the card with the value in the bestOption variable (which will be of SOME value).

This code is ugly for probably several reasons (all advice is appreciated), but what really strikes ME is the double assignment to variable bestOption (a struct).

Question: is there a more elegant solution so there is only ONE assignment (I could create a small function that does the assignment, but then I end up with conceptually the same issue)

Having this statement twice hurts...

 bestOption = ( card , newPenaltyValue )

(side information: g.cardStack[0] is an in a previous turn played card which is face up on the stack. The handcards are compared to this card)

 //Find penalty and card for card in hand with lowest possible penalty //AND without using the bonusChip var bestOption: (card: Card!, penaltyValue: Int!)? func compareReplaceBestOption(card: Card) { let newPenaltyValue = card.nr - g.cardStack[0].nr - 1 if bestOption == nil { bestOption = ( card , newPenaltyValue ) } else { if ( newPenaltyValue < bestOption!.penaltyValue ) { bestOption = ( card , newPenaltyValue ) } } }

You could flip the if around and use if let instead which would make your code safer becasue then you won't need to force unwrap bestOption. The only way I can see to remove one of the assignments is to make bestOption non-optional, and initialize it with a value that is guaranteed to be the smallest and get replaced. — David Skrundz
– David Skrundz, Commented Jul 14, 2015 at 14:27

Blake Merryman · Accepted Answer · 2017-01-11 00:20:12Z

Updated for Swift 3.0:

if let best = bestOption, best.penaltyValue > newPenaltyValue { best = (car, newPenaltyValue) }

(Really just losing the where clause.)

Old Updated Answer:

After revisiting this answer, I think we can improve to make this more Swifty:
if let best = bestOption where best.penaltyValue > newPenaltyValue { best = (car, newPenaltyValue) } 
We use the optional unwrapping feature and the where-clause to provide some addition cases to check against (even using the now-unwrapped best). Much easier to read and we drop that pesky !.

Original Answer:

How about:

if bestOption == nil || newPenaltyValue < bestOption!.penaltyValue { bestOption = (car, newPenaltyValue) }

Happens to the best of us. The elegant solution is always the simplest.

Collectives™ on Stack Overflow

Remove repeated code possible, is there a more elegant solution?

1 Answer 1

1 Comment

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

1 Comment

Related