【问题标题】:How do I get rid of all this duplicate code?我如何摆脱所有这些重复的代码?
【发布时间】:2023-03-31 01:40:01
【问题描述】:
    private Optional<Player> playerWithMostCards()
    {
        Player scoringPlayer = null;
        int maxCount = 0;

        for(Player p : players)
        {
            final int count = p.pile.size();

            if(count > maxCount)
            {
                scoringPlayer = p;
                maxCount = count;
            }
            else if(count == maxCount)
            {
                scoringPlayer = null;
            }
        }

        return Optional.ofNullable(scoringPlayer);
    }

    private Optional<Player> playerWithMostSevens()
    {
        Player scoringPlayer = null;
        int maxCount = 0;

        for(Player p : players)
        {
            int count = 0;

            for(Card c : p.pile)
            {
                if(c.is(Card.Value.SEVEN))
                {
                    count++;
                }
            }

            if(count > maxCount)
            {
                scoringPlayer = p;
                maxCount = count;
            }
            else if(count == maxCount)
            {
                scoringPlayer = null;
            }
        }

        return Optional.ofNullable(scoringPlayer);
    }

    private Optional<Player> playerWithMostSpades()
    {
        Player scoringPlayer = null;
        int maxCount = 0;

        for(Player p : players)
        {
            int count = 0;

            for(Card c : p.pile)
            {
                if(c.is(Card.Suit.SPADES))
                {
                    count++;
                }
            }

            if(count > maxCount)
            {
                scoringPlayer = p;
                maxCount = count;
            }
            else if(count == maxCount)
            {
                scoringPlayer = null;
            }
        }

        return Optional.ofNullable(scoringPlayer);
    }
    
    private Optional<Player> playerWithSevenOfSpades()
    {
        for(Player p : players)
        {
            for(Card c : p.pile)
            {
                if(c == new Card("7S"))
                {
                    return Optional.of(p);
                }
            }
        }

        return Optional.empty();
    }

    private void updateScores()
    {
        for(Player p : players)
        {
            p.score = p.scopas;
        }

        playerWithMostCards().ifPresent(p -> p.score += 1);
        playerWithMostSevens().ifPresent(p -> p.score += 1);
        playerWithMostSpades().ifPresent(p -> p.score += 1);
        playerWithSevenOfSpades().ifPresent(p -> p.score += 1);
    }

基本上,我正在制作一个纸牌游戏(称为 Scopa),当调用 updateScores() 时,应该更新每个玩家的分数。玩家可以通过拥有最多的牌、拥有最多的 7 或拥有黑桃 7 来获得积分。确定谁拥有最多的牌、7 和黑桃的逻辑非常相似。如何避免在这三种方法中重复逻辑?谢谢。

【问题讨论】:

    标签: java code-duplication


    【解决方案1】:

    对于最后两个,您可以创建一个函数。我不熟悉您使用的编程语言(我什至不知道它是哪种语言 - 请下次标记您的问题),但它看起来像这样:

    private Optional<Player> playerWithMostCards()
    {
        Player scoringPlayer = null;
        int maxCount = 0;
    
        for(Player p : players)
        {
            final int count = p.pile.size();
    
            if(count > maxCount)
            {
                scoringPlayer = p;
                maxCount = count;
            }
            else if(count == maxCount)
            {
                scoringPlayer = null;
            }
        }
    
        return Optional.of(scoringPlayer);
    }
    
    private Optional<Player> playerWithMost([type] cvalue) // I don't know what the type of Card.[Value/Suit].[SEVEN/SPADES] is.
    {
        Player scoringPlayer = null;
        int maxCount = 0;
    
        for(Player p : players)
        {
            int count = 0;
    
            for(Card c : p.pile)
            {
                if(c.is(cvalue))
                {
                    count++;
                }
            }
    
            if(count > maxCount)
            {
                scoringPlayer = p;
                maxCount = count;
            }
            else if(count == maxCount)
            {
                scoringPlayer = null;
            }
        }
    
        return Optional.of(scoringPlayer);
    }
    private Optional<Player> playerWithSevenOfSpades()
    {
        for(Player p : players)
        {
            for(Card c : p.pile)
            {
                if(c == new Card("7S"))
                {
                    return Optional.of(p);
                }
            }
        }
    
        return Optional.empty();
    }
    
    private void updateScores()
    {
        for(Player p : players)
        {
            p.score = p.scopas;
        }
    
        playerWithMostCards().ifPresent(p -> p.score += 1);
        playerWithMost(Card.Value.SEVEN).ifPresent(p -> p.score += 1);
        playerWithMost(Card.Suit.SPADES).ifPresent(p -> p.score += 1);
        playerWithSevenOfSpades().ifPresent(p -> p.score += 1);
    }
    

    我不确定如何将顶部的集成到函数中。

    【讨论】:

    • 感谢您的回复,这是 Java 顺便说一句。我应该记得标记我的问题。无论如何,您的解决方案不是很适用。我不能以这种方式组合这两种方法,因为参数的类型不一样。一个是 Card.Suit,另一个是 Card.Value。这些是枚举。
    • 哦,好的。抱歉,我帮不上忙。
    【解决方案2】:

    另一种解决方案是只使用一个函数来计算分数。您在玩家列表中循环 4 次,并且在卡片中循环 3 次。你只需要一个循环给玩家,一个循环让卡片检查一切。

    类似这样的:

    private void updateScores()
        {
            Player playerWithMostCards = null;
            Player playerWithMostSevens = null;
            Player playerWithMostSpades = null;
            int mostCards = 0;
            int mostSevens = 0;
            int mostSpades = 0;
    
            for(Player p : players)
            {
                // Most cards
                final int playerPileSize = p.pile.size();            
    
                if(playerPileSize > mostCards)
                {
                  playerWithMostCards = p;
                  mostCards = playerPileSize;
                }
                else if(playerPileSize == mostCards)
                {
                  playerWithMostCards = null;
                }
    
                // NumberOfSevens and Spades
                int numberOfSevens = 0
                int numberOfSpades = 0
    
                for(Card c : p.pile)
                {
                    if(c.is(Card.Value.SEVEN))
                    {
                      numberOfSevens++;
                    }
    
                    if(c.is(Card.Suit.SPADES))
                    {
                        numberOfSpades++;
                    }
                }
    
                if (numberOfSevens > maxSevens) {
                  playerWithMostSevens = p
                  mostSevens = numberOfSevens              
                } else if (numberOfSevens == maxSevens) {
                  playerWithMostSevens = null
                }
    
                 if (numberOfSpades > maxSpades) {
                  playerWithMostSpades = p
                  mostSevens = numberOfSpades              
                } else if (numberOfSpades == maxSpades) {
                  playerWithMostSpades = null
                }
            }
    
            playerWithMostCards.score += 1;
            playerWithMostSevens.score += 1;
            playerWithMostSpades.score += 1;      
    
        }
    

    我没有对此进行测试,但这表明了粗略的想法。你可以通过添加最后一个函数来练习。

    【讨论】:

    • 感谢您的回复!我知道我在播放器列表上循环了很多次。不过,我正在尝试在某种程度上将我的代码模块化,并使方法尽可能短。您在此处编码的方法做了很多事情,并且扩展性不是很好。
    • 是的,这是一个有效的观点,在这种情况下,我真的很喜欢你的代码。它非常明确,每个功能都很清楚。根据上下文创建具有很大差异的“全能”函数,例如 mostCards 或 mostSevens 当您需要重构并且您必须考虑多件事情而不是一件简洁的事情时,可能会导致更多问题(例如只有大多数卡片)。
    【解决方案3】:

    我不喜欢我想出的方法的名称,但这是我一段时间后得出的。它大大减少了我拥有的代码量,并且具有同样多的灵活性,如果不是更多的话。如果您能想出一个更好的“playerBestMatching”名称,请告诉我,我可以编辑它。

    编辑:“playerBestMatching”现在是“playerWithMostCardsMatching”

        private Optional<Player> playerWithMostCardsMatching(
            final Predicate<Card> predicate)
        {
            Player scoringPlayer = null;
            int maxCount = 0;
    
            for(Player p : players)
            {
                final int count = (int)p.pile.stream()
                                             .filter(predicate)
                                             .count();
    
                if(count > maxCount)
                {
                    scoringPlayer = p;
                    maxCount = count;
                }
                else if(count == maxCount)
                {
                    scoringPlayer = null;
                }
            }
    
            return Optional.ofNullable(scoringPlayer);
        }
    
        private void updateScores()
        {
            for(Player p : players)
            {
                p.score = p.scopas;
            }
    
            playerWithMostCardsMatching(c -> true).ifPresent(p -> p.score += 1);
            playerWithMostCardsMatching(c -> c.is(Card.Value.SEVEN)).ifPresent(p -> p.score += 1);
            playerWithMostCardsMatching(c -> c.is(Card.Suit.SPADES)).ifPresent(p -> p.score += 1);
            playerWithMostCardsMatching(c -> c == new Card("7S")).ifPresent(p -> p.score += 1);
        }
    

    【讨论】:

    • 欢迎使用函数式编程 :) 意识到您可以将行为作为参数传递将改变您看待问题的方式。
    • 名称建议:playerWithMostCardsMatching.
    • @JohannesKuhn 真是个好名字,我也想到了。我只是有点厌烦它有多长。无论如何我都会用它来编辑我的解决方案:-)
    猜你喜欢
    • 2020-03-25
    • 2018-03-04
    • 1970-01-01
    • 2010-11-25
    • 2015-06-22
    • 1970-01-01
    • 2012-10-08
    • 1970-01-01
    • 1970-01-01
    相关资源
    最近更新 更多