Optimizing class that contains CAMLQuery, multitier architecture

Question

I'm new to developing sp projects , especially with multitier architecture, so I'm looking for some advices and suggestion how to optimize my code.I've got lot of "similar" queries that I need to perform , so my idea is to write method that will take query string as parametar and return SPListItemCollection, something like this:

Private static SPListItemCollection GetPhoneNumbers(string queryString)
    {
        using (SPSite currentSite = new SPSite(Models.Sol.SiteName))
        {
            using (SPWeb currentWeb = currentSite.OpenWeb())
            {
                SPList myList = currentWeb.Lists[Models.Sol.PB1.ListName];
                SPQuery query = new SPQuery();

                query.Query = queryString;
                return myList.GetItems(query);
            }
        }

    }

Then I have CAMLQueryBuilder class that I've created , and idea is to create static string field for each query I need (strings will be passed to GetPhoneNumbersmethod. Also I have 2 static properties List<string>Office getting list of offices and string Department.

I have 3 queries, first one should return all Employee numbers, second one should return all employee numbers from the employee office, and third employee numbers from employee office and department.Also , employee can work in 2 offices ,so that made this a little bit difficult. I've created static method for that purpose

public static string OfficeSearch(int n)
{

   if (n == 1)
   {
      return "<Eq><FieldRef Name='" + Models.Office + "'/><Value Type='Choice'>" + Office[n-1] + "</Value></Eq>";

   }           

      string s=  "<Or>";
      s += "<Eq><FieldRef Name='" + Models.Office + "'/><Value Type='Choice'>" + Office[n - 1] + "</Value></Eq>";
      s += OfficeSearch(n - 1);
      s += "</Or>";
      return s;

    }

And also I've created static field for some code that is repeating

public static string EmployeeNum ="<IsNotNull><FieldRef Name='" + Models.EmployeeName + "'/></IsNotNull>";

First query, returning all employee numbers :

public static string AllEmployee = "<Where>" + EmployeeNum + "</Where>";

Second query, returning employee numbers from same Office :

public static string EmployeeOffice = 
"<Where>" +
"<And>" +
EmployeeNum +  
OfficeSearch() +                                       
"</And>" +
"</Where>";

Third query , returning employee number from same office and department

public static string EmployeeOfficeDepartment =
"<Where>" +
"<And>" +
"<Eq><FieldRef Name='" + Models.Department + "'/><Value Type='Choice'>" +
 Department + "</Value></Eq>" +
 "<And>" +
 OfficeSearch() +
 EmployeeNum +
 "</And>" +
 "</And></Where>";

I believe that there are many different approaches that are more efficient , so please if you have any idea share it with me.Thank you.

Answer 1

You can make it art, or you can just accept your working solution and move on.

The more you optimize CAML generating code from engineering perspective, the more difficult it is for others (and you) to understand the implementation and make changes and fixes later. Now the queries are easy to read, so I'd prefer that over overly engineered CAML generator engine.

Answer 2

To keep it more lightweight you can pass on references for heavy objects already created.

For Instance:

Private static SPListItemCollection GetPhoneNumbers(string queryString, ref SPWeb currentWeb)
    {
        SPList myList = currentWeb.Lists[Models.Sol.PB1.ListName];
        SPQuery query = new SPQuery();    
        query.Query = queryString;
        return myList.GetItems(query);
    }

So any method calling GetPhoneNumbers can pass the reference of the already created object of that web, instead of creating 2 heavy objects as below

SPSite currentSite = new SPSite(Models.Sol.SiteName)
SPWeb currentWeb = currentSite.OpenWeb()

Answer 3

Your idea of mutualizing code is good IMO. I like your OfficeSearch method.
About your const, you should indent them for better reading.

Regarding the code itself now:

• Opening new SPSite/SPWeb objects is costly. Consider opening only once and passing these objects as parameters (but don't forget to close them at the end). Or, if you're in a Web context (Web part, page...), you can simply rely on the contextual objects (SPContext.Current.Web) that are already opened for you, and do not require disposing. Also, note they're under the identity of the current user.
• Instead of using: SPList myList = currentWeb.Lists[Models.Sol.PB1.ListName]; and thus rely on name of the list, use currentWeb.GetList(SPUrlUtility.CombineUrl(currentWeb.Url, listWebRelativeUrl));