Method of storing strings other than List

Question

I'm developing a poker app and currently I want to store all the card combinations names I'm planning to use list and do something like this :

private static List<string> combinationNames = new List<string>
{
    " High Card ",
    " Pair ",
    " Two Pair ",
    " Three of a Kind ",
    " Straight ",
    " Flush ",
    " Full House ",
    " Four of a Kind ",
    " Straight Flush ",
    " Royal Flush ! "
};

for (int j = 0; j < combinationNames.Count; j++)
{
    if (current == j)
    {
        MessageBox.Show("You Have : ", combinationNames[j]);

    }
}

So is there a better way of storing those names and later access them like I did ?

Define "better". What is causing you a problem at the moment? — Lee Taylor, Jan 12 '16 at 23:24
Why do you need a loop? Mb simply `combinationNames[current]`? — Kirill Polishchuk, Jan 12 '16 at 23:25
there is actually no current i just wrote this i do some other checks before it also there's no problem with my code im asking for a better way of storing those strings instead of putting them in the list — kopelence, Jan 12 '16 at 23:26
But "better" really depends on what you ultimately end up doing with the rest of your code. This is probably something you will be best to just do, then learn from experience. — Lee Taylor, Jan 12 '16 at 23:28
You can use Dictionary or HashSet. Look here for more: https://msdn.microsoft.com/en-us/library/system.collections.generic(v=vs.110).aspx — joordan831, Jan 12 '16 at 23:29
You've still not defined what you mean by better. Its kind of like asking if there is a better tool than a hammer - it very much depends if you are driving nails or turning screws — Stewart_R, Jan 12 '16 at 23:29
by better i mean more compact .. English is not my native language so i thought that they have a similar meaning — kopelence, Jan 12 '16 at 23:30
@kopelence Just get on with your poker game, and see how it pans out. — Lee Taylor, Jan 12 '16 at 23:31
All of those plays have a value (at least, some of them "weight" more than others), so would recommend using a Enumeration: http://stackoverflow.com/a/479453/335905 — celerno, Jan 12 '16 at 23:33
Yea i was thinking exactly for enum but im not sure if it's going to do any better — kopelence, Jan 12 '16 at 23:35
The higher hurdle will be detecting those correctly in hands. Since you wont be adding to the list over time, this is a case where an array would work just fine — Ňɏssa Pøngjǣrdenlarp, Jan 12 '16 at 23:36
Well i actually have this already im doing a win method and i want to print the winner + the hand this list is just so i dont need to do `if(current==1)` etc — kopelence, Jan 12 '16 at 23:37
@kopelence I would go with a Dictionary, then I can access the hand by a well-defined name. — AnjumSKhan, Jan 13 '16 at 05:50
An unrelated note on things: there's no difference between a straight flush and a royal flush; a royal flush is merely the highest straight flush you can have. So, my opinion is that for the same reason you don't (in this list) distinguish between a K-high straight flush and a 5-high straight flush, there's no reason to distinguish between an A-high straight flush and any other. — mah, Jan 15 '16 at 15:43
This is not the method for checking combinations it's just a message box shower so in case you got royal flush it's quite rare and i just want to notice it. — kopelence, Jan 16 '16 at 01:16

score 0 · Accepted Answer · answered Jan 13 '16 at 05:42

There's not much to go on in your question to understand what's specifically wrong with the code you have. That said, at the very least I would expect the following to be an improvement:

private readonly static string[] combinationNames =
{
    " High Card ",
    " Pair ",
    " Two Pair ",
    " Three of a Kind ",
    " Straight ",
    " Flush ",
    " Full House ",
    " Four of a Kind ",
    " Straight Flush ",
    " Royal Flush ! "
};

if (current >= 0 && current < combinationNames.Length)
{
    MessageBox.Show("You Have : ", combinationNames[current]);
}

I.e.:

Since the list won't change, it can be an array instead of a list
Since the list object won't change, the variable can be readonly
All the code you did does with j is compare it to current; there's no need to enumerate every possible value for j…just make sure current is within the valid range and then use its value directly.

Note on that last point it's not really clear where you get current from, but likely it should already be guaranteed to be valid before you get as far as displaying the text, so you shouldn't even really need the range check. I just put that there to ensure the new version of code above is reasonably consistent with the behavior of the code you showed (what little was there).

If you need more specific advice than the above, please explain more precisely what you think would be "better" and in what way the code you have now is not sufficiently addressing your needs. I.e. what does the code do now and how is that different from what you want it to do?

Method of storing strings other than List

1 Answers1