Answered Dynamically add/remove items from List<string> based on user input?

[Ren6175]

I posted here the other day and got a good answer. I’ve been working on the code and I have a new challenge. I created a simple program that chooses a random item from a list. I figured out how to make it dynamic using checkboxes. I have 10 groups of data that I created using list<string>. If box A is checked the button chooses a random item from group A. If box A,B,C are checked it chooses a random one from from all three groups. I did this by writing them to a hidden listview box and return a random item from the list then clearing the list.

Anyway, what I need to do now is add another set of checkboxes that will add/remove monsters from the different groups. For context the items are types of monsters for a board game. But most people don’t have all the different monsters. So I want to be able to have them select which expansions they have then that will add/remove that monster from the appropriate list when choosing a random monster.

The only solution I see as a novice is to use a bunch of if statements in front of the .Add. For example,
If (checkboxA.checked) {monster.Add.(“Dragon”);}

Is this the best way? Any advice is appreciated. Thanks.

 

[Ren6175]

Okay. That helps a lot. Now I see how the code can be used to check both lists before returning a value. Just need to figure out how to create both lists in a way that the code can understand.

 

[Ren6175]

Okay. Sorry, but I feel like I might be close to understanding this. If you don’t mind a few more clarifying responses.

[QUOTE="jmcilhinney, post: 11611,

public enum MonsterType
{
    Aerial,
    Terrestrial,
    Aquatic
}

public class Monster
{
    public MonsterType Type { get; set; }

    // ...
}

So this creates “Type” so when you call monster.type later on it will refer back to this. Where do I create the lists that correspond to each type (e.g. spider is terrestrial, dragon is aerial)?

var filteredMonsters = (from monster in monsters
                        where (aerialCheckBox.Checked && monster.Type == MonsterType.Aerial) ||
                              (terrestrialCheckBox.Checked && monster.Type == MonsterType.Terrestrial) ||
                              (aquaticCheckBox.Checked && monster.Type == MonsterType.Aquatic)
                        select monster).ToArray();
var monster = filteredMonsters[myRandom.Next(filteredMonsters.Length)];

[/QUOTE]
Ok. For this part it goes through and returns a true if both the aerial checkbox is checked AND the monster.type is aerial.

Two questions: the || operator is an OR statement that doesn’t check the right side if the left is true but for my purposes both boxes or all 3 could be checked so I think I need to use |.
Lastly what do I do with the result of the last operation? Would I just set a textbox equal to monster?
Thanks.

 

[jmcilhinney]

If you do things as I have, there's not necessarily a need to create separate lists. If you just have every monster in a single list then you can filter by type any time you want. That said, if you need to access just monsters of a particular type often, it would aid performance to have those prefiltered lists, e.g.

var aerialMonsters = monsters.Where(m => m.Type == MonsterType.Aerial).ToArray();

As I implied in post #3, monsters would be a List<Monster> containing every Monster object. This will create an array containing the Monster objects where the Type property is equal to Aerial.

The query I showed using the CheckBoxes will include monsters where the Type is a particular value if and only if the corresponding CheckBox is checked. You can add as many conditions as you need; one for each Type value. The Boolean operators && and || both short-circuit, i.e. they both stop evaluating conditions as soon as the result of the overall expression cannot change. In my example, there are three conditions being ORed and evaluation will end as soon as one of those conditions evaluates to true. The Boolean operators & and | are the non-short-circuiting versions but are rarely used, because short-circuiting is almost always a good thing.

In my example, the monster variable contains a random monster of a type specified by the user based on the checked CheckBoxes. What you do with that is completely up to you. You're the one who asked how to get such a random monster. What did you want to do with it in the first place? Do that.

Note that you can always recombine separate lists into one. The alternative to filtering a complete list as I have shown is to conditionally combine partial lists, e.g.

IEnumerable<Monster> filteredMonsterList = new Monster[] {};

if (aerialCheckBox.Checked)
{
    filteredMonsterList = filteredMonsterList.Concat(aerialMonsters);
}

if (terrestrialCheckBox.Checked)
{
    filteredMonsterList = filteredMonsterList.Concat(terrestrialMonsters);
}

if (aquaticCheckBox.Checked)
{
    filteredMonsterList = filteredMonsterList.Concat(aquaticMonsters);
}

var filteredMonsters = filteredMonsterList.ToArray();
var monster = filteredMonsters[myRandom.Next(filteredMonsters.Length)];

 

[jmcilhinney]

Starting with the example in my previous post, if you wanted to make that more dynamic then you could assign each partial list to the Tag of the corresponding CheckBox and then do this:

IEnumerable<Monster> filteredMonsterList = new Monster[] {};

foreach (var monsterTypeCheckBox in Controls.OfType<CheckBox>().Where(cb => cb.Checked))
{
    filteredMonsterList = filteredMonsterList.Concat((List<Monster>) monsterTypeCheckBox.Tag);
}

 

[Ren6175]

Thanks for the detailed explanation. I thought I was getting close to understanding the code without your explanation and I did understand some of it, but other stuff I was way off. I appreciate your time.

 

[Ren6175]

In my example, the monster variable contains a random monster of a type specified by the user based on the checked CheckBoxes. What you do with that is completely up to you. You're the one who asked how to get such a random monster. What did you want to do with it in the first place? Do that.

One more thing before I mark this as answered. It sounded like you might be a little frustrated at the end there. Sorry about all my questions. I just wanted to understand everything instead of just having you write the code for me. I mean, don't get me wrong, it was awesome that you did write it, but I didn't want to just copy you without knowing what I was copying.

Anyway, thanks for you help.

 

[jmcilhinney]

I can come across as frustrated at any time and it may or may not correspond to actual frustration. Just take what I post at face value. If you try to read too much into it then you'll think that I'm pissed off all the time.

 

[Ren6175]

Haha. Okay. I changed this to answered, but I actually have another question now.

public enum MonsterType
        {
            Building, Cave, Civilized, Cold, Cursed, Dark, Hot, Mountain, Water, Wilderness
        }
public class Monster
        {
            public MonsterType Type { get; set; }
            //
            
        }

What do I put here ? I know that somewhere (maybe here or in the List<monster> I need to define which monsters are which type.

Random rnd = new Random();
            var filteredMonsters = (from allmonster in allmonsters
                                where (building.Checked && allmonster.Type == MonsterType.Building) ||
                                      (cave.Checked && allmonster.Type == MonsterType.Cave) ||
                                      (civilized.Checked && allmonster.Type == MonsterType.Civilized)
                                select allmonster).ToArray();
            var monster = filteredMonsters[rnd.Next(filteredMonsters.Length)];

            var aerialMonsters = allmonsters.Where(m => m.Type == MonsterType.Building).ToArray();

            textBox1.Text = Convert.ToString(monster);

Also, you can't see it, but I'm still getting an error above where it says allmonster.Type. It says, "string does not contain a definition for Type." I assume this error relates to whatever I'm missing above. I've spent a few hours searching MSDN and other tutorials, but I can't figure out how to define .Type. I will keep searching but if you can explain this to me, I think I will have it.

Cheers,
Dave

 

[jmcilhinney]

When you define an enum, the underlying type is int by default and the values are sequential starting at 0 by default. That means that, if you don't set the value of a MonsterType property then it will take on the zero value by default, i.e. Building in your case. If every instance of your Monster type should have its Type property set then the most appropriate way to go is to declare a constructor that requires a value for that property:

public class Monster
{
    public MonsterType Type { get; set; }
    
    public Monster(MonsterType type)
    {
        Type = type;
    }
}

Now, it's impossible to create a new Monster without specifying what type it is, e.g.

var monster = new Monster(MonsterType.Cave);

If there are any other properties that are required then you should set them via the constructor too. Optional properties can be set after the fact:

var monster = new Monster();

monster.Type = MonsterType.Cave;

or, more succinctly:

var monster = new Monster {Type = MonsterType.Cave};

As for that error, I think I recall seeing ealsewhere that you were simply using a string containing a name to represent a monster. That's no good any more because, as the error message says, the String class has no Type property. You need to use your Monster class to represent a monster, probably moving the strings you have into a Name property.

 

[Ren6175]

I finally got everything to work. Monsters have two types. Can I do this?

Monster lavabeetle = new Monster("Lava Beetle", MonsterType.Hot|MonsterType.Cave);

Or do I need to define that each Monster has more than one type like this?

public Monster(string monsterName, MonsterType type, MonsterType type)
            {
                MonsterName = monsterName;
                Type = type;

            }

 

[jmcilhinney]

There are a few things you should do if you want a monster to have multiple types. Some are requirements and some are convention:

1. Use a plural name for your Enum.
2. Decorate your Enum with the Flags attribute.
3. Explicitly set the values of your Enum fields with increasing powers of 2.
4. Only use 0 for a field that specifically represents no value.

My example Enum would become:

[Flags()]
public enum MonsterTypes
{
    Aerial = 1,
    Terrestrial = 2,
    Aquatic = 4
}

You do indeed then combine multiple values using the bitwise OR operator, i.e. |. You can't then use the equality operator alone to determine whether a Monster is a particular type though, because a combined value doesn't equal a singular value. You need to determine whether a combined value contains a singular value, which you do by masking, e.g. this:

var aerialMonsters = allmonsters.Where(m => m.Type == MonsterType.Building).ToArray();

becomes this:

var aerialMonsters = allmonsters.Where(m => (m.Type & MonsterTypes.Building) == MonsterTypes.Building).ToArray();

 

[Ren6175]

I just wanted to share how I got it to work. I filtered monsters into groups, then added them to a list if the appropriate box was checked. Converted that to an array and chose a random monster from that array. Thanks again.

private void Button3_Click(object sender, EventArgs e)
        {
            Random rnd = new Random();

            var buildingMonsters = allmonsters.Where(m => (m.Type & MonsterTypes.Building) == MonsterTypes.Building).ToArray();
            var caveMonsters = allmonsters.Where(m => (m.Type & MonsterTypes.Cave) == MonsterTypes.Cave).ToArray();
            var civilizedMonsters = allmonsters.Where(m => (m.Type & MonsterTypes.Civilized) == MonsterTypes.Civilized).ToArray();
            var coldMonsters = allmonsters.Where(m => (m.Type & MonsterTypes.Cold) == MonsterTypes.Cold).ToArray();
            var cursedMonsters = allmonsters.Where(m => (m.Type & MonsterTypes.Cursed) == MonsterTypes.Cursed).ToArray();
            var darkMonsters = allmonsters.Where(m => (m.Type & MonsterTypes.Dark) == MonsterTypes.Dark).ToArray();
            var hotMonsters = allmonsters.Where(m => (m.Type & MonsterTypes.Hot) == MonsterTypes.Hot).ToArray();
            var mountainMonsters = allmonsters.Where(m => (m.Type & MonsterTypes.Mountain) == MonsterTypes.Mountain).ToArray();
            var waterMonsters = allmonsters.Where(m => (m.Type & MonsterTypes.Water) == MonsterTypes.Water).ToArray();
            var wildernessMonsters = allmonsters.Where(m => (m.Type & MonsterTypes.Wilderness) == MonsterTypes.Wilderness).ToArray();


            List<Monster> randommonster = new List<Monster>();
            if (building.Checked) { randommonster.AddRange(buildingMonsters); }
            if (cave.Checked) { randommonster.AddRange(caveMonsters); }
            if (civilized.Checked ){randommonster.AddRange(civilizedMonsters); }
            if (cold.Checked ){randommonster.AddRange(coldMonsters);}
            if (cursed.Checked){randommonster.AddRange(cursedMonsters);}
            if (dark.Checked ){randommonster.AddRange(darkMonsters);}
            if (hot.Checked ){randommonster.AddRange(hotMonsters);}
            if (mountain.Checked ){randommonster.AddRange(mountainMonsters);}
            if (water.Checked ){randommonster.AddRange(waterMonsters);}
            if (wilderness.Checked){randommonster.AddRange(wildernessMonsters);}
            Monster[] filteredMonsters = randommonster.ToArray();
            
            
            if (building.Checked || cave.Checked || civilized.Checked || cold.Checked || cursed.Checked || dark.Checked || hot.Checked || mountain.Checked || water.Checked || wilderness.Checked)
            {

                var chosenMonster = filteredMonsters[rnd.Next(filteredMonsters.Length)];
                textBox4.Text = Convert.ToString(chosenMonster.MonsterName);
            }
            else
            {
                textBox4.Text = "You must choose at least one monster group!";
            }

 

[jmcilhinney]

There are a few changes you should make there.

1. Don't create a new Random object inside a method. It likely wouldn't be a problem in this case but that can lead to the same numbers being generated multiple times. Create a single instance of the Random class, assign it to a member variable and then use it each time.
2. There's no point going through those first two blocks of code if no CheckBoxes are checked so that last if statement should actually be the very first thing you do. If no selections have been made, why do all that filtering?
3. You're being quite wasteful there, creating lots of intermediate arrays and lists, most of which are unnecessary.

With regards to point 3, if you use an IEnumerable<T> instead of a List<T> in that second block then you avoid materialising the list until you actually know what you need, so you can go straight to the array without doubling up the storage to create the List<T> in between:

IEnumerable<Monster> filteredMonsterList = new Monster[] {};
if (building.Checked) { filteredMonsterList = filteredMonsterList.Concat(buildingMonsters); }
// ...
if (wilderness.Checked){ filteredMonsterList = filteredMonsterList.Concat(wildernessMonsters); }
var filteredMonsters = filteredMonsterList.ToArray();

You need that empty array to start with because you need something to concatenate to but that's still more efficient. The thing is, you don't need to create all those intermediate filtered arrays either. Clarity of code is important so it is worth giving up some efficiency to get it but, in this case, I don;t see a problem with this:

IEnumerable<Monster> filteredMonsterList = new Monster[] {};
if (building.Checked) { filteredMonsterList = filteredMonsterList.Concat(allMonsters.Where(m => (m.Type & MonsterTypes.Building) == MonsterTypes.Building)); }
// ...
if (wilderness.Checked){ filteredMonsterList = filteredMonsterList.Concat(allMonsters.Where(m => (m.Type & MonsterTypes.Wilderness) == MonsterTypes.Wilderness)); }
var filteredMonsters = filteredMonsterList.ToArray();

Actually, something else that I just realised is that, if the same monster can have multiple types, you're actually going to end up with duplicates in that final array. That means that some monsters will be incorrectly more likely to be selected. For that reason, you should remove duplicates:

var filteredMonsters = filteredMonsterList.Distinct().ToArray();

One more option is that you could actually improve efficiency as I've suggested without compromising readability by using a method to do the concatenation, e.g.

private IEnumerable<Monster> ConcatFilteredMonsters(
    IEnumerable<Monster> allMonsters,
    IEnumerable<Monster> filteredMonsters,
    MonsterTypes type)
{
    return filteredMonsters.Concat(allMonsters.Where(m => (m.Type & type) == type));
}

You can then do this:

if (building.Checked) { filteredMonsterList = ConcatFilteredMonsters(allMonsters, filteredMonsterList, MonsterTypes.Building); }

 

[jmcilhinney]

Personally, I would probably assign the corresponding MonsterTypes value to the Tag property of each CheckBox, or possibly even create my own class that inherits CheckBox and adds a dedicated property. You could then put all your CheckBoxes into an array and do this:

foreach (var type in allCheckBoxes.Where(cb => cb.Checked).Select(cb => (MonsterTypes)cb.Tag))
{
    filteredMonsterList = ConcatFilteredMonsters(allMonsters, filteredMonsterList, type);
}