patterncsharpMinor
WinForms Poker (complete project)
Viewed 0 times
pokercompleteprojectwinforms
Problem
I have quite huge question/favor to ask but I hope I can get some useful answers. This is my first complete project, I'm still learning C#, my first programming language.
I'm afraid that there will be a lot of things that people won't probably understand but you can also check some of my other questions they include fairly enough information about some of my code.
The main thing about the project is obviously the way I determine the play hand. Here's a link to a little bit more detailed explanation of why I'm using modulus and divide operator to check the current card type/suit : Defining what combination the user have in Poker
How I keep things up-to date: https://codereview.stackexchange.com/questions/121942/update-method-using-timer-in-winforms-poker-app
How I actually check if the player has a specific combination: Strategy pattern using an abstract class and an interface
There are a few more question in my user profile which you can try using to get a better understanding of what's going on. Any improvements are welcomed as well as any suggestion that I can implement.
```
using System;
using System.Collections.Generic;
using System.Drawing;
using System.IO;
using System.Linq;
using System.Threading.Tasks;
using System.Windows.Forms;
using Poker.Achievements;
using Poker.Combinations;
using Poker.Users;
using Flush = Poker.Achievements.Flush;
using FourOfAKind = Poker.Achievements.FourOfAKind;
using FullHouse = Poker.Achievements.FullHouse;
using RoyalFlush = Poker.Achievements.RoyalFlush;
using Straight = Poker.Achievements.Straight;
using StraightFlush = Poker.Achievements.StraightFlush;
namespace Poker
{
public partial class MainPoker : Form
{
public enum TableCards
{
FirstCard = 12,
SecondCard = 13,
ThirdCard = 14,
FourthCard = 15,
FifthCard = 16
}
#region Variables
public static Classic Classic = new Classic();
public static RoyalFlu
I'm afraid that there will be a lot of things that people won't probably understand but you can also check some of my other questions they include fairly enough information about some of my code.
The main thing about the project is obviously the way I determine the play hand. Here's a link to a little bit more detailed explanation of why I'm using modulus and divide operator to check the current card type/suit : Defining what combination the user have in Poker
How I keep things up-to date: https://codereview.stackexchange.com/questions/121942/update-method-using-timer-in-winforms-poker-app
How I actually check if the player has a specific combination: Strategy pattern using an abstract class and an interface
There are a few more question in my user profile which you can try using to get a better understanding of what's going on. Any improvements are welcomed as well as any suggestion that I can implement.
```
using System;
using System.Collections.Generic;
using System.Drawing;
using System.IO;
using System.Linq;
using System.Threading.Tasks;
using System.Windows.Forms;
using Poker.Achievements;
using Poker.Combinations;
using Poker.Users;
using Flush = Poker.Achievements.Flush;
using FourOfAKind = Poker.Achievements.FourOfAKind;
using FullHouse = Poker.Achievements.FullHouse;
using RoyalFlush = Poker.Achievements.RoyalFlush;
using Straight = Poker.Achievements.Straight;
using StraightFlush = Poker.Achievements.StraightFlush;
namespace Poker
{
public partial class MainPoker : Form
{
public enum TableCards
{
FirstCard = 12,
SecondCard = 13,
ThirdCard = 14,
FourthCard = 15,
FifthCard = 16
}
#region Variables
public static Classic Classic = new Classic();
public static RoyalFlu
Solution
Quick glance, going over this from top to bottom:
Do the assigned indices have any special meaning?? why are they there? Either add a comment explaining wtf they mean, or shred them. I daresay you don't actually need them...
nope, nope, nope, nope, nope, nope, nope, nope, nope, nope, nope, nope, nope, nope. THAT is not the intent of region and using it like that is a utterly stupid idea. You're just paving the way for a god-class that does every. single. thing. ever.
That makes your code complex where you should interject abstractions and make code as simple as possible. Regions are just ... meh, mostly.
Two things: Your lines are significantly too long, or rather broken up in unuseful places. Also that's probably better off in a class dedicated to handling achievements.
Again. Linelength.
Also have you heard of Arrays? Consider using something like:
Additionally (because I just realize it now) you probably shouldn't make those
Also... Why is everything
Avoid static Like The Plague
Let's get this straight: The ideal object-oriented project has nothing, I repeat NOTHING in static scope,
that's data.
You should avoid static like the plague, unless you're
implementing pure functions (methods without state or side-effects).
Achievements Class.. please.
A separate class for keeping score...
Okay... Aside from the humungous unused code-block you commented out (and that should be removed, if you don't need it) that's it for your region.
I think if you move all these into specialized classes to separate your concerns, you're busy for a while, so I'll stop here.
It's important to separate responsibilities (what classes do) to make them easy to understand and follow. Why? Because if you have a bug in a laser-focused class it's probably significantly easier to find, than in a mess of ... at least 5 classes that are crammed into one :)
public enum TableCards
{
FirstCard = 12,
SecondCard = 13,
ThirdCard = 14,
FourthCard = 15,
FifthCard = 16
}Do the assigned indices have any special meaning?? why are they there? Either add a comment explaining wtf they mean, or shred them. I daresay you don't actually need them...
#region Variablesnope, nope, nope, nope, nope, nope, nope, nope, nope, nope, nope, nope, nope, nope. THAT is not the intent of region and using it like that is a utterly stupid idea. You're just paving the way for a god-class that does every. single. thing. ever.
That makes your code complex where you should interject abstractions and make code as simple as possible. Regions are just ... meh, mostly.
public static RoyalFlush RoyalFlushAchievement = //..
public static StraightFlush StraightFlushAchievement = //..
public static FourOfAKind FourOfAKindAchievement = //..
public static FullHouse FullHouseAchievement = //..
public static Flush FlushAchievement = //..
public static Straight StraightAchievement = //..
public static MoneyFirst MoneyFirstAchievement = //..
public static MoneySecond MoneySecondAchievement = //..
public static PlayedHands PlayedHandsAchievement = //..Two things: Your lines are significantly too long, or rather broken up in unuseful places. Also that's probably better off in a class dedicated to handling achievements.
public static Bot Bot1 = new Bot(10000, AnchorStyles.Left, "Bot 1", 2, (int)UsersProperties.CUser.Bot1,
new Point(15, 420), false, new Tuple(Player.Chips, 6));
public static Bot Bot2 = new Bot(10000, AnchorStyles.Left, "Bot 2", 4, (int)UsersProperties.CUser.Bot2,
new Point(75, 65), true, new Tuple(Player.Chips, 6));
public static Bot Bot3 = new Bot(10000, AnchorStyles.Left, "Bot 3", 6, (int)UsersProperties.CUser.Bot3,
new Point(590, 25), true, new Tuple(Player.Chips, 6));
// ...Again. Linelength.
Also have you heard of Arrays? Consider using something like:
public static Bot[] bots = new Bot[]
{
// bot initialization
}Additionally (because I just realize it now) you probably shouldn't make those
public. Restrict visibilities as much as possible. Every field that's private can't be tampered with. Which means you don't have to worry about it containing some crap you don't want there. That makes it easier to write straightforward code.Also... Why is everything
static? Advice from my latest java answer applies:Avoid static Like The Plague
Let's get this straight: The ideal object-oriented project has nothing, I repeat NOTHING in static scope,
that's data.
You should avoid static like the plague, unless you're
implementing pure functions (methods without state or side-effects).
public static int[] AllAchievements { get; set; } = {Achievements Class.. please.
public static int AchivementMoney { get; set; }
public static int AchivementHands { get; set; }
public static int Bb { get; set; } = 500;
public static int Sb { get; set; } = 250;
public static int Call { get; set; } = 500;
public static int Raise { get; set; }
public static int Rounds { get; set; }
public static int TempCall { get; set; }
public int Folds { get; set; }
public int WonHands { get; set; }
public int LostHands { get; set; }
public int PlayedHands1 { get; set; }A separate class for keeping score...
Okay... Aside from the humungous unused code-block you commented out (and that should be removed, if you don't need it) that's it for your region.
I think if you move all these into specialized classes to separate your concerns, you're busy for a while, so I'll stop here.
It's important to separate responsibilities (what classes do) to make them easy to understand and follow. Why? Because if you have a bug in a laser-focused class it's probably significantly easier to find, than in a mess of ... at least 5 classes that are crammed into one :)
Code Snippets
public enum TableCards
{
FirstCard = 12,
SecondCard = 13,
ThirdCard = 14,
FourthCard = 15,
FifthCard = 16
}#region Variablespublic static RoyalFlush RoyalFlushAchievement = //..
public static StraightFlush StraightFlushAchievement = //..
public static FourOfAKind FourOfAKindAchievement = //..
public static FullHouse FullHouseAchievement = //..
public static Flush FlushAchievement = //..
public static Straight StraightAchievement = //..
public static MoneyFirst MoneyFirstAchievement = //..
public static MoneySecond MoneySecondAchievement = //..
public static PlayedHands PlayedHandsAchievement = //..public static Bot Bot1 = new Bot(10000, AnchorStyles.Left, "Bot 1", 2, (int)UsersProperties.CUser.Bot1,
new Point(15, 420), false, new Tuple<int?, int>(Player.Chips, 6));
public static Bot Bot2 = new Bot(10000, AnchorStyles.Left, "Bot 2", 4, (int)UsersProperties.CUser.Bot2,
new Point(75, 65), true, new Tuple<int?, int>(Player.Chips, 6));
public static Bot Bot3 = new Bot(10000, AnchorStyles.Left, "Bot 3", 6, (int)UsersProperties.CUser.Bot3,
new Point(590, 25), true, new Tuple<int?, int>(Player.Chips, 6));
// ...public static Bot[] bots = new Bot[]
{
// bot initialization
}Context
StackExchange Code Review Q#122425, answer score: 8
Revisions (0)
No revisions yet.