Search query utilizing menus and checkboxes

Question

I'm dealing with a searching query where the user will pick from menus and checkboxes the variables-criteria I'm passing to the toWeb method and a list will be returned. This works fine but I'm going to add two more variables and it's already hard to follow.

 public List<newEvent> toWeb(string name, string egk, string 
 typosSumvan, Boolean olaTaSumvanta)
 {
 List<newEvent> listaSumvantwn = new List<newEvent>();
 if (olaTaSumvanta == false)
 {
 if (egk != "")
 {
 if (typosSumvan != "")
 {
 using (var db = new CMMSEntity())
 {
 foreach (var a in db.sumvanta)
 {
 if (a.apodektis == name && a.egkatastasi == egk && a.eidosSumvan == typosSumvan)
 {
 listaSumvantwn.Add(readFromTable(a));
 }
 }
 }
 }
 else
 {
 using (var db = new CMMSEntity())
 {
 foreach (var a in db.sumvanta)
 {
 if (a.apodektis == name && a.egkatastasi == egk)
 {
 listaSumvantwn.Add(readFromTable(a));
 }
 }
 }
 }
 }
 else
 {
 if (typosSumvan != "")
 {
 using (var db = new CMMSEntity())
 {
 foreach (var a in db.sumvanta)
 {
 if (a.apodektis == name && a.eidosSumvan == typosSumvan)
 {
 listaSumvantwn.Add(readFromTable(a));
 }
 }
 }
 }
 else
 {
 using (var db = new CMMSEntity())
 {
 foreach (var a in db.sumvanta)
 {
 if (a.apodektis == name)
 {
 listaSumvantwn.Add(readFromTable(a));
 }
 }
 }
 }
 }
 }
 else
 {
 if (egk != "")
 {
 if (typosSumvan != "")
 {
 using (var db = new CMMSEntity())
 {
 foreach (var a in db.sumvanta)
 {
 if (a.egkatastasi == egk && a.eidosSumvan == typosSumvan)
 {
 listaSumvantwn.Add(readFromTable(a));
 }
 }
 }
 }
 else
 {//anazitisi xwris tupo
 //anazitisi xwris tupo sumvan
 using (var db = new CMMSEntity())
 {
 foreach (var a in db.sumvanta)
 {
 if (a.egkatastasi == egk)
 {
 listaSumvantwn.Add(readFromTable(a));
 }
 }
 }
 }
 }
 else
 { 
 if (typosSumvan != "")
 {
 using (var db = new CMMSEntity())
 {
 foreach (var a in db.sumvanta)
 {
 if (a.eidosSumvan == typosSumvan)
 {
 listaSumvantwn.Add(readFromTable(a));
 }
 }
 }
 }
 else
 {
 using (var db = new CMMSEntity())
 {
 foreach (var a in db.sumvanta)
 {
 listaSumvantwn.Add(readFromTable(a));
 }
 }
 }
 }
 }
 return listaSumvantwn;
} 

• Heslacher: What type are the items of db.sumvanta ?
• Oh hi Mark: db.sumvanta is an sql table will strings and datetime cells mostly.
• Heslacher: so var a in db.sumvanta refers to a DataRow ?
• Oh hi Mark: exactly a holds the name of the cells and i'm passing them on an object, but that's for another post!
• Jamal: Please do not edit "thanks" into a post. Just upvotes will do.

Answer 1

Emmm... can't you just join your bool conditions? Or am I missing something?

//method names and class names should start with capital letter
public List<NewEvent> ToWeb(string name, string egk, string typosSumvan, Boolean olaTaSumvanta)
{
 List<NewEvent> listaSumvantwn = new List<NewEvent>();
 using (var db = new CMMSEntity())
 {
 foreach (var a in db.sumvanta)
 {
 if (!olaTaSumvanta && a.apodektis != name) continue;
 if (!String.IsNullOrEmpty(egk) && a.egkatastasi != egk) continue;
 if (!String.IsNullOrEmpty(typosSumvan) && a.eidosSumvan != typosSumvan)) continue;
 listaSumvantwn.Add(ReadFromTable(a));
 }
 }
 return listaSumvantwn;
}

Or using LINQ

public List<NewEvent> ToWeb(string name, string egk, string typosSumvan, Boolean olaTaSumvanta)
{
 using (var db = new CMMSEntity())
 {
 return db.sumvanta.Where(a => olaTaSumvanta || a.apodektis == name)
 .Where(a => String.IsNullOrEmpty(egk) || a.egkatastasi == egk)
 .Where(a => String.IsNullOrEmpty(typosSumvan) || a.eidosSumvan == typosSumvan)
 .Select(a => ReadFromTable(a))
 .ToList();
 }
}

Also you should consider using English language for naming purposes.

• t3chb0t: Almost everything is fine but the lengthy condition inside the if is a no-go ;-]
• Nikita B: Not as lengthy, as original if chain though ^^
• Oh hi Mark: Thank you for your answer, I saw you updated and added the continue keyword. Why is that? It works without it.
• Nikita B: Personal preference. Feel free to use the version you like, they do exactly the same thing. :)

Answer 2

Each of your parameters is either adding a filter, or an ignored value. We can construct a list of Predicates, each one corresponding to a single parameter.

 public List<newEvent> toWeb(string name, string egk, string 
 typosSumvan, Boolean olaTaSumvanta)
 {
 var predicates = new List<Predicate<DataRow>>();
 if (!olaTaSumvanta) { predicates.Add(row => row.apodektis == name) }
 if (egk != "") { predicates.Add(row => row.egkatastasi == egk) }
 if (typosSumvan != "") { predicates.Add(row => row.eidosSumvan == typosSumvan) }
 using (var db = new CMMSEntity())
 {
 foreach (var a in db.sumvanta)
 {
 if (a.IsValidEntity(predicates))
 {
 listaSumvantwn.Add(readFromTable(a));
 }
 }
 }
}

Using an extension method taken from this answer, we then apply all the filters.

public static Boolean IsValidEntity<T>(this T entity, IEnumerable<Predicate<T>> predicates)
{
 return predicates.All(p => p(entity));
}

Answer 3

Take a step back and look at the structure of the code and look for a repeating pattern. If you do that, you will see that following:

using (var db = new CMMSEntity())
{
 foreach (var a in db.sumvanta)
 {
 if (SOME BOOLEAN EXPRESSION INVOLVING a)
 {
 listaSumvantwn.Add(readFromTable(a));
 }
 }
}

Extracting this into a sub function will greatly improve this code.

private void SomeGoodFunctionName(Func<TYPE OF a, bool> shouldAdd)
{
 using (var db = new CMMSEntity())
 {
 foreach (var a in db.sumvanta)
 {
 if (shouldAdd(a))
 {
 listaSumvantwn.Add(readFromTable(a));
 }
 }
 }
}

Which is then used like the following to never be nested more than 3 levels.

if (olaTaSumvanta == false)
{
 if (egk != "")
 {
 if (typosSumvan != "")
 {
 SomeGoodFunctionName(a => a.apodektis == name && a.egkatastasi == egk && a.eidosSumvan == typosSumvan);
 }

Since the boolean expressions are some what complicated, it would be better to create stand alone functions with names describing what is being checked. Then use that instead of a long lambda expression.

Note: In the last repeated block, there is no if block inside the foreach loop. This is equivalent to the boolean expression being true.

Answer 4

Let's assume the elements of the db.sumvanta are of type Record.
You could move creation of the resulting List to the method:

public static List<newEvent> GetRecords(Func<Record, bool> match)
{
 List<newEvent> listaSumvantwn = new List<newEvent>();
 using (var db = new CMMSEntity())
 {
 foreach (var a in db.sumvanta)
 {
 if (match(a))
 {
 listaSumvantwn.Add(readFromTable(a));
 }
 }
 }
 return listaSumvantwn;
}

Then your method turns into the following:

public List<newEvent> toWeb(string name, string egk, string typosSumvan, Boolean olaTaSumvanta)
{
 if (!olaTaSumvanta)
 {
 if (egk != "")
 {
 return typosSumvan != ""
 ? GetRecords(a => a.apodektis == name && a.egkatastasi == egk && a.eidosSumvan == typosSumvan)
 : GetRecords(a => a.apodektis == name && a.egkatastasi == egk);
 }
 return typosSumvan != ""
 ? GetRecords(a => a.apodektis == name && a.eidosSumvan == typosSumvan)
 : GetRecords(a => a.apodektis == name);
 }
 if (egk != "")
 {
 return typosSumvan != ""
 ? GetRecords(a => a.egkatastasi == egk && a.eidosSumvan == typosSumvan)
 : GetRecords(a => a.egkatastasi == egk);
 }
 return typosSumvan != ""
 ? GetRecords(a => a.eidosSumvan == typosSumvan)
 : GetRecords(a => true);
}