More efficient way to find all elements in a List

Question

I got this code:

List<string> values = getValues();
Details detailsData = getDetails();

if (values[0].Equals(detailsData.Attr1))
    if (values[1].Equals(detailsData.Attr2))
        if (values[2].Equals(detailsData.Attr3))
            if (values[3].Equals(detailsData.Attr4))
                if (values[4].Equals(detailsData.Attr5))
                    return true;

getValues() returns a string list that i'm hoping (^^) is present on Details object.

Is there a more efficient or more "pretty" way to implement this code above?

EDIT

 class Details : MainDetails
{
    string Attr3{ get; set; }
    string Attr4{ get; set; }
    string Attr5{ get; set; }
}

 class MainDetails
{
    string Attr1{ get; set; }
    string Attr2{ get; set; }
}

Btw there could be some other objects like Details that extend MainDetails and require this validation. So this is code is gonna be used multiple times.

Answer 1

Simple improvement:

if (values[0].Equals(detailsData.Attr1) &&
    values[1].Equals(detailsData.Attr2) &&
    values[2].Equals(detailsData.Attr3) &&
    values[3].Equals(detailsData.Attr4) &&
    values[4].Equals(detailsData.Attr5))
{
    return true;
}

If you're only doing this in one place, I wouldn't bother going any further. If you do the same thing in several places, you might consider setting up e.g. a function turning your Details into another List<string> and then compare them using e.g. SequenceEquals.

Edit: Although the more I consider it, the more it seems comparing a flat list of strings to one in a hierarchy of classes seems like a bad idea, here's an example of how you could do this in an extensible fashion. The base class "consumes" the top two values, then hands off to the derived class to check the remainder:

class MainDetails
{
    string Attr1 { get; set; }
    string Attr2 { get; set; }

    protected virtual bool Matches(IEnumerator<string> e)
    {
        // Check the first two items exist and match
        return
            e.MoveNext() && e.Current.Equals(Attr1) &&
            e.MoveNext() && e.Current.Equals(Attr2);
    }

    public bool Matches(IEnumerable<string> details)
    {
        using (var e = details.GetEnumerator())
        {
            // Check the details match. (Optionally check
            // that there are no "extra" details.)
            return Matches(e); // && !e.MoveNext();
        }
    }
}

class Details : MainDetails
{
    string Attr3 { get; set; }
    string Attr4 { get; set; }
    string Attr5 { get; set; }

    protected override bool Matches(IEnumerator<string> e)
    {
        // Check the MainDetails match, and the next three too.
        return base.Matches(e) &&
            e.MoveNext() && e.Current.Equals(Attr3) &&
            e.MoveNext() && e.Current.Equals(Attr4) &&
            e.MoveNext() && e.Current.Equals(Attr5);
    }
}

...

List<string> values = getValues();
Details detailsData = getDetails();

if (detailsData.Matches(values))
    return true;

I'm not sure of your exact use case but hopefully this migth give you some ideas.

Answer 2

If you want a clean code where you use it you can put some indexers on your class.

    class Details : MainDetails
    {
        string Attr3 { get; set; }
        string Attr4 { get; set; }
        string Attr5 { get; set; }

        public override string this[int index]
        {
            get
            {
                switch (index)
                {
                    case 3:
                        return Attr3;
                    case 4:
                        return Attr4;
                    case 5:
                        return Attr5;
                }
                return base[index];
            }
        }
    }

    class MainDetails
    {
        string Attr1 { get; set; }
        string Attr2 { get; set; }

        public virtual string this[int index] 
        {
            get
            {
                switch (index)
                {
                    case 1:
                        return Attr1;
                    case 2:
                        return Attr2;
                    default:
                        throw new NotImplementedException();
                }
            }
        }
    }

This way you can use a for-loop to compare

for(int i = 0; i < 5; i++)
{
   if(values[i] != detailsdata[i+1])
      return false;
}
return true;

same could be achieved with just putting a Method on the class like GetAtttributValue(int i);

Answer 3

In Addition to Rawlings answer. A function to turn the Details-object into a list, could look like this:

private static IEnumerable<string> GetDetailProperties(Details details) {
    yield return details.Attr1;
    yield return details.Attr2;
    yield return details.Attr3;
    yield return details.Attr4;
    yield return details.Attr5;
}

You can than use the SequenceEquals method to compare both lists.

Answer 4

I would say something like

List<string> values = getValues();
Details detailsData = getDetails();

if (values.SequenceEqual(new[] { detailsData.Attr1, detailsData.Attr2, detailsData.Attr3, detailsData.Attr4, detailsData.Attr5, }))
    return true;

This uses the SequenceEqual extension method of LINQ, see MSDN: Enumerable.SequenceEqual<TSource>.

Answer 5

There is no "clearer" way of doing this while your properties are named like that. You could use the reflection (to save 2 code lines), but this will hit perfomance issue.

Answer 6

If you cannot change your properties to all be in an array, then you can at least write the code a bit more prettily... although this is extremely subjective!

List<string> values = getValues();
Details detailsData = getDetails();

if
(
    values[0].Equals(detailsData.Attr1) &&
    values[1].Equals(detailsData.Attr2) &&
    values[2].Equals(detailsData.Attr3) &&
    values[3].Equals(detailsData.Attr4) &&
    values[4].Equals(detailsData.Attr5)
)
{
    return true;
}