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);
}
Basically, I'm making a card game (called Scopa), and when updateScores() is called, the scores of every player should be updated. Players can earn a point by having the most cards, having the most sevens, or having the seven of spades. The logic for determining who has the most cards, sevens, and spades is very similar. How can I avoid repeating the logic across these three methods? Thank you.
I don't like the name of the method I came up with, but here is what I arrived at after some time. It heavily reduces the amount of code that I had and has just as much flexibility, if not more. Let me know if you can think of a better name for "playerBestMatching", and I can edit this.
Edit: "playerBestMatching" is now "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);
}