patterncsharpMinor
Even handler for text input preview
Viewed 0 times
previewtextinputforevenhandler
Problem
In a WPF application I have the following event handler for
```
private void OnTextComboBoxPreviewTextInput(object sender, TextCompositionEventArgs e)
{
var comboBox = sender as ComboBox;
if (!comboBox.IsDropDownOpen)
{
return;
}
if (!comboBox.IsEditable || !comboBox.IsReadOnly)
{
return;
}
foreach (var item in comboBox.Items)
{
if (item == null)
{
continue;
}
var textSearch = TextSearch.GetTextPath(comboBox);
var stringItem = item as string;
if (stringItem != null)
{
if (stringItem.StartsWith(e.Text, StringComparison.InvariantCultureIgnoreCase))
{
SelectItemInComboBox(comboBox, item);
break;
}
continue;
}
var dependencyObjItem = item as DependencyObject;
if (dependencyObjItem != null)
{
var textMember = TextSearch.GetText(dependencyObjItem);
if (!string.IsNullOrEmpty(textMember))
{
var selectorFunc = ExpressionHelper.GetMemberFunction(dependencyObjItem, textMember);
var textValue = selectorFunc(dependencyObjItem);
if (textValue.ToString().StartsWith(e.Text, StringComparison.InvariantCultureIgnoreCase))
{
SelectItemInComboBox(comboBox, item);
break;
}
continue;
}
}
if (!string.IsNullOrEmpty(textSearch))
{
var selectorFunc = ExpressionHelper.GetMemberFunction(item, textSearch);
var textValue = selectorFunc(item);
if (textValue.ToString().StartsWith(e.Text, StringComparison.InvariantCultureIg
PreviewTextInput event in ComboBox control. The main purpose of this code is to select an item in ComboBox via pressing a letter key. There is duplicated logic in this code and I want it removed.```
private void OnTextComboBoxPreviewTextInput(object sender, TextCompositionEventArgs e)
{
var comboBox = sender as ComboBox;
if (!comboBox.IsDropDownOpen)
{
return;
}
if (!comboBox.IsEditable || !comboBox.IsReadOnly)
{
return;
}
foreach (var item in comboBox.Items)
{
if (item == null)
{
continue;
}
var textSearch = TextSearch.GetTextPath(comboBox);
var stringItem = item as string;
if (stringItem != null)
{
if (stringItem.StartsWith(e.Text, StringComparison.InvariantCultureIgnoreCase))
{
SelectItemInComboBox(comboBox, item);
break;
}
continue;
}
var dependencyObjItem = item as DependencyObject;
if (dependencyObjItem != null)
{
var textMember = TextSearch.GetText(dependencyObjItem);
if (!string.IsNullOrEmpty(textMember))
{
var selectorFunc = ExpressionHelper.GetMemberFunction(dependencyObjItem, textMember);
var textValue = selectorFunc(dependencyObjItem);
if (textValue.ToString().StartsWith(e.Text, StringComparison.InvariantCultureIgnoreCase))
{
SelectItemInComboBox(comboBox, item);
break;
}
continue;
}
}
if (!string.IsNullOrEmpty(textSearch))
{
var selectorFunc = ExpressionHelper.GetMemberFunction(item, textSearch);
var textValue = selectorFunc(item);
if (textValue.ToString().StartsWith(e.Text, StringComparison.InvariantCultureIg
Solution
-
I prefer a simple cast over
-
When it makes sense to group code together do so.
-
Try to use LINQ where possible.
-
Place variables close to where they are being used. (
-
Don't overuse
-
Using
-
Identify redundant code, see what you can eliminate and what you can't. You might have to introduce new intermediate variables. (e.g.
Here is the complete reworked example:
I will leave it up to you as an exercise whether you can further eliminate the duplicate code from
I prefer a simple cast over
as when I don't expect an object to be of any other type. If you do expect this, use as and be sure to check for null.var comboBox = (ComboBox)sender;-
When it makes sense to group code together do so.
if (!comboBox.IsDropDownOpen || !combobox.IsEditable || !combobox.IsReadOnly)-
Try to use LINQ where possible.
-
Place variables close to where they are being used. (
textSearch)-
Don't overuse
var. This is subjective, and there are several discussion about it. (also see comments in the code underneath)-
Using
continue can make it more difficult to get an overview of the code. In some occasions it is useful, but look at the code below to see how you could replace it with normal if else conditions instead.-
Identify redundant code, see what you can eliminate and what you can't. You might have to introduce new intermediate variables. (e.g.
textToCompare)Here is the complete reworked example:
private void OnTextComboBoxPreviewTextInput(object sender, TextCompositionEventArgs e)
{
var comboBox = (ComboBox)sender;
if (!comboBox.IsDropDownOpen || !combobox.IsEditable || !combobox.IsReadOnly)
{
return;
}
foreach (var item in comboBox.Items.Where( i => i != null ))
{
string textToCompare = null;
if (item is string)
{
textToCompare = (string)item;
}
else if (item is DependencyObject)
{
DependencyObject dependencyObjItem = (DependencyObject)item;
string textMember = TextSearch.GetText(dependencyObjItem);
if (!string.IsNullOrEmpty(textMember))
{
// From this sample, I don't know what the following type is!
// I wouldn't use var here.
var selectorFunc = ExpressionHelper.GetMemberFunction(dependencyObjItem, textMember);
textToCompare = selectorFunc(dependencyObjItem);
}
}
else
{
string textSearch = TextSearch.GetTextPath(comboBox);
if (!string.IsNullOrEmpty(textSearch))
{
var selectorFunc = ExpressionHelper.GetMemberFunction(item, textSearch);
textToCompare = selectorFunc(item);
}
}
if (textToCompare != null && textToCompare.StartsWith(e.Text, StringComparison.InvariantCultureIgnoreCase))
{
SelectItemInComboBox(comboBox, item);
break;
}
}
e.Handled = true;
}
private void SelectItemInComboBox(ComboBox comboBox, object item)
{
var comboBoxItem = comboBox.ItemContainerGenerator.ContainerFromItem(item) as ComboBoxItem;
comboBoxItem.IsSelected = true;
}I will leave it up to you as an exercise whether you can further eliminate the duplicate code from
selectorFunc, and whether it would be useful.Code Snippets
private void OnTextComboBoxPreviewTextInput(object sender, TextCompositionEventArgs e)
{
var comboBox = (ComboBox)sender;
if (!comboBox.IsDropDownOpen || !combobox.IsEditable || !combobox.IsReadOnly)
{
return;
}
foreach (var item in comboBox.Items.Where( i => i != null ))
{
string textToCompare = null;
if (item is string)
{
textToCompare = (string)item;
}
else if (item is DependencyObject)
{
DependencyObject dependencyObjItem = (DependencyObject)item;
string textMember = TextSearch.GetText(dependencyObjItem);
if (!string.IsNullOrEmpty(textMember))
{
// From this sample, I don't know what the following type is!
// I wouldn't use var here.
var selectorFunc = ExpressionHelper.GetMemberFunction(dependencyObjItem, textMember);
textToCompare = selectorFunc(dependencyObjItem);
}
}
else
{
string textSearch = TextSearch.GetTextPath(comboBox);
if (!string.IsNullOrEmpty(textSearch))
{
var selectorFunc = ExpressionHelper.GetMemberFunction(item, textSearch);
textToCompare = selectorFunc(item);
}
}
if (textToCompare != null && textToCompare.StartsWith(e.Text, StringComparison.InvariantCultureIgnoreCase))
{
SelectItemInComboBox(comboBox, item);
break;
}
}
e.Handled = true;
}
private void SelectItemInComboBox(ComboBox comboBox, object item)
{
var comboBoxItem = comboBox.ItemContainerGenerator.ContainerFromItem(item) as ComboBoxItem;
comboBoxItem.IsSelected = true;
}Context
StackExchange Code Review Q#5754, answer score: 7
Revisions (0)
No revisions yet.