Improve Code: Compare 2 list of elements

Question

Hi, sorry for the newbie question but, I'm new to programming..

I want to check if there's already no more than one element of TypeA in listOfDifferentTypes. I've following code:

     public void CheckType ( Object param)
     {
            if ( param is TypeA )
            {
                int i = 0;
                TypeA paramToCheck = ( TypeA ) param;

                foreach ( var paramB in listOfDifferentTypes )
                {
                    if ( paramB is TypeA )
                    {
                        var paramInList = ( TypeA ) paramB;
                        if ( paramToCheck.ID == paramInList.ID )
                        {
                            i++;
                        }
                    }
                }
                if ( i > 1 )
                {
                    paramToCheck.m_Error = "ErrorText";
                }    
            }
    }

I consider it's not very clean solution. Can this code be improved / optimized?

Answer 1

You could use LINQ for this :) It will look nice:

//Checks for string - resplace <string> with <some type> for other types
private bool moreThanOne(List<object> differentTypes)
{
    return differentTypes.OfType<string>().Count() > 1;
}

Usage:

List<object> listOfDifferentTypes = new List<object> { "string", 13, 52, "string", 54.3f };

var res = moreThanOne(listOfDifferentTypes);

I see you also checks for some sort of ID then try this:

UPDATED to do what your code is doing

Updated: replaced .Count() with .Skip(1).Any() so it will stop if more than 1 is found :)

private void CheckType(object param, List<object> differentTypes)
{
    var paramToCheck = param as TypeA;

    if (paramToCheck == null) return;

    var res = differentTypes.OfType<TypeA>().Where(t => t.ID == paramToCheck.ID).Skip(1).Any();

    if (res) paramToCheck.m_Error = "error text";
}

As I have done, you can replace:

if (param is TypeA)
{
    TypeA paramToCheck = (TypeA) param;
    ... Do something

With:

TypeA paramToCheck = param as TypeA; //Returns null if not a TypeA
if (param == null) return;

... Do something

It is a little faster :)

Answer 2

Your original solution, rewritten:

public void CheckType(Object param)
{
   TypeA paramToCheck = param as TypeA;
   int count = 0;
   if (paramToCheck != null)
   {
       foreach (var paramB in listOfDifferentTypes)
       {
           var paramInList = paramB as TypeA;
           if (paramInList != null && paramToCheck.ID == paramInList.ID)
           {
               count++;

               if (count > 1)
               {
                   paramToCheck.m_Error = "ErrorText";
                   break;
               }
           }
       }
   }

}

Notes:

• use the as keyword with a comparison against null to perform type conversion,
• combine multiple conditions into a single if statement (using the AND (&&) operator),
• use the break statement to exit the foreach loop as soon as your condition has been satisfied.
• This is just a cleansed version of your original code; there are no doubt better ways to achieve your desired behaviour :-)

Edit: updated re: comments (thanks for pointing out my previous mistake!)