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

Code optimization by constant if-structure iteration removal

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

Problem

Below you will find the specific code that I am attempting to optimize. I have a method previous to called getCount() which, based on similar parameters as below, calculates how many record sets of 75 will be pulled based on one of the below query.

The Query that is executed is based on the department, projLead and status string while projSet is used to skip records.

So basically, count returns the number of iterations that will need to be performed to obtain the full result set. An ajax call inside a for loop iterates until i > the last projSet() and calls the below method.

The problem with this is that every time it gets the next sub result set of 75 records it needs to go through the if structure below. Is there a way to basically have the below method determine which query to use, return the query as a String or something and then in another method pass the queryString and then run it as a query until the number of sets is greater than the total number of sets requiring pulling? This way, the if structure is just used to determine the query which can be passed into another method with just the query being pass and run inside it and it returns the result sets.

The iterative part would be handled via ajax, but I do not know how to return the query and have it passed into another method through another ajax call where it just executes the query.

```
public ActionResult ProjList(String department, String projLead, String status, int projSet)
{
//Query where all filters are applied
var query = db.LatestStatus.Where(x => (x.Department.CompareTo(department) == 0)
&& (x.ProjectLeader.CompareTo(projLead) == 0)
&& (x.Status.CompareTo(status) == 0))
.OrderBy(x => x.ProjectID).Skip(projSet * 75).Take(75).ToList();

//Check to see which filters are not applied
// - > all filters are not applied
// - > find which filters are applied and return it's query
if (de

Solution

Besides the two answers already given, I'd also suggest the following.

Rename projLead to projectLeader. Same for projSet.

But most importantly, I'd rethink your entire if. I suspect there are several errors -- department.Equals("Status")? department.Equals("Project Leader")? -- but more importantly I cannot figure out the logic and I suspect you're missing plenty of cases. Try to document the behavior you expect to see, and implement that as simple as possible.

Your method has four parameters, which IMHO is starting to become unwieldy. Can't you pass a class like this?

public class SearchParameters
{
    public string Department {get;set;}
    public string ProjectLeader {get;set;}
    public string Status {get;set;}
    public int ProjSet {get;set;}
}


Don't mix String and int; be consistent.

Code Snippets

public class SearchParameters
{
    public string Department {get;set;}
    public string ProjectLeader {get;set;}
    public string Status {get;set;}
    public int ProjSet {get;set;}
}

Context

StackExchange Code Review Q#69848, answer score: 5

Revisions (0)

No revisions yet.