javakotlinlambdafunctional-programmingjava-stream

How to refactor a nested switch-case (Java) or when (Kotlin)?


Requirement: I need code a Jo-Ken-Pô game which can be played with multiple players. There are 5 roles (Spock, scissors, paper, stone and lizard). I can accomplish it with these two chained when/switches (the code bellow uses when() because it is in Kotlin but same idea can be applied using switch in Java):

  when (u1.play) {
        PlayType.SPOCK -> when (u2.play) {
            //SPOCK WINS
            PlayType.TESOURA -> return u1
            PlayType.PEDRA -> return u1
            //SPOCK LOSES
            PlayType.PAPEL -> return u2
            PlayType.LAGARTO -> return u2
        }
        PlayType.TESOURA -> when (u2.play) {
            //TESOURA (scissors) WINS
            PlayType.PAPEL -> return u1
            PlayType.LAGARTO -> return u1
            //TESOURA (scissors) LOSES
            PlayType.SPOCK -> return u2
            PlayType.PEDRA -> return u2
        }
        PlayType.PAPEL -> when (u2.play) {
            //PAPEL (paper) WINS
            PlayType.SPOCK -> return u1
            PlayType.PEDRA -> return u1
            //PAPEL (paper) LOSES
            PlayType.TESOURA -> return u2
            PlayType.LAGARTO -> return u2
        }
        PlayType.PEDRA -> when (u2.play) {
            //PEDRA (stone) WINS
            PlayType.LAGARTO -> return u1
            PlayType.TESOURA -> return u1
            //PEDRA (stone) LOSES
            PlayType.SPOCK -> return u2
            PlayType.PAPEL -> return u2
        }
        PlayType.LAGARTO -> when (u2.play) {
            //LAGARTO (lizard) WINS
            PlayType.SPOCK -> return u1
            PlayType.PAPEL -> return u1
            //LAGARTO (lizard) LOSES
            PlayType.TESOURA -> return u2
            PlayType.PEDRA -> return u2
        }
    }

I have read hours and hours trying to find how to make this code more elegant using lambda but I can't find any clue.

I will paste the whole code here. Although you see I am using lambda at least to call the method I am certainly missing some powerful feature of lambda and almost coding in a pre-Java 8 classical way.

All users come from a H2 database. Here is the repository:

import com.mycomp.jokenpo.model.User
import org.springframework.data.repository.CrudRepository

interface UserRepository : CrudRepository<User, Long>

User model:

import com.mycomp.jokenpo.enums.PlayType
import javax.persistence.*


@Entity
data class User(
        @Id
        @GeneratedValue(strategy = GenerationType.IDENTITY)
        val id: Long,

        @Column(nullable = false)
        val name: String,

        @Enumerated
        @Column(nullable = false)
        val play: PlayType

)

Enum PlayType

enum class PlayType(val value: Int) {
    SPOCK(1), TESOURA(2), LAGARTO(3), PAPEL(4), PEDRA(5)
}

The service *** HERE ARE THE ISSUES ***

import com.mycomp.jokenpo.enums.PlayType
import com.mycomp.jokenpo.model.User
import com.mycomp.jokenpo.respository.UserRepository
import org.springframework.stereotype.Component

@Component
class GameService(private val userRepository: UserRepository) {

    fun returnWinnerBetweenTwoPlayers(u1: User, u2: User): User {

        when (u1.play) {
            PlayType.SPOCK -> when (u2.play) {
                //SPOCK WINS
                PlayType.TESOURA -> return u1
                PlayType.PEDRA -> return u1
                //SPOCK LOSES
                PlayType.PAPEL -> return u2
                PlayType.LAGARTO -> return u2
            }
            PlayType.TESOURA -> when (u2.play) {
                //TESOURA (scissors) WINS
                PlayType.PAPEL -> return u1
                PlayType.LAGARTO -> return u1
                //TESOURA (scissors) LOSES
                PlayType.SPOCK -> return u2
                PlayType.PEDRA -> return u2
            }
            PlayType.PAPEL -> when (u2.play) {
                //PAPEL (paper) WINS
                PlayType.SPOCK -> return u1
                PlayType.PEDRA -> return u1
                //PAPEL (paper) LOSES
                PlayType.TESOURA -> return u2
                PlayType.LAGARTO -> return u2
            }
            PlayType.PEDRA -> when (u2.play) {
                //PEDRA (stone) WINS
                PlayType.LAGARTO -> return u1
                PlayType.TESOURA -> return u1
                //PEDRA (stone) LOSES
                PlayType.SPOCK -> return u2
                PlayType.PAPEL -> return u2
            }
            PlayType.LAGARTO -> when (u2.play) {
                //LAGARTO (lizard) WINS
                PlayType.SPOCK -> return u1
                PlayType.PAPEL -> return u1
                //LAGARTO (lizard) LOSES
                PlayType.TESOURA -> return u2
                PlayType.PEDRA -> return u2
            }
        }
        return u1
    }

    fun playGameWithAll(): User? {
        val allUsers = userRepository.findAll().toList()

        val winner = allUsers.reduce { a, b ->
            returnWinnerBetweenTwoPlayers(a, b)
        }

        if (allUsers.filter { player -> player.play == winner.play }
                        .count() == 1)
            return winner
        else
            return null

    }
}

The code above works as expected but I have the feeling I am coding badly for two reasons:

  1. I split two small lambda statements: reduce to compare each other play type and just to figure if there is more than one winner I coded .count separated.
  2. Certainly there might be a more elegant and readable way to code two chained switches with lambda but I can get even the first step to try.

P.S.: the code is Kotlin but if you suggest something in Java I can easily translate it to Kotlin. Any trick or suggestion on how refactor will be highly appreciated.

Anyone interested to get the game feel free to clone from https://github.com/jimisdrpc/games.git.

Edited after Mikhail's answer

Main.java

    package poc;
    
    import java.util.List;
    import java.util.Map;
    import java.util.Optional;
    import java.util.Set;
    
    public class Main {
    
        public static void main(String[] args) {
    
            //Fake returned list from database
            List<User> usersList = List.of(
                    new User(1L, "Jimis", PlayType.LAGARTO),
                    new User(2L, "Drpc", PlayType.PAPEL));
            
            
            //User winnerUser = returnWinnerBetweenTwoPlayers(usersList.get(0), usersList.get(1));
            
            Optional<User> winnerUser  = usersList.stream().reduce( (a, b) ->
            returnWinnerBetweenTwoPlayers(a , b));
            
            System.out.print(winnerUser);
    
        }
    
        //Trying to refactoring from classical switch to some structure for using with lambda
        private final static Map<PlayType, Set<PlayType>> CONFIG = Map.of(
                PlayType.SPOCK,
                    Set.of(PlayType.TESOURA, PlayType.PEDRA), 
                PlayType.TESOURA, 
                    Set.of(PlayType.PAPEL, PlayType.LAGARTO)
    
        );
    
        private static User returnWinnerBetweenTwoPlayers(User u1, User u2) {
/// ****** Exception next line
            if (CONFIG.get(u1.getPlay()).contains(u2.getPlay())) {
                return u1;
            }
            return u2;
        }
    }

User model

package poc;

public class User {

    Long id;
    String name;
    PlayType play;

    public User(Long id, String name, PlayType play) {
        super();
        this.id = id;
        this.name = name;
        this.play = play;
    }

//... getters/setters removed
}

PlayType enum

package poc;

public enum PlayType {
    SPOCK,
    TESOURA, 
    PEDRA, 
    PAPEL,
    LAGARTO;
}

Exception when running this part: CONFIG.get(u1.getPlay()).contains(u2.getPlay()):

Exception in thread "main" java.lang.NullPointerException
    at moduleinfo/poc.Main.returnWinnerBetweenTwoPlayers(Main.java:39)
    at moduleinfo/poc.Main.lambda$0(Main.java:21)
    at java.base/java.util.stream.ReduceOps$2ReducingSink.accept(ReduceOps.java:123)
    at java.base/java.util.AbstractList$RandomAccessSpliterator.forEachRemaining(AbstractList.java:720)
    at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:484)
    at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:474)
    at java.base/java.util.stream.ReduceOps$ReduceOp.evaluateSequential(ReduceOps.java:913)
    at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
    at java.base/java.util.stream.ReferencePipeline.reduce(ReferencePipeline.java:558)
    at moduleinfo/poc.Main.main(Main.java:20)

If I try simplify and call without stream().reduce() I get this issue at the same point:

Exception in thread "main" java.lang.NullPointerException
    at moduleinfo/poc.Main.returnWinnerBetweenTwoPlayers(Main.java:49)
    at moduleinfo/poc.Main.main(Main.java:19)

Final solution

public class Main {

    public static void main(String[] args) {

        //Fake returned list from database
        List<User> usersList = List.of(
                new User(1L, "Jogador 1", PlayType.PEDRA),
                new User(2L, "Jogador 2", PlayType.TESOURA),
                new User(3L, "Jogador 3", PlayType.TESOURA),
                new User(4L, "Jogador 4", PlayType.SPOCK)
                );
        
        
        Optional<User> winnerUser  = usersList.stream().reduce( (a, b) ->
            returnWinnerBetweenTwoPlayers(a , b));
        
        System.out.print(winnerUser.get().getName());

    }

    private final static Map<PlayType, Set<PlayType>> CONFIG = Map.of(
            PlayType.SPOCK,
                Set.of(PlayType.TESOURA, PlayType.PEDRA), 
            PlayType.TESOURA, 
                Set.of(PlayType.PAPEL, PlayType.LAGARTO),
            PlayType.PAPEL, 
                Set.of(PlayType.SPOCK, PlayType.PEDRA),
            PlayType.PEDRA, 
                Set.of(PlayType.LAGARTO, PlayType.TESOURA),
            PlayType.LAGARTO, 
                Set.of(PlayType.SPOCK, PlayType.PAPEL)
    );

    private static User returnWinnerBetweenTwoPlayers(User u1, User u2) {
        if (CONFIG.get(u1.getPlay()).contains(u2.getPlay())) {
            return u1;
        }
        return u2;
    }
}

Solution

  • This switch inside switch construction looks really hard to read, you're right.

    Think of it this way: each PlayType wins from some others. And this information look like a configuration, so can be described in a declarative way, like this:

    So you can just define a Map<PlayType, Set<PlayType>> and verify if u2.play is contained by map.get(u1.play)

    UPD. Code example (java, written in notepad, so may contain some syntax mistakes)

    class GameService {
      private final static Map<PlayType, Set<PlayType>> CONFIG = Map.of(
        PlayType.SPOCK, Set.of(PlayType.TESOURA, PlayType.PEDRA),
        PlayType.TESOURA, Set.of(PlayType.PAPEL, PlayType.LAGARTO)
        //etc
      );
    
      function User returnWinnerBetweenTwoPlayers(User u1, User u2){
        if (CONFIG.get(u1.getType()).contains(u2.getType()){
          return u1;
        }
        return u2;
      } 
    }