8
\$\begingroup\$

There are three servers. I want to assign a job to a server which has least NotStarted jobs. I have achieved this, but my code is very lengthy. I don't know how to minimize my code without changing my concept. Here is my code:

public List<int> GetPaServer()
{
    List<int> PaServers = new List<int>();
    using (PaEntities pa = new PaEntities())
    {
        var PaServer = from server in pa.AppPM_Pa_Server
                          where server.IsActive == true
                          select server.ServerId;
        foreach (var paServer in PaServer)
        {
            PaServers.Add(paServer);
        }
    }
    return PaServers;
}


//Method to get the serverid for each request.
public int GetPaQueue()
{
    using (PaEntities server = new PaEntities())
    {
        List<int> Paserver = new List<int>();
        Paserver = GetPaServer();
        string server1 = string.Empty;
        string server2 = string.Empty;
        string server3 = string.Empty;
        foreach (int paserver in Paserver)
        {
            if (paserver == 1)
            {
                server1 = "active";
            }
            else if (paserver == 2)
            {
                server2 = "active";
            }
            else if (paserver == 3)
            {
                server3 = "active";
            }
        }
        int retVal = 0;

        // Get the Server NotStarted details here
        var NotStarted1 = (from serverID in server.AppPM_Pat
                           where serverID.Status == "NotStarted" && serverID.ServerId == 1
                           select serverID.ServerId).Count();
        var NotStarted2 = (from serverID in server.AppPM_Pat
                           where serverID.Status == "NotStarted" && serverID.ServerId == 2
                           select serverID.ServerId).Count();
        var NotStarted3 = (from serverID in server.AppPM_Pat
                           where serverID.Status == "NotStarted" && serverID.ServerId == 3
                           select serverID.ServerId).Count();

        // Get the  Server Started details here
        var server_Started1 = (from serverID in server.AppPM_Pat
                               where serverID.Status == "Started" && serverID.ServerId == 1
                               select serverID.ServerId).Count();
        var server_Started2 = (from serverID in server.AppPM_Pat
                               where serverID.Status == "Started" && serverID.ServerId == 2
                               select serverID.ServerId).Count();
        var server_Started3 = (from serverID in server.AppPM_Pat
                               where serverID.Status == "Started" && serverID.ServerId == 3
                               select serverID.ServerId).Count();

        //Get the server number for each  request
        //control comes here only when the server is active 
        if (server1 == "active" && server2 == "active" && server3 == "active")
        {
            if (NotStarted1 == 0 && NotStarted2 == 0 && NotStarted3 == 0)
            {
                if ((server_Started1 > server_Started2) && (server_Started1 > server_Started3))
                {
                    retVal = 2;
                }
                else if (server_Started1 == 0 && server_Started2 == 0 && server_Started3 == 0)
                {
                    retVal = 1;
                }
                else if (server_Started1 == 1 && server_Started2 == 1 && server_Started3 == 1)
                {
                    retVal = 1;
                }
                else if ((server_Started2 > server_Started1) && (server_Started2 > server_Started3))
                {
                    retVal = 1;
                }
                else if ((server_Started3 > server_Started1) && (server_Started3 > server_Started2))
                {
                    retVal = 1;
                }
                else if (server_Started1 == server_Started2 && server_Started3 == 0)
                {
                    retVal = 3;
                }
                else if (server_Started1 == server_Started3 && server_Started2 == 0)
                {
                    retVal = 2;
                }
                else if (server_Started2 == server_Started3 && server_Started1 == 0)
                {
                    retVal = 1;
                }
            }
            //control comes here only when the third server is active after some time
            else if (NotStarted1 == NotStarted2 && NotStarted3 == 0)
            {
                if (server_Started1 == 1 & server_Started2 == 1 & server_Started3 == 0)
                {
                    retVal = 3;
                }
                else if (server_Started1 == 1 & server_Started2 == 1 & server_Started3 == 1)
                {
                    retVal = 3;
                }
            }
            else if ((NotStarted1 > NotStarted2) && NotStarted3 == 0)
            {
                if (server_Started1 == 1 & server_Started2 == 1 & server_Started3 == 0)
                {
                    retVal = 3;
                }
                else if (server_Started1 == 1 & server_Started2 == 1 & server_Started3 == 1)
                {
                    retVal = 3;
                }
            }
            else if ((NotStarted2 > NotStarted1) && NotStarted3 == 0)
            {
                if (server_Started1 == 1 & server_Started2 == 1 & server_Started3 == 0)
                {
                    retVal = 3;
                }
                else if (server_Started1 == 1 & server_Started2 == 1 & server_Started3 == 1)
                {
                    retVal = 3;
                }
            }
            //control comes here only when the first server is active after some time
            else if (NotStarted2 == NotStarted3 && NotStarted1 == 0)
            {
                if (server_Started3 == 1 & server_Started2 == 1 & server_Started1 == 0)
                {
                    retVal = 1;
                }
                else if (server_Started1 == 1 & server_Started2 == 1 & server_Started3 == 1)
                {
                    retVal = 1;
                }
            }
            else if ((NotStarted2 > NotStarted3) && NotStarted1 == 0)
            {
                if (server_Started3 == 1 & server_Started2 == 1 & server_Started1 == 0)
                {
                    retVal = 1;
                }
                else if (server_Started1 == 1 & server_Started2 == 1 & server_Started3 == 1)
                {
                    retVal = 1;
                }
            }
            else if ((NotStarted3 > NotStarted2) && NotStarted1 == 0)
            {
                if (server_Started3 == 1 & server_Started2 == 1 & server_Started1 == 0)
                {
                    retVal = 1;
                }
                else if (server_Started1 == 1 & server_Started2 == 1 & server_Started3 == 1)
                {
                    retVal = 1;
                }
            }
            //control comes here only when the second server is active after some time
            else if (NotStarted3 == NotStarted1 && NotStarted2 == 0)
            {
                if (server_Started3 == 1 & server_Started1 == 1 & server_Started2 == 0)
                {
                    retVal = 2;
                }
                else if (server_Started1 == 1 & server_Started2 == 1 & server_Started3 == 1)
                {
                    retVal = 2;
                }
            }
            else if ((NotStarted3 > NotStarted1) && NotStarted2 == 0)
            {
                if (server_Started3 == 1 & server_Started1 == 1 & server_Started2 == 0)
                {
                    retVal = 2;
                }
                else if (server_Started1 == 1 & server_Started2 == 1 & server_Started3 == 1)
                {
                    retVal = 2;
                }
            }
            else if ((NotStarted1 > NotStarted3) && NotStarted2 == 0)
            {
                if (server_Started3 == 1 & server_Started1 == 1 & server_Started2 == 0)
                {
                    retVal = 2;
                }
                else if (server_Started1 == 1 & server_Started2 == 1 & server_Started3 == 1)
                {
                    retVal = 2;
                }
            }
            else if (NotStarted1 == 1 && NotStarted2 == 0 && NotStarted3 == 0)
            {
                if ((server_Started1 > server_Started2) && (server_Started1 > server_Started3))
                {
                    retVal = 2;
                }
                else if (server_Started1 == 0 && server_Started2 == 0 && server_Started3 == 0)
                {
                    retVal = 1;
                }
                else if (server_Started1 == 1 && server_Started2 == 1 && server_Started3 == 1)
                {
                    retVal = 1;
                }
                else if ((server_Started2 > server_Started1) && (server_Started2 > server_Started3))
                {
                    retVal = 1;
                }
                else if ((server_Started3 > server_Started1) && (server_Started3 > server_Started2))
                {
                    retVal = 1;
                }
                else if (server_Started1 == server_Started2 && server_Started3 == 0)
                {
                    retVal = 3;
                }
                else if (server_Started1 == server_Started3 && server_Started2 == 0)
                {
                    retVal = 2;
                }
                else if (server_Started2 == server_Started3 && server_Started1 == 0)
                {
                    retVal = 1;
                }
            }
            else if (NotStarted1 == 1 && NotStarted2 == 1 && NotStarted3 == 0)
            {
                if ((server_Started1 > server_Started2) && (server_Started1 > server_Started3))
                {
                    retVal = 2;
                }
                else if (server_Started1 == 0 && server_Started2 == 0 && server_Started3 == 0)
                {
                    retVal = 1;
                }
                else if (server_Started1 == 1 && server_Started2 == 1 && server_Started3 == 1)
                {
                    retVal = 1;
                }
                else if ((server_Started2 > server_Started1) && (server_Started2 > server_Started3))
                {
                    retVal = 1;
                }
                else if ((server_Started3 > server_Started1) && (server_Started3 > server_Started2))
                {
                    retVal = 1;
                }
                else if (server_Started1 == server_Started2 && server_Started3 == 0)
                {
                    retVal = 3;
                }
                else if (server_Started1 == server_Started3 && server_Started2 == 0)
                {
                    retVal = 2;
                }
                else if (server_Started2 == server_Started3 && server_Started1 == 0)
                {
                    retVal = 1;
                }
            }
            else if (NotStarted1 > NotStarted2 && NotStarted1 > NotStarted3)
            {
                if (server_Started1 == 1 && server_Started2 == 1 && server_Started3 == 1)
                {
                    if (NotStarted2 > NotStarted3)
                    {
                        retVal = 3;
                    }
                    else
                    {
                        retVal = 2;
                    }
                }
            }
            else if (NotStarted2 > NotStarted3 && NotStarted2 > NotStarted1)
            {
                if (server_Started1 == 1 && server_Started2 == 1 && server_Started3 == 1)
                {
                    if (NotStarted1 > NotStarted3)
                    {
                        retVal = 3;
                    }
                    else
                    {
                        retVal = 1;
                    }
                }
            }
            else if (NotStarted3 > NotStarted1 && NotStarted3 > NotStarted2)
            {
                if (server_Started1 == 1 && server_Started2 == 1 && server_Started3 == 1)
                {
                    if (NotStarted1 > NotStarted2)
                    {
                        retVal = 2;
                    }
                    else
                    {
                        retVal = 1;
                    }
                }
            }
            else if (NotStarted1 == NotStarted2 && NotStarted2 == NotStarted3 && NotStarted1 == NotStarted3)
            {
                if (server_Started1 == 1 && server_Started2 == 1 && server_Started3 == 1)
                {
                    retVal = 1;
                }
            }
            else if (NotStarted1 == NotStarted2 && NotStarted1 > NotStarted3 && NotStarted2 > NotStarted3)
            {
                if (server_Started1 == 1 && server_Started2 == 1 && server_Started3 == 1)
                {
                    retVal = 3;
                }
            }
            else if (NotStarted2 == NotStarted3 && NotStarted2 > NotStarted1 && NotStarted3 > NotStarted1)
            {
                if (server_Started1 == 1 && server_Started2 == 1 && server_Started3 == 1)
                {
                    retVal = 1;
                }
            }
            else if (NotStarted1 == NotStarted3 && NotStarted1 > NotStarted2 && NotStarted3 > NotStarted2)
            {
                if (server_Started1 == 1 && server_Started2 == 1 && server_Started3 == 1)
                {
                    retVal = 2;
                }
            }                    
        }
        //control comes here only when server1 and server2 is active
        else if (server1 == "active" && server2 == "active")
        {
            if (NotStarted1 > NotStarted2)
            {
                if (server_Started1 == 1 && server_Started2 == 1)
                {
                    retVal = 2;
                }
            }
            else if (NotStarted1 == NotStarted2)
            {
                if (server_Started1 == 0 && server_Started2 == 0)
                {
                    retVal = 1;
                }
                else if (server_Started1 == 1 && server_Started2 == 0)
                {
                    retVal = 2;
                }
                else if (server_Started1 == 0 && server_Started2 == 1)
                {
                    retVal = 1;
                }
                else if (server_Started1 == 1 && server_Started2 == 1)
                {
                    retVal = 1;
                }
            }
            else
            {
                if (server_Started1 == 1 && server_Started2 == 1)
                {
                    retVal = 1;
                }
            }
        }
        //control comes here only when server3 and server2 is active
        else if (server2 == "active" && server3 == "active")
        {
            if (NotStarted2 > NotStarted3)
            {
                if (server_Started2 == 1 && server_Started3 == 1)
                {
                    retVal = 3;
                }
            }
            else if (NotStarted2 == NotStarted3)
            {
                if (server_Started2 == 0 && server_Started3 == 0)
                {
                    retVal = 2;
                }
                else if (server_Started2 == 1 && server_Started3 == 0)
                {
                    retVal = 3;
                }
                else if (server_Started2 == 0 && server_Started3 == 1)
                {
                    retVal = 2;
                }
                else if (server_Started2 == 1 && server_Started3 == 1)
                {
                    retVal = 2;
                }
            }
            else
            {
                if (server_Started2 == 1 && server_Started3 == 1)
                {
                    retVal = 2;
                }
            }
        }
        //control comes here only when server1 and server3 is active
        else if (server1 == "active" && server3 == "active")
        {
            if (NotStarted1 > NotStarted3)
            {
                if (server_Started1 == 1 && server_Started3 == 1)
                {
                    retVal = 3;
                }
            }
            else if (NotStarted1 == NotStarted3)
            {
                if (server_Started1 == 0 && server_Started3 == 0)
                {
                    retVal = 1;
                }
                else if (server_Started1 == 1 && server_Started3 == 0)
                {
                    retVal = 3;
                }
                else if (server_Started1 == 0 && server_Started3 == 1)
                {
                    retVal = 1;
                }
                else if (server_Started1 == 1 && server_Started3 == 1)
                {
                    retVal = 1;
                }
            }
            else
            {
                if (server_Started1 == 1 && server_Started3 == 1)
                {
                    retVal = 1;
                }
            }
        }
        return retVal;
    }
}
\$\endgroup\$
0

2 Answers 2

10
\$\begingroup\$

A few simple things before getting to the meat of the problem.

Method variables start with lowercase letters. There are a number of cases where you are not consistent with this and it will confuse other people that read your code. For the same reason, you should be consistent with camelCase of underscore_separated variable names.

Don't create a new List instance if you are going to assign the result of a function to that variable on the next line.

List<int> Paserver = new List<int>();
Paserver = GetPaServer();

This can all just be one line of code.

Abstract away repeated functionality in methods. The code you are using to create NotStarted# and server_Started# only changes based on the id and status string.

private int serverJobCount(PaEntities server, int id, string status) {
  return (from serverID in server.AppPM_Pat
       where serverID.Status == status && serverID.ServerId == id
       select serverID.ServerId).Count();
}

Now to your actual question. As you have seen, the process for doing this gets complicate when there are three servers. And it would get even longer if you had 4. The good news is that there is already a way to solve this for any number of servers. You should think of this problem as if you gave a value to each server and that value represented how much that server should be favoured to get a new job. Once you have that, all you need to do is sort those vales and the first server in the list is the one you should schedule the job with.

To make this easier, lets first make a class that represents a server. This will remove the need for all of your thing# and otherThing# variables.

public class Server : IComparable {
  //It looks like the server can only have one started job, so I made this boolean.
  public bool Started { get; private set; }
  public int NotStarted { get; private set; }

  public Server(bool started, int notStarted) {
    Started = started;
    NotStarted = notStarted;
  }

  //This is where the magic happens
  public int CompareTo(object obj) {
    Server other = obj as Server;
    if (other != null) {
      //This is where you put your ordering logic. I filled in values to give you a feel.
      //You should ensure the logic is correct. I also noticed that the posted code did not cover all cases.
      if (NotStarted < other.NotStarted) {
        return -1;
      } else if (NotStarted == other.NotStarted) {
        if (Started && !other.Started) {
          return -1;
        } else if (Started == other.NotStarted) {
          return 0;
        } else {
          return 1;
        }
      } else {
        return 1;
      }
    } else {
     throw new ArgumentException("Object is not a Server");
    }
  }
}

Now this is all you have to do to get the next server id.

public int getServerToSchedule() {
  using (PaEntities server = new PaEntities()) {
    List<Server> servers = new List<Server>();
    foreach (int paServer in GetPaServer()) {
    servers.Add(new Server(serverJobCount(server, paServer, "Started") == 1,
        serverJobCount(server, paServer, "NotStarted")));
    servers.Sort()
    return servers.First();
  }
}
\$\endgroup\$
6
\$\begingroup\$

Use arrays to hold your servers, then loop over the arrays instead of repeating the same code for each server. In the loop you have a server AND its index, so you can omit changing just one number and the server while copying around the code. Rinse and repeat for NotStarted and the other values you calculate for each server.

Furthermore, it looks like you have an unrolled sorting algorithm in your code. If you combine a server and its NotStarted and other calculated values into a struct, you can just use an array of struct values and then sort it with a custom sorting function. Or use a LINQ expression. I'm still seven hours away from my dev PC, maybe I can hack something together this evening...

\$\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.