Changing the opacity property

Question

I have a dropdownbox, ActiviteitAardItems, where ActiviteitAard items can be checked (checkbox). If one (or more) are checked the property Opacity will be changed, and the code beneath will be executed. The Tijdblok mentioned below also has a property Activiteit and ActiviteitAard.

public double Opacity
{
    get
    {
        if (Planning.ActiviteitAardItems.Where(aa => aa.IsChecked == true).Count() > 0)
        {
            if (this.Tijdblokken.Where(t => t.Activiteit != null && t.Activiteit.ActiviteitAard != null).Count() > 0)
            {
                Tijdblok tijdblok;
                var tijdblokken = new List<Tijdblok>();

                foreach (var item in Planning.ActiviteitAardItems.Where(aa => aa.IsChecked == true))
                {
                    tijdblok = this.Tijdblokken.Where(t => t.Activiteit != null && t.Activiteit.ActiviteitAard != null && t.Activiteit.ActiviteitAard.Code == item.Code).FirstOrDefault();
                    if (tijdblok != null)
                        tijdblokken.Add(tijdblok);
                }

                tijdblok = tijdblokken.OrderByDescending(tb => tb.Activiteit.RoosterKleurPrioriteit).FirstOrDefault();

                if (tijdblok != null && tijdblok.Activiteit != null && tijdblok.Activiteit.ActiviteitAard != null)
                {
                    if (Planning.ActiviteitAardItems.Where(aa => aa.IsChecked).Select(x => x.Code).Contains(tijdblok.Activiteit.ActiviteitAard.Code))
                    {
                        return 1.0;
                    }
                    else
                    {
                        return 0.1;
                    }
                }
                else
                {
                    return 0.1;
                }

            }
            else
            {
                return 0.1;
            }
        }

        return 1.0;
    }
}

Answer 1

I think there is probably more that could be offered as the code like something that could do with a makeover, but here's a couple of minor points for consideration.

1. To avoid any potential full enumeration of the items to retrieve the count of items in the list use either the .Count property or the option I prefer .Any(). e.g. Planning.ActiviteitAardItems.Where(aa => aa.IsChecked == true).Any()

2. I personally think lines like aa.Checked == true is unecessary noise. You can achieve the same result by just going .Where(aa => aa.IsChecked).

3. If you are using the same IEnumerable multiple times consider first assigning it to a local variable and doing a ToList() on it. This means that you will avoid any potential multiple enumerations of the dataset as ToList() will force it all back into memory.

   For example

   var ardCheckedItems = Planning.ActiviteitAardItems.Where(aa => aa.IsChecked).ToList();
   

   You can now use this same variable in a couple of places in your code.

4. Does 1.0 and 0.1 mean anything? You could perhaps convert them to constants to avoid any repetition. See example below.

5. Here is an attempt to reduce the Arrow code effect we sometimes find ourselves writing and seems evident in this case.

public double Opacity
{
    get
    {
        const double returnOne = 1.0; // for lack of better names
        const double returnTwo = 0.1;

        var ardCheckedItems = Planning.ActiviteitAardItems.Where(aa => aa.IsChecked).ToList();

        if(!ardCheckedItems.Any()) return returnOne;            

        tijdBlokkenItems = this.Tijdblokken.Where(t => t.Activiteit != null && t.Activiteit.ActiviteitAard != null).ToList();

        if(!tijdBlokkenItems.Any()) return returnTwo;

        Tijdblok tijdblok;
        var tijdblokken = new List<Tijdblok>();

        // See Edit below for an example of how to possibly re-write this
        foreach (var item in ardCheckedItems)
        {
            tijdblok = tijdBlokkenItems.Where(t => t.Activiteit.ActiviteitAard.Code == item.Code).FirstOrDefault();
            if (tijdblok != null)
                tijdblokken.Add(tijdblok);
        }

        tijdblok = tijdblokken.OrderByDescending(tb => tb.Activiteit.RoosterKleurPrioriteit).FirstOrDefault();

        var objectsNotNull = tijdblok != null && tijdblok.Activiteit != null && tijdblok.Activiteit.ActiviteitAard != null;
        var anyCheckedItems = ardCheckedItems.Any(x => x.Code == tijdblok.Activiteit.ActiviteitAard.Code);

        return objectsNotNull && anyCheckedItems ? returnOne : returnTwo;
    }
}

EDIT: After Trevor pointed me in the right direction in order to improve the foreach statement I did a quick search on if it would be easy enough to do an extension method to return items that are not Null. Stack overflow to the rescue https://stackoverflow.com/questions/14469159/linq-selectmany-and-where-extension-method-ignoring-nulls. So altering that slighty I came up with and extension method like so:

public static IEnumerable<TResult> WhereNotNull<TSource, TResult>(
            this IEnumerable<TSource> source, Func<TSource, TResult> selector)
            where TResult : class
{
    return source.Select(selector).Where(sequence => sequence != null);
}

which means I believe we can re-write the foreach like so:

var tijdblokken = ardCheckedItems.WhereNotNull(ard => tijdBlokkenItems.FirstOrDefault(t.Activiteit.ActiviteitAard.Code == ard.Code)).ToList();