1
\$\begingroup\$

I have some elixir code I am using to score a ping pong game. Since I am new to functional programming I was hoping there might be a more elegant solution to this.

This part of the code is used in a GenServer to check if the next point could result in a win. So that means will the leading players next point meet the minimum score requirement (11 or 21 in ping pong) and also be 2 points in front of the other play.

Anyone got a better idea here?

def handle_call(:next_point_wins, _from, game) do
#game.score = [10,5]
nextPlayer1 = List.first(game.score) + 1
nextPlayer2 = List.last(game.score) + 1
scoreDifference =
 case nextPlayer1 > nextPlayer2 do
 true
 ->
 leader = nextPlayer1
 nextPlayer1 - nextPlayer2
 false
 ->
 leader = nextPlayer2
 nextPlayer2 - nextPlayer1
 end
cond do
 scoreDifference + 1 >= 2 and leader >= game.winningScore
 ->
 IO.puts "SOMEONE IS ABOUT TO WIN"
 {:reply, :ok, game}
 true 
 -> 
 IO.puts "NO-ONE IS ABOUT TO WIN"
 {:reply, :ok, game}
end
end
asked Oct 26, 2016 at 9:23
\$\endgroup\$
0

1 Answer 1

1
\$\begingroup\$

There's a few things you could do to improve this code:

Don't assign inside a case statement.

You assign a value to leader inside this case statement:

scoreDifference =
 case nextPlayer1 > nextPlayer2 do
 true
 ->
 leader = nextPlayer1
 nextPlayer1 - nextPlayer2
 false
 ->
 leader = nextPlayer2
 nextPlayer2 - nextPlayer1
 end

Support for this is deprecated in Elixir, and I assume will eventually be removed. The Elixir compiler will complain about this when it sees it, and you should listen to it. Generally you should find a different way to express this. One option is to return a tuple from your case:

{scoreDifference, leader} =
 case nextPlayer1 > nextPlayer2 do
 true -> 
 {nextPlayer1 - nextPlayer2, nextPlayer1}
 false -> 
 {nextPlayer2 - nextPlayer1, nextPlayer2}
 end

Use if when appropriate.

A cond statement that has one branch with conditions and one true branch is basically just an if statement. So it'd be simpler to just use an if:

if (scoreDifference + 1) >= 2 and leader >= game.winningScore do
 IO.puts "SOMEONE IS ABOUT TO WIN"
else
 IO.puts "NO-ONE IS ABOUT TO WIN"
end
{:reply, :ok, game}

(I've also moved the reply out of the if statement there, since it was the same across each. If you wanted to do different things you could move it back in easy enough).

Use Pattern Matching to destructure the scores.

Assuming you are only ever expecting 2 entries in your score list, you could use pattern matching to extract the two scores.

[nextPlayer1, nextPlayer2] = game.score

I'm not adding 1 to the scores here, because this isn't neccesary to calculate the difference correctly. (It is neccesary to add one to check if someone has a winning score, but I'll come to that).

Simplifying the logic

You can just subtract one score from the other and then call abs on the result to get a positive number for the score difference:

scoreDifference = abs(nextPlayer1 - nextPlayer2)

Then you can find the leader with Enum.max:

leader = Enum.max(game.score)

Putting it all together

If you take all these improvements you might end up with something like:

[nextPlayer1, nextPlayer2] = game.score
scoreDifference = abs(nextPlayer1 - nextPlayer2)
leaderScore = Enum.max(game.score)
if scoreDifference + 1 >= 2 and (leaderScore + 1) >= game.winningScore do
 IO.puts "SOMEONE IS ABOUT TO WIN"
else
 IO.puts "NO-ONE IS ABOUT TO WIN"
end
{:reply, :ok, game}
answered Nov 3, 2016 at 18:41
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.