CODEWITHSUNDEEP
refactoringdry
komizo
komizo

Reputation: 1061

Best practice, good patterns to avoid DRY violation

I would like to get some advice regarding refactorization of the following functions:

There are three functions which perform almost the same logic.

public SendResult ExecuteSendMessageByEmployee(Employee emp, SendingOptions options) {
  var result = SendMessage(emp);

  if (options.DepictEmpData)
  {
    _generator.DepictData(emp);
  }

  if (options.DepictResult)
  {
    _generator.DepictData(result);
  }
}

public SendResult ExecuteSendMessageByID(int employeeID, SendingOptions options) {

  var result = SendMessage(employeeID);

  var emp = _empService.GetByID(employeeID);

  if(emp == null)
  {
     _generator.NotifyNoEmployeeFound(employeeID);
     return new SendResult(){Success = false};
  } 

  if (options.DepictEmpData)
  {
    _generator.DepictData(emp);
  }

  if (options.DepictResult)
  {
    _generator.DepictData(result);
  }

  return result;
}

public SendResult ExecuteSendMessageByDNA(list<byte> dna, SendingOptions options) {

  var result = SendMessage(dna);

  var emp = _empService.GetByID(dna);

  if(emp == null)
  {
     _generator.NotifyNoEmployeeFound(dna);
  }      

  if (options.DepictEmpData)
  {
    _generator.DepictData(emp);
  }

  if (options.DepictResult)
  {
    _generator.DepictData(result);
  }

  return result;
}

The functions above are quite similar but the system can associate employee by different data. As depicted: sometimes I have full Employee object, another time I have only ID, and another time DNA bytes.

The functions above have been made only to depict use case.

The application needs in many places execute functions with different params; SendOptions can also change.

Do you have any advice to reduce redundant code? Be more KISS, not violate DRY pattern?

Upvotes: 1

Views: 116

Answers (1)

Carl Manaster
Carl Manaster

Reputation: 40356

Just call your first method from the second two methods.

public SendResult ExecuteSendMessageByEmployee(Employee emp, SendingOptions options) {
  var result = SendMessage(emp);

  if (options.DepictEmpData) {
    _generator.DepictData(emp);
  }

  if (options.DepictResult) {
    _generator.DepictData(result);
  }
}

public SendResult ExecuteSendMessageByID(int employeeID, SendingOptions options) {
   var emp = _empService.GetByID(employeeID);

   if (emp == null) {
       _generator.NotifyNoEmployeeFound(employeeID);
      return new SendResult(){Success = false};
   }

  return ExecuteSendMessageByEmployee(emp, options);
}

public SendResult ExecuteSendMessageByDNA(list<byte> dna, SendingOptions options) {
   var emp = _empService.GetByID(dna);

   if (emp == null) {
       _generator.NotifyNoEmployeeFound(employeeID);
   }

  return ExecuteSendMessageByEmployee(emp, options);
}

Upvotes: 3

Related Questions