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

Applying different filters within loops

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

Problem

What I have is an outer loop which loops through a list of strings, followed by another loop which loops through a list of filters; and if the current column is the current filter in the list of filters, it then applies this filter.

I'm just trying to tidy up a messy function but can't see anyway of doing so.

```
public static bool PassRules(ListcolumnsList, List customfilters )
{
var i = 0;
foreach (var column in columnsList)
{
var curFilter = new Customfilters();

foreach (var customfilterse in customfilters.Where(customfilterse => customfilterse.Field.Equals(Filed[i])))
{
curFilter = customfilterse;
}

if ( !string.IsNullOrEmpty(curFilter.Filter))
{
int n;
var isNumeric = int.TryParse(column, out n);
if (isNumeric)
{
if (!Compare(curFilter.Expression, Convert.ToDouble(column), Convert.ToDouble(curFilter.Filter)))
{
return false;
}
}
else
{
if (curFilter.Expression.Equals("IN") || curFilter.Expression.Equals("NOT IN"))
{
var toList = curFilter.Filter.Split(Convert.ToChar(","));

if (!CompairInNotIn(toList.ToList(), curFilter.Expression, column))
{
return false;
}
}
else
{
if (!Compare(curFilter.Expression, column, curFilter.Filter))
{
return false;
}
}
}
}
i++;
}
return true;
}

public static bool Compare(string op, T left, T right) where T : IComparable
{
switch (op)
{
case "": return left.CompareTo(right) > 0;
case "=": return left.CompareTo(right) >= 0;
case "==": return left.Equals(right);

Solution

Can't you change:

if ( !string.IsNullOrEmpty(curFilter.Filter)) {
  ..Lots of code..
}


to

if ( string.IsNullOrEmpty(curFilter.Filter)) {
  i++;
  continue;
}


?
This saves you one indent. (The else clause becomes redundant)

Also I would most likely swap around the if statements for isNumeric and the IN and NOT IN (and their corresponding contents). If only because the two function calls:

if (!Compare(curFilter.Expression, Convert.ToDouble(column), Convert.ToDouble(curFilter.Filter))) {
  return false;
}


and

if (!Compare(curFilter.Expression, column, curFilter.Filter)) {
  return false;
}


look very similar, so imo it's better to group them together. (Though this is mere aesthetics)

Other than that I'm looking at some more fundamental changes if you really want to tidy up the structure. But that goes beyond the scope of this post. (I'm at work at the moment, I know, bad excuse. See what other people have to say.)

Code Snippets

if ( !string.IsNullOrEmpty(curFilter.Filter)) {
  ..Lots of code..
}
if ( string.IsNullOrEmpty(curFilter.Filter)) {
  i++;
  continue;
}
if (!Compare(curFilter.Expression, Convert.ToDouble(column), Convert.ToDouble(curFilter.Filter))) {
  return false;
}
if (!Compare(curFilter.Expression, column, curFilter.Filter)) {
  return false;
}

Context

StackExchange Code Review Q#62301, answer score: 3

Revisions (0)

No revisions yet.