0
\$\begingroup\$

Here is my issue.I have two classes:

public class Role
{
  public string RoleTitle { get; set; }
  public string Office { get; set; }
  public string Department { get; set; }
}

And

public class Employe
{
  public string Name { get; set; }
  public List<Role> Roles { get; set; }
}

As you can see, class Employee contains ListRoles as a property.I have one method that process some share point list and returns one Employee object with all of his roles, something like this:

John Johnson

  • Regular Office1 Department2

  • Regular Office2 Department1

  • Manager Office1 Department3

  • OfficeDirector Office1 Departmnet4 etc.

As you can see one employee can have lot of different roles and lot of same roles but for different office/department (OK, I know that this is not real life situation, I'm just using this as an example). Now I'm using this Role information to process another SP List to get more information that I need to display.Roles are very important, because queries are based on them, what employee can see or not.Also, office and department are crucial information because rules for roles are also based on them.

My current code looks like this, I'm going through foreach loop and for each role, which I determine on RoleTitle, I create query, which will be added to final query (using OR).

public void GetEmployeeInformation(Employee emp)
{
    query = string.Empty;
    List<string> queries = new List<string>();
    foreach (Role role in emp.Roles)
    {
        switch (role.RoleTitle)
        {
            case ("Regular"):
                {
                    query = CamlQuery(role.Office, role.Department, "Office", "Department", "Choice");
                    break;
                    }

                case ("Manager"):
                    {
                        query = CamlQuery("Department", role.Department);
                        break;
                    }
                case ("OfficeDirector"):
                    {
                        query = CamlQuery("Office", role.Offices);
                        break;
                    }
                case ("Admin"):
                    {
                        query = CamlQuery("some implementation")
                        break;
                    }
            }
            queries.Add(query);
        }
//further implementation of method

    }

Everything is working fine, I got right results, but I'm wondering is there any chance that I can optimize code, because, for example, if I have 5 regular roles for each one it will go through the loop.

Another idea I got is to make changes in Role class, and create ListOffices, and ListDepartments so when I query in first method, I will get, for each different role, something like this

Manager:

  • Office1 Department1
  • Office2 Department2
  • Office3 Department3

Now, I will have only one Role object for every role with all office/department information inside, and I can go through foreach and inside Camlquery use one recursive method that will process all offices/departments for that role and create final query.The recursive method I've created is based on indexes of elements inside the lists, because when I create query I know that for first Manager role office/department pair is on [0] position in the list.But I've been told that using indexes in situation like this are very bad practice, so I've been stucked with this idea.

Do you have any idea how can I optimize this code by avoiding going through foreach loop for every single role and not using this approach with Lists and indexes?

\$\endgroup\$
  • 2
    \$\begingroup\$ Welcome to code review. I have to say a couple things about your question. This question may be on the threshold of the scope of code-review since you are in some way asking for proposals of approaches that you should take. In the end you should only expect to have code that you include on your question to be reviewed too. However we can still proceed by reviewing GetEmployeeInformation. The question itself seems interesting though. Should you for example complement it with the inclusion of CamlQuery? \$\endgroup\$ – Bruno Costa Jan 31 '16 at 19:35
  • 2
    \$\begingroup\$ //further implementation of method Please add the rest of the implementation when possible. More context gives better answers. \$\endgroup\$ – Mast Jan 31 '16 at 20:20
  • \$\begingroup\$ Is this an example of some other "real" code you have elsewhere, or is this an example like "this is a learning exercise" example? \$\endgroup\$ – RubberDuck Jan 31 '16 at 22:19
2
\$\begingroup\$

You can use linq GroupBy and Select to reduce your list to distinct RoleTitle's in your foreach loop:

foreach(Role role in emp.Roles.GroupBy(r=> r.RoleTitle).Select(g => g.First()))
{
    //...
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.