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
1 Answer 1
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}