HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

EntityFramework-based filtering

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
entityframeworkbasedfiltering

Problem

I am trying to use the following form to enter new info into a database.

I am trying to use the entity framework.

I have the following classes of Interest:

public class InventoryContext : DbContext
{
    public DbSet Items { get; set; }

    ....Other DbSets follow...
}

[Table("Items")]
public class Item
{
    #region Strings
    public string Color { get; set; }
    public string FullName { get; set; }
    [Column(@"Sheet/Roll")]
    public string Type { get; set; }
    public string PrimaryMachine { get; set; }
    public string Alias { get; set; }
    public string Brand { get; set; }
    public string Finish { get; set; }

    #endregion

    #region Long
    public long ID { get; set; }
    public decimal? Weight { get; set; }
    #endregion

    #region Doubles
    public decimal? Size1 { get; set; }
    public decimal? Size2 { get; set; }
    public decimal? Size3 { get; set; }
    #endregion.
}


And I am stuck on filtering the dgvAllItems, based on the selected drop down values. I think my code is ugly and there is a better way.

```
private void ComboBox_SelectedIndexChanged(object sender, EventArgs e)
{

ComboBox s = (ComboBox)sender;

IQueryable query = c.Items;

foreach (ComboBox cb2 in gbFilters.Controls.OfType().Where(com => com.Text != ""))
{
string currentpropname = cb2.Name.Substring(2);

if (cb2.Name.Substring(2, 4) == "Size" || cb2.Name == "cbWeight")
{
decimal? currentpropvalue = Convert.ToDecimal(cb2.Text);
PropertyInfo propertyInfo = typeof(Item).GetProperty(currentpropname);
ParameterExpression pe = Expression.Parameter(typeof(Item), "e");
MemberExpression me = Expression.MakeMemberAccess(pe, propertyInfo);
ConstantExpression ce = Expression.Constant(currentpropvalue, typeof(decimal?));
BinaryExpression be = Expression.Equal(me, ce);

Expression>

Solution

I'm about to watch Vikings so here are a few quick pointers:

Naming

Your code contains variables named me, pe, ce, etc. This tells me nothing about what that variable does.

I'm not experienced with Expressions but I assume Expression.Equal refers to an == invocation. Such a variable could then be rewritten as equalsCondition, for example. This tells me a lot more about will also read easier when done everywhere:

expression = equalsCondition(itemValue, constantValue)


Duplication

Your if statements are very similar to eachother; similar to the point that I'm pretty sure you copy-pasted most of it. This is a sign that you need to factor it out to a method.

An example could be this:

private Expression> lambda GenerateQuery(object propertyValue)
{
    PropertyInfo propertyInfo = typeof(Item).GetProperty(currentpropname);
    ParameterExpression pe = Expression.Parameter(typeof(Item), "e");
    MemberExpression me = Expression.MakeMemberAccess(pe, propertyInfo);
    ConstantExpression ce = Expression.Constant(currentpropvalue, typeof(decimal?));
    BinaryExpression be = Expression.Equal(me, ce);
    Expression> lambda = Expression.Lambda>(be, pe);
}


and used as such:

if (cb2.Name.Substring(2, 4) == "Size" || cb2.Name == "cbWeight")
{
    decimal? currentpropvalue = Convert.ToDecimal(cb2.Text); 
    query = query.Where(GenerateQuery(currentpropvalue));
} else {
    string currentpropvalue = cb2.Text;
    query = query.Where(GenerateQuery(currentpropvalue));
}


If you push these two changes through to the several places they manifest, you'll find that your code has shrunk a lot and is a lot more readable.

Code Snippets

expression = equalsCondition(itemValue, constantValue)
private Expression<Func<Item, bool>> lambda GenerateQuery(object propertyValue)
{
    PropertyInfo propertyInfo = typeof(Item).GetProperty(currentpropname);
    ParameterExpression pe = Expression.Parameter(typeof(Item), "e");
    MemberExpression me = Expression.MakeMemberAccess(pe, propertyInfo);
    ConstantExpression ce = Expression.Constant(currentpropvalue, typeof(decimal?));
    BinaryExpression be = Expression.Equal(me, ce);
    Expression<Func<Item, bool>> lambda = Expression.Lambda<Func<Item, bool>>(be, pe);
}
if (cb2.Name.Substring(2, 4) == "Size" || cb2.Name == "cbWeight")
{
    decimal? currentpropvalue = Convert.ToDecimal(cb2.Text); 
    query = query.Where(GenerateQuery(currentpropvalue));
} else {
    string currentpropvalue = cb2.Text;
    query = query.Where(GenerateQuery(currentpropvalue));
}

Context

StackExchange Code Review Q#48435, answer score: 4

Revisions (0)

No revisions yet.