Review - I

During code reviews, one of the things that is considered is the "Command Query Separation Principle" Violation.

Operations should either do something and return nothing or return something and do nothing, NOT both.

The common violations are

1. Getter Methods having void as their return types and
2. Setter methods having some return values.

Getter Methods having void as their return types.

For eg:

void GetSalary(Employee emp)

{

Money salary;

//Fetch Salary from some source.

emp.Salary = salary;

}

Generally, these violations can be fixed by renaming the method name to Update.

void UpdateSalary(Employee emp);

Setter methods having somereturn values.

For eg:

bool SetDepartment(Employee emp);

Here, it gets tricky. The programmer sets the department inside the method and wants the confirmation by the return value.

There are other ways to handle this rather than sending the return value.

1. Having a method HasDepartment() in the Employee, to check whether it has Department.
2. Raise the exception if no matching department can be found for the employee.

The other ways by which this principle is violated are clearly trying to do too many things in a method and has to be refactored.

But, there is a case where this violation is acceptable, which is called Fluent Interfaces. In this case, the signature will become something like the following.

Employee SetDepartment(Employee emp);

In this case also, generally the return values are the entity types and not the primitive or Value Object types.