CODEWITHSUNDEEP
c#
user5515846
user5515846

Reputation:

How to avoid code duplication inside two methods?

I have two identical methods, but one of them have return statement inside try catch

public void A(Guid agentId)
{
     var agent = _agentsProvider.GetAgentById(agentId);
     var updateCompletionSource = C(agentId);
     try
     {
         var cacheEntry = UpdateAgentMetadataCacheEntry(agent, true, false);
         updateCompletionSource.SetResult(cacheEntry);
     }
     catch (Exception e)
     {
         updateCompletionSource.SetException(e);
     }
}

private Entry B(IAgent agent)
{
     var updateCompletionSource = C(agent.Id);
     try
     {
          var cacheEntry = UpdateAgentMetadataCacheEntry(agent, false, false);
          updateCompletionSource.SetResult(cacheEntry);
          return cacheEntry;
      }
      catch (Exception e)
      {
          updateCompletionSource.SetException(e);
          return GetPreviousCacheEntry();
      }
}

How to collect identical part and create new method with this part?

Upvotes: 2

Views: 234

Answers (2)

joordan831
joordan831

Reputation: 720

Like Jon said, you don't need method A. Just add another parameter for boolean value.

public void A(Guid agentId)
{
    var agent = _agentsProvider.GetAgentById(agentId);
    AnotherA(agent, true);
}

private Entry B(IAgent agent)
{
    return AnotherA(agent, false);
}

private Entry AnotherA(IAgent agent, bool a)
{
    try
    {
         var cacheEntry = UpdateAgentMetadataCacheEntry(agent, a, false);
         updateCompletionSource.SetResult(cacheEntry);
         return cacheEntry;
    }
    catch (Exception e)
    {
        updateCompletionSource.SetException(e);
        return GetPreviousCacheEntry();
    }
}

Upvotes: 0

Jon Skeet
Jon Skeet

Reputation: 1499800

Unless GetPreviousCacheEntry could have problematic side-effects, it seems to me that you don't need method A at all.

Just call method B and ignore the return value if you're not interested in it.

As noted in comments, the methods aren't identical other than the return statements though - because they use a different second argument for UpdateAgentMetadataCacheEntry, and they have different parameters too (one has a Guid and one has an Agent). You could refactor this into:

private Entry B(IAgent agent, bool foo)
{
     var updateCompletionSource = C(agent.Id);
     try
     {
          var cacheEntry = UpdateAgentMetadataCacheEntry(agent, foo, false);
          updateCompletionSource.SetResult(cacheEntry);
          return cacheEntry;
      }
      catch (Exception e)
      {
          updateCompletionSource.SetException(e);
          return GetPreviousCacheEntry();
      }
}

... with a meaningful name for foo, obviously. I'll assume the difference in parameter type isn't a problem in reality.

Upvotes: 11

Related Questions