CODEWITHSUNDEEP
wpfevent-handling
Christian Ruppert
Christian Ruppert

Reputation: 3779

C# EventHandler Beautiful Code (How To?)

I admit, it is kind of tiny, but I am looking for better ways to do the following code blocks. They should be self explaining...

    private void listBoxItem_PreviewMouseDown(object sender, MouseButtonEventArgs e)
    {
        var listBoxItem = sender as ListBoxItem;
        if (listBoxItem != null)
        {
            var clickObject = listBoxItem.DataContext as ClickObject;
            if (clickObject != null)
            {
                clickObject.SingleClick();
            }
        }
    }

Another ugly one:

    private void listBox_SelectionChangedA(object sender, SelectionChangedEventArgs e)
    {
        var lB = sender as ListBox;
        if (lB != null)
            StatusBoxA.Text = "Elements selected" + lB.SelectedItems.Count;
    }

Yeah, I know, its not near-death-urgent. But I do NOT like the (if != null). Any magic ideas to shorten it even more :-)

Btw, I found some nice info about a similar topic: Loops on Null Items Nice to read...

Upvotes: 5

Views: 1225

Answers (8)

Cameron MacFarland
Cameron MacFarland

Reputation: 71856

Using the same idea as Utaal's solution, but as an extension method...

public static void As<TSource>(this object item, Action<TSource> action) where TSource : class
{
    var cast = item as TSource;

    if (cast != null)
        action(cast);
}

private void listBoxItem_PreviewMouseDown(object sender, MouseButtonEventArgs e)
{
    sender.As<ListBoxItem>(listBoxItem => 
        listBoxItem.DataContext.As<ClickObject>(clickObject =>
            clickObject.SingleClick()));
}

Upvotes: 0

Utaal
Utaal

Reputation: 8534

One-liner:

private void listBox_SelectionChangedA(object sender, SelectionChangedEventArgs e)
{
    As<ListBox>(sender, (lB) => StatusBoxA.Text = "Elements selected" + lB.SelectedItems.Count);
}

or, nested:

private void listBoxItem_PreviewMouseDown(object sender, MouseButtonEventArgs e)
{
    As<ListBoxItem>(sender, (listBoxItem) => {
        As<ClickObject>(listBoxItem.DataContext,
            (clickObject) => clickObject.SingleClick());
    };
}

using this static generic method (T is destination type, input is object to cast, code is a delegate (or lambda expression) to execute on success:

static void As<T>(object input, Action<T> code) where T : class
{
  T foo = input as T;
  if (foo != null)
  code(foo);
}

Upvotes: 2

Sergey Aldoukhov
Sergey Aldoukhov

Reputation: 22744

private void listBoxItem_PreviewMouseDown(object sender, MouseButtonEventArgs e)
{
        var listBoxItem = sender as ListBoxItem;
        if (listBoxItem == null) return;

        var clickObject = listBoxItem.DataContext as ClickObject;
        if (clickObject == null) return;

        clickObject.SingleClick();
}

Upvotes: 3

ageektrapped
ageektrapped

Reputation: 14562

Since you're using known events from the .NET framework (as opposed to a third party) and from the code it looks like you're only using those methods for specific classes (i.e. ListBoxItems and ListBoxes), there are a few things you know to be true:

  • sender will never be null
  • sender will always be a ListBoxItem, or ListBox, respectively

So why use the as operator? Just cast!

Then the first snippet becomes

private void listBoxItem_PreviewMouseDown(object sender, MouseButtonEventArgs e)
{
        var listBoxItem = (ListBoxItem)sender;
        var clickObject = (ClickObject)listBoxItem.DataContext;
        clickObject.SingleClick();
}

Note this isn't true in the general case (you wouldn't do this if you were handling all PreviewMouseDown events in that one handler for all Control types), but for event handling code like this, especially in UI code, you can be as certain as you can be of anything, that sender will not be null and sender will be of the type you expect.

Upvotes: 1

Diadistis
Diadistis

Reputation: 12174

I love good, clean code but in most cases, clean & elegant doesn't mean short and smart. Code brevity is good for competitions. Changing an "if not null" statement to a foreach might seem way cooler but it's harder for everyone else working in the project to understand what you are trying to accomplish. Believe me, even you won't remember it a few months later :P. Your code is just fine as it is!

Upvotes: 8

eulerfx
eulerfx

Reputation: 37719

You can add extensions methods to Form elements, which can then trigger the events:

public static void OnSelectionChanged(this ListBox b, Action<ListBox> a)
{
    b.SelectedIndexChanged += (s,e) =>
    {
        if (s is ListBox)
           a(s as ListBox);           
    };
}

Upvotes: 0

ChrisW
ChrisW

Reputation: 56113

This is supposed to be the same as the first one, reformatted a little:

private void listBoxItem_PreviewMouseDown(object sender, MouseButtonEventArgs e)
{
    ClickObject clickObject;
    if (
        ((sender as ListBoxItem) != null) &&
        ((clickObject = ((ListBoxItem)sender).DataContext as ClickObject) != null)
        )
    {
        clickObject.SingleClick();
    }
}

Upvotes: 0

benPearce
benPearce

Reputation: 38333

Maybe I am just being pedantic but why do you need to cast the sender if you are using the event within its host containers code.

Regardless of who made the change to a list, couldn't you just give your listbox a name and use that.

<ListBox x:Name="listbox1" />

private void listBox_SelectionChangedA(object sender, SelectionChangedEventArgs e)
{
    StatusBoxA.Text = "Elements selected" + listbox1.SelectedItems.Count;
}

Or you could even achieve some of this using binding with no code behind.

Upvotes: 0

Related Questions