Max
Max

Reputation: 53

Check if object of type X is in list and update corresponding field fails for reasons I don't understand

I'm trying to check if a list of Quests contains a Quest of Type StartingQuest. If it does, I want to increase the field OgresKilled that belongs to the Quest by 1.

The following code does not work.

I get the error message in the second line of code where I check ... == StartingQuest:

Error CS0119 StartingQuest is a type, which is not valid in the given context

This does not make sense to me, as I explicitly want to check against the type.

if (Globals.ActiveQuests.Contains(new StartingQuest()))
{
    StartingQuest a = Globals.ActiveQuests.Find(a => a.GetType == StartingQuest);
    a.OgresKilled += 1;
}

Can someone please explain what my mistake is? And is this an efficient way of doing what I want to do?

Upvotes: 0

Views: 185

Answers (3)

SomeBody
SomeBody

Reputation: 8743

Your if check will always fail, because you look for a new instance of StartingQuest. If it is a reference type, this will never be true, because the newly created instance will have a different reference than all the quests in the list. Moreover, you are iterating the list twice, one time in the Contains method, and one time in the Find method. This will fix these issues:

var startingQuest = Globals.ActiveQuests.FirstOrDefault(x => x.GetType() == typeof(StartingQuest));
// as an alternative, this is also possible:
// var startingQuest = Globals.ActiveQuests.OfType<StartingQuest>().FirstOrDefault();
if(!(startingQuest is null))
{
     startingQuest.OgresKilled += 1;
}

FirstOrDefault returns the first instance that matches a criterion. If no item is found, the default value of that type is returned (null for reference types). Hence you check whether the return value is not null. If it's not null, an item was found and you can increment your OrgresKilled variable.

Upvotes: 2

Jon Skeet
Jon Skeet

Reputation: 1502306

Other answers have explained aspects that are incorrect in the code in the question:

  • It checks for equality between an existing quest and a brand-new StartingQuest instance, which is unlikely to ever be true
  • It tries to use the method group GetType directly, instead of calling the method (as a.GetType())
  • It tries use a type name directly in the == expression, where you intended to evaluate a Type reference - typeof will help there.

I would suggest a slight modification of SomeBody's answer (in fact explaining an option already present in a comment).

Currently both the question and all the answers here are checking for the exact type StartingQuest, which is somewhat brittle. It's usually more useful to check whether something is a target type or a derived type, even if you don't currently have any derived types.

LINQ provides the OfType method which will find elements of a sequence in precisely that manner - and the resulting sequence has an element type of that "target type", which means that if you need to access something that's in StartingQuest but not the base type (Quest or whatever it is), you'll be fine.

So I'd definitely use:

// Import the System.Linq namespace to make the OfType and FirstOrDefault 
// extension methods available
using System.Linq;

...

var startingQuest = Globals.ActiveQuests.OfType<StartingQuest>().FirstOrDefault();
if (startingQuest is object)
{
   startingQuest.OgresKilled++;
}

The use of is object instead of != null is just to use the more modern idiom for checking whether something is non-null.

Upvotes: 4

Magnetron
Magnetron

Reputation: 8573

Your error is because you have to get the type of StartingQuest:

a.GetType() == typeof(StartingQuest)

GetType returns a Type, so you also need an instance of the type Type to compare to. That's what typeof does, it returns an instance of Type that corresponds to the type of its argument.

Upvotes: 1

Related Questions