You have an public abstract class
called ICombinationAnalyzer
: that's wrong, only interfaces can use the I
prefix. Instead I'd expect this class to have Base
in its name at the start or the end (whatever is your preference).
This seems odd: user.Power = powerType + user.Type*100;
To me it would make more sense for the user to have a powerType
field, and for Power
to simply be a get
which returns powerType + Type*100;
.
I'm confused by all of UpdateHand(UsersProperties user, int combinationType, int powerType)
, to be honest: this method updates two properties of its parameter user
and then calls MainPoker.Win.Add(new Hand {Power = user.Power, Current = user.Type});
which doesn't look like it updates a Hand
, but instead adds a new one.
Why are combinationType
and powerType
int
s? Their names suggest they should be enum
s, but then you use them in a calculation...
This whole method throws up all kinds of warning signs to me and I can't shake the feeling you have some fundamental flaws in the design of your solution.
The RoyalFlush
also raises some questions: Hand.Combinations.RoyalFlush
is cast to an int
-- so why isn't it one in the first place? Same with Cards.CardTypes.Ace
and others in that namespace.
Why the need for Hand.Combinations.
and Cards.CardTypes.
in the first place? It seems odd to use subclasses for such fundamental values.
This whole block is just a mess:
t[0] == (int) Cards.CardTypes.Ace
&& t.Contains((int) Cards.CardTypes.Ten)
&& t.Contains((int) Cards.CardTypes.Jack)
&& t.Contains((int) Cards.CardTypes.Queen)
&& t.Contains((int) Cards.CardTypes.King)))
Even formatted like this I feel there's too much going on: the casts to int
don't help, plus there's the odd check that the first item is an Ace
while you use .Contains()
for the other types.
But Straight is even worse than RoyalFlush
and just is incomprehensible. What is op
? Why do you need to execute int[] op = Straight.Select(o => o/4).Distinct().ToArray();
each time -- it's not likely to change, now is it?
op[j] + 1 == op[j + 1] && op[j] + 2 == op[j + 2] && op[j] + 3 == op[j + 3] && op[j] + 4 == op[j + 4]
suggests that op
is sorted, but there's no evidence of this being the case.
This next bit looks a lot like a similar block in RoyalFlush
above and I'm wondering whether they shouldn't be the same.
if (op[j] != (int) Cards.CardTypes.Ace
|| op[j + 1] != (int) Cards.CardTypes.Ten
|| op[j + 2] != (int) Cards.CardTypes.Jack
|| op[j + 3] != (int) Cards.CardTypes.Queen
|| op[j + 4] != (int) Cards.CardTypes.King)
{
continue;
}
The "problem" you identify seems trivial. You can easily reduce it to this:
var listCombinations = new List<SwitchCombination>
{
new SwitchCombination(new HighCard(user, _reserve)),
new SwitchCombination(new PairFromTable(user,_reserve)),
new SwitchCombination(new PairInHand(user, _reserve)),
new SwitchCombination(new PairHandTable(user, _reserve)),
new SwitchCombination(new TwoPairFromTable(user, _reserve)),
new SwitchCombination(new TwoPairHandPairTablePair(user, _reserve)),
new SwitchCombination(new TwoPairHandTable(user, _reserve)),
new SwitchCombination(new TwoPairTwoDifferent(user, _reserve)),
new SwitchCombination(new ThreeOfAKind(user, _reserve)),
new SwitchCombination(new Combinations.Straight(user, _reserve)),
new SwitchCombination(new Combinations.Flush(user, _reserve)),
new SwitchCombination(new Combinations.FullHouse(user, _reserve)),
new SwitchCombination(new Combinations.FourOfAKind(user, _reserve)),
new SwitchCombination(new Combinations.StraightFlush(user, _reserve)),
new SwitchCombination(new Combinations.RoyalFlush(user, _reserve)),
}
I'd probably refactor that even more, but right now you only posted fragments of your code and it's too much work to fill in the gaps.