0

I am planning to move the validations to a separate function for granularity and easy to maintain unit tests. However, I would need some variables which are hitting the database in the Validate method to be reused in Process method and make sure it is unit testable.

Current Implementation

interface ICustomerService
{
   void Process();
}

public class CustomerService: ICustomerService
{
  ICustomerDbService db;
  public CustomerService(ICustomerDbService customerDbService)
  {
     db = customerDbService;
  }


  public void Process()
  {
    //validations
    var customer = db.GetCustomer(customerId);
    if(customer == null)
      return "Not found";

    //processing
  }
}

//Usage
ICustomerService service = new CustomerService(dbService);
service.Process();

Future Implementation

interface ICustomerService
{
   bool Validate(int customerId);
   void Process();
}

public class CustomerService: ICustomerService
{
  ICustomerDbService db;
  public CustomerService(ICustomerDbService customerDbService)
  {
     db = customerDbService;
  }

  public bool Validate(int customerId)
  {
    var customer = db.GetCustomer(customerId);
    if(customer == null)
      return "Not found";

    //other processing with multiple(3-4) common variables 
  }

  public void Process()
  {
    var customer = db.GetCustomer(customerId); // How to avoid this call
  }
}

//Usage
ICustomerService service = new CustomerService(dbService);
bool isValid = service.Validate(10)
if(isValid)
{
   service.Process();
}
Sunny
  • 4,765
  • 5
  • 37
  • 72
  • Service will receive the minimum params from the calling class i.e, customerId and need to perform processing within the service based on it. – Sunny Oct 26 '17 at 16:46
  • Can only one customer be validated at a time? Also right now `Validate` returns a `string`, not a `bool` – yinnonsanders Oct 26 '17 at 16:49
  • @yinnonsanders, yes only one customer is validated and processed. – Sunny Oct 26 '17 at 16:53
  • @Sunny This appears to be an [XY problem](https://meta.stackexchange.com/questions/66377/what-is-the-xy-problem). What is the ultimate goal you are trying to achieve? Also, the question in its current state is unclear as it is incomplete. Read [ask] and then provide a [mcve] that can be used to better understand your problem. – Nkosi Oct 26 '17 at 16:59
  • @Nkosi Goal is to separate validation logic from the processing logic. Right now, I have the validation logic inside the process method. – Sunny Oct 26 '17 at 17:01
  • @Sunny then show the current and desired structures. Not much help can be provided with the current state of the question. – Nkosi Oct 26 '17 at 17:04
  • @Nkosi, I have updated the details. – Sunny Oct 26 '17 at 17:09
  • Have you read https://stackoverflow.com/questions/34571/how-do-i-test-a-class-that-has-private-methods-fields-or-inner-classes? – Neville Kuyt Oct 26 '17 at 17:09
  • 1
    If it's invalid to call `Process` without calling `Validate` and vice-versa, then I wouldn't make them both public, not even for unit testing purposes – Sam I am says Reinstate Monica Oct 26 '17 at 17:14
  • Instead, write unit tests that target each aspect of the validate&process routine – Sam I am says Reinstate Monica Oct 26 '17 at 17:18
  • @Sunny The example provided is not valid. Please take the time to review what you are submitting. That said based on what is currently provided. You can always delegate the Validation out into its own abstraction so it can be treated in isolation is needed. – Nkosi Oct 26 '17 at 17:22

2 Answers2

1

Seems like your CustomerService have more then one responsibilities
- Validate customer
- Process validated result

You can introduce three classes with responsibilities
- Customer validation
- Processing customer data
- Combining validation and processing

And make Validation method return required data for Process method, which give you possibility to have separated logic and save you from possible "problems" of sharing state/variables of the classes.

public class CustomerValidation
{
    public Customer Validate(int customerId)
    {
        // Validation logic which "produce" instance of customer by given id
        return customer;
    }
}

public class CustomerProcess
{
    public void Process(Customer customer)
    {
        // Process given customer
    }
}

public class CustomerService
{
    private CustomerValidation _validation;
    private CustomerProcess _process;

    public CustomerService(CustomerValidation validation, CustomerProcess process)
    {
        _validation = validation;
        _process = process;
    }

    public void DoStaff(int customerId)
    {
        var customer = _validation.Validate(customerId);
        if (customer != null)
        {
            _process.Process(customer);
        }
    }
}

Which will be used like this

var validation = new CustomerValidation();
var process = new CustomerProcess();
var service = new CustomerService(validation, process);

service.DoStaff(customerId);

Instead of "actual" implementation of Validation and Process classes you can introduce abstraction (interfaces), which give you possibility to replace different validation implementations and write unit tests which test actual logic of the Service class - which is combining validate and process logics.

Nkosi
  • 235,767
  • 35
  • 427
  • 472
Fabio
  • 31,528
  • 4
  • 33
  • 72
  • This is a more SOLID approach. I was waiting for OP to provide a proper mcve before suggesting this. – Nkosi Oct 26 '17 at 17:46
0

This is the pattern I'd go for that's closest to what it seems you want to do

public class CustomerService: ICustomerService
{
  ICustomerDbService db;
  public CustomerService(ICustomerDbService customerDbService)
  {
     db = customerDbService;
  }

  public bool Validate(int customerId)
  {
    var customer = db.GetCustomer(customerId);
    return Validate(customer);
  }

  public void Process(int customerId)
  {
    var customer = db.GetCustomer(customerId);
    if(Validate(customer))
    {
      //do processing...
    }
  }

  private bool Validate(Customer customer, /*other args*/)
  {
    if(customer == null)
      return "Not found";
    //other processing with multiple(3-4) common variables 
  }
}