Refactoring methods that use the same code but different methods inside

Question

I have some methods that are the same except one row (i call different methods on the object client). I will have more methods like this.

Is there any solution to this except using Reflection ?

 private void initClerks(Client client)
            {
                string[] pks = client.ClerksPKS.Trim(','). Split(',');

                foreach (string pk in pks)
                {
                    string data = JObject.Parse(DBUtils.GetData(Constants.DBProcedures.GetProcedures.GetWorkerDetailsByPkid, pk))[Constants.ResponseJson.Data].ToString();

                    client.addClerk(JsonConvert.DeserializeObject<Clerk[]>(data)[0]);

                }

            }


private void initManagers(Client client)
            {
                string[] pks = client.ManagerPK.Trim(',').Split(',');



                foreach (string pk in pks)
                {
                    string data = JObject.Parse(DBUtils.GetData(Constants.DBProcedures.GetProcedures.GetWorkerDetailsByPkid, pk))[Constants.ResponseJson.Data].ToString();
                    client.addManager(JsonConvert.DeserializeObject<Manager[]>(data)[0]);

                }

            }

Answer 1

The typical path for extracting a method is to find the differences and declare a corresponding method arguments. Let take your initClerks method and find the word Clerk inside. There are 3 of them, one being a Type, so we'll make a generic method with generic argument T corresponding to Clerk. The mapping will be something like this

(1) client.ClerksPKS maps to Func<Client, string>
(2) JsonConvert.DeserializeObject<Clerk[]> maps to JsonConvert.DeserializeObject<T[]>
(3) client.addClerk maps to Action<Client, T>

So the common method becomes

void Init<T>(Client client, Func<Client, string> getPKS, Action<Client, T> addItem)
{
    string[] pks = getPKS(client).Trim(','). Split(',');
    foreach (string pk in pks)
    {
        string data = JObject.Parse(DBUtils.GetData(Constants.DBProcedures.GetProcedures.GetWorkerDetailsByPkid, pk))[Constants.ResponseJson.Data].ToString();
        addItem(client, JsonConvert.DeserializeObject<T[]>(data)[0]);
    }
}

and the usage

private void initClerks(Client client)
{
    Init<Clerk>(client, c => c.ClerksPKS, (c, x) => c.addClerk(x));
}

private void initManagers(Client client)
{
    Init<Manager>(client, c => c.ManagerPK, (c, x) => c.addManager(x));
}

Answer 2

Taking into account this is not external legacy lib I would suggest refactoring Client class as well to simplify its API (I would change more but let's stop at some point)

pseudocode:

// taking into account Client, manager all are workers
class Client
{
    // further whenever you need filter out managers use LINQ OfType<>
    List<Workers> workers;

    public void Add<T>(T worker) where T: Worker
    {
        workers.Add(client);
    }
}

See Extract method approach,

private void Initialize<T>(Client client, string[] pks)
{
    foreach (string pk in pks)
    {
        string data = JObject.Parse(DBUtils.GetData(Constants.DBProcedures.GetProcedures.GetWorkerDetailsByPkid, pk))[Constants.ResponseJson.Data].ToString();
        client.Add(JsonConvert.DeserializeObject<T[]>(data)[0]);    
    }
}

private void initClerks(Client client)
{
    string[] pks = client.ClerksPKS.Trim(',').Split(',');
    Initialize<Clerk>(client, pks);
}

private void initManagers(Client client)
{
    string[] pks = client.ManagerPK.Trim(',').Split(',');
    Initialize<Manager>(client, pks);           
}

And going forward thsoe two intiClerks/initmanagers looks redundant, just inline calls to Initialize (surely if entire code base not more complex than you shown here)

Answer 3

Create base class to add the common logic in it base Init method. then in the class that implements BaseClientInitializer override Insert method. This design pattern is called template method.

public abstract class BaseClientInitializer
  {
    private string[] keys;
    public BaseClientInitializer(string[] keys)
    {
      this.keys = keys;
    }
public abstract void Insert(string data);

public void Init()
{
  foreach (string pk in keys)
  {
    var data = JObject.Parse(DBUtils.GetData(Constants.DBProcedures.GetProcedures.GetWorkerDetailsByPkid, pk))[
        Constants.ResponseJson.Data].ToString();
    this.Insert(data);
  }
}   


 }

public class Clerks : BaseClientInitializer
  {
    private Client client;
    public Clerks(Client client) : base(client.ClerksPKS.Trim(','). Split(','))
    {
      this.client = client;
    }

    public override void Insert(string data)
    {
      client.addClerk(JsonConvert.DeserializeObject<Clerk[]>(data)[0]);
    }
  }

private void initClerks(Client client)
{
     var clerksInitializer = new ClerksInitializer(client);
    clerksInitializer.Init();
}

Do the same for init managers. The difference is not only in the insert method the pks array is also different: client.ClerksPKS.Trim(','). Split(',') - for clerks and client.ManagerPK.Trim(',').Split(',') for managers. So simply extract method will not work correctly.

Answer 4

You can do this by passing an action into a method. Something like

private void actOnData(Client client, string[] pks, Action<Client, string> addThing)
{
    foreach (string pk in pks)
    {
        string data = JObject.Parse(DBUtils.GetData(Constants.DBProcedures.GetProcedures.GetWorkerDetailsByPkid, pk))[Constants.ResponseJson.Data].ToString();
        addThing(client, data);
    }
}

private void initClerks(Client client)
{
    string[] pks = client.ClerksPKS.Trim(',').Split(',');
    actOnData(client,pks,(c,d) => { c.addClerk(JsonConvert.DeserializeObject<Clerk[]>(d)[0]); });
}

private void initManagers(Client client)
{
    string[] pks = client.ManagerPK.Trim(',').Split(',');
    actOnData(client, pks, (c, d) => { c.addManager(JsonConvert.DeserializeObject<Manager[]>(d)[0]); });
}