Hay mucha teoría acerca de las ventajas de aplicar esto, léelo por tu cuenta por ejemplo en https://es.wikipedia.org/wiki/Complejidad_ciclom%C3%A1tica.
Visual Studio 2010, 2012 y 2013 tienen un excelente Analizador de Código, donde arroja un warning o error de acuerdo a ciertas reglas. Por ejemplo por defecto indicará un warning cuando la CC sea mayor a 25.
PHP tiene una herramienta similar llamada PHPLOC.
Tips en General
- Crea métodos privados usando nombres claros para el código duplicado.
- Crea métodos privados con nombres claros que contengan funcionalidades encapsuladas no directamente relacionadas con la lógica de negocio. Cómo por ejemplo:
- Acceso a datos
- Llamadas a Web services
- Código de patrones de diseño, etc.
- Crea métodos privados con nombres claros para encapsular cada pieza con lógica de dominio
- Refactoriza cada método usando las siguientes reglas:
A. Realiza validación de campos o datos de entrada primero y devolver un error o lanza excepciones si falla alguna la validación.
B. Realiza la validación de los datos del negocio y retorna un error o lanza si la validación falla
C. Recién ahora ejecuta la operación misma del negocio
D. Agrega comentarios de porqué haces tal cosa y no otra
E. Aplica las siguientes reglas de oro:
- No declares las variables al inicio del método, en vez de ello declara las variables en el punto exacto que las necesites.
- No reutilices la misma variable para múltiples propósitos.
- Evita las siguientes prácticas que muchos desarrolladores aplican jurando que ellos optimizan el código para mejor performance (los compiladores actuales no lo necesitan). Estas acciones reducen las lecturas pero no dan ninguna mejora de performance:
- No intentes minimizar el número de llamadas a métodos.
- De nuevo, no declares las variables al inicio del método, sino en el punto que la requieras.
- Permite varios returns desde un método, puede ayudar a veces a mejorar el CC.
- Si usas Using (StreamReader x...) no hagas adentro un x.Close() ya que el método Close() se gatilla automáticamente con el Dispose() del Using.
- No uses estas formas de evaluar equivalencia:
- if (10 == n)
- if (n.Equals(10))
- No uses string.Concat para concatenar strings, en cambio usa "+".
Ejemplo
Con tal de ejemplificar lo anterior, tomemos un caso de una aplicación bancaria:Este método realiza el siguiente algoritmo:
Revisa si el monto solicitado a retirar es menor a la cantidad almacenada en su cuenta corriente
Si es así, realiza el retiro
Si no,
Chequea si tiene línea de crédito para ver si el puede realizar un retiro
Si no tiene cupo en la línea de crédito, rechaza la solicitud
Si no,
Calcula el máximo de dinero que pueda sacar sin necesidad de aprobación de un ejecutivo. Este cálculo depende del tipo de cuenta
Si el total de dinero en la línea le alcanza, libera el pago
Si no, iniciar el flujo de aprobación por un ejecutivo
Este método de negocio usa las propiedades AccountRepository y TransactionHistoryRepository de la clase para realizar el acceso a datos y la propiedad CreditHistoryServiceAgent para invocar a los Web services.
Este método usa un patrón de diseño estratégicamente para realizar cierta lógica de negocio según cada tipo de cuenta. La instancia se crea usando el patrón Abstract Factory y se aplica el factory correcto usando el tipo cuenta que está cargada en un diccionario.
Este método también inicializa el flujo de aprobación si se requiere.
Este es el método principal Withdraw(...) que permite retiro de dinero:
////// Withdraw money from a bank account /// /// withdrawal request ///A response that indicate the results of the processing of request ///Throws when the input request object is null ///Throws when the requested account no is invalid public TransactionResponse Withdraw(WithdrawRequest request) { TransactionResponse response = null; //Check if the request is null if (request != null) { using (TransactionScope transactionScope = new TransactionScope()) { //Get account Account account = AccountRepository.GetAccount(request.AccountNo); //Check if account is null if (account != null) { //Check if amount is less than the account balance if (request.Amount < account.Balance) { //Reduce the amout from the balance account.Balance -= request.Amount; AccountRepository.Save(account); //Create history entry TransactionHistory history = new TransactionHistory(); history.TransactionType = (int)TransactionType.Withdraw; history.AccountId = account.AccountId; history.Time = DateTime.Now; history.Amount = request.Amount; TransactionHistoryRepository.Add(history); //Return success response response = new TransactionResponse() { ResponseCode = (int)TransactionResponseCodes.Succeeded, ResponseMessage = Resources.TransactionCompleted, }; } else { //Check if OD can be issued based for the customer PersonIdentity identify = new PersonIdentity() { FullName = account.Owner.FullName, NationalIdentityCardNo = account.Owner.NationalIdentityCardNo, }; bool canIssueOD = CreditHistoryServiceAgent.CanIssueOD(identify); if (canIssueOD) { //Process OD //Retrieve the amount that can be withdrawn without any approval IAccountStrategyFactory stratergyFactory; if (!accountStratergyFactories.TryGetValue(account.Type, out stratergyFactory)) { throw new ArgumentException("Invalid Account Type"); } IAccountWithdrawalStrategy withdrawalStratergy = stratergyFactory.GetWithdrawalStrategy(); decimal withdrawalLimit = withdrawalStratergy.GetODLimitWithoutApproval(account); //check if the amount is less than the withdrawal limit if (account.Balance - request.Amount > withdrawalLimit) { //reduce the amount from account balance account.Balance -= request.Amount; AccountRepository.Save(account); //create transaction history entry TransactionHistory history = new TransactionHistory(); history.TransactionType = (int)TransactionType.Withdraw; history.Time = DateTime.Now; history.Amount = request.Amount; TransactionHistoryRepository.Add(history); //Return success response response = new TransactionResponse() { ResponseCode = (int)TransactionResponseCodes.Succeeded, ResponseMessage = Resources.TransactionCompleted, }; } else { //Initiate approval workflow IWithdrawalApprovalWorkflow workflow = WorkflowFactory.GetWithdrawApprovalWorkflow(); workflow.Request = request; workflow.Start(); //Return pending approval response response = new TransactionResponse() { ResponseCode = (int)TransactionResponseCodes.PendingApproval, ResponseMessage = Resources.PendingApproval, }; } } else { //Return request failed response response = new TransactionResponse() { ResponseCode = (int)TransactionResponseCodes.Failed, ResponseMessage = Resources.FailedDueToBadCreditHistory, }; } } } else { throw new ArgumentException("Invalid Account No"); } //Mark the transaction to be commited transactionScope.Complete(); } } else { throw new ArgumentNullException("request"); } return response; }
Refactorizando
1. Elimina el código duplicado. Crea un nuevo método que guarde en TransactionHistoryRepository y lo llamas 2 veces de Withdraw(...):
private TransactionResponse PerformWithdrawal(WithdrawRequest request, TransactionResponse response, Account account)
{
//Reduce the amout from the balance
account.Balance -= request.Amount;
AccountRepository.Save(account);
//Create history entry
TransactionHistory history = new TransactionHistory();
history.TransactionType = (int)TransactionType.Withdraw;
history.AccountId = account.AccountId;
history.Time = DateTime.Now;
history.Amount = request.Amount;
TransactionHistoryRepository.Add(history);
//Return success response
response = new TransactionResponse()
{
ResponseCode = (int)TransactionResponseCodes.Succeeded,
ResponseMessage = Resources.TransactionCompleted,
};
return response;
}
2. Saca la funcionalidad que no está relacionada directamente con la lógica de negocio hacia métodos separados, esto ayuda a mejorar la mantenibilidad del código, por lo tanto se tiene un Withdraw(...) y un PerformWithdrawal(...) con llamadas a otros métodos:
public TransactionResponse Withdraw(WithdrawRequest request)
{
TransactionResponse response = null;
//Check if the request is null
if (request != null)
{
using (TransactionScope transactionScope = new TransactionScope())
{
//Get account
Account account = AccountRepository.GetAccount(request.AccountNo);
//Check if account is null
if (account != null)
{
//Check if amount is less than the account balance
if (request.Amount < account.Balance)
{
response = PerformWithdrawal(request, response, account);
}
else
{
if (CanIssueODBasedOnCreditHistory(account))
{
//Process OD
decimal withdrawalLimit = GetODLimitWithoutApproval(account);
//check if the amount is less than the withdrawal limit
if (account.Balance - request.Amount > withdrawalLimit)
{
response = PerformWithdrawal(request, response, account);
}
else
{
response = StartWithdrawApprovalWorkflow(request, response);
}
}
else
{
//Return request failed response
response = GetFailedResponse(response);
}
}
}
else
{
throw new ArgumentException("Invalid Account No");
}
//Mark the transaction to be commited
transactionScope.Complete();
}
}
else
{
throw new ArgumentNullException("request");
}
return response;
}
private TransactionResponse PerformWithdrawal(WithdrawRequest request,
TransactionResponse response, Account account)
{
//Reduce the amout from the balance
account.Balance -= request.Amount;
AccountRepository.Save(account);
CreateWithdrawalHistoryEntry(request, account);
//Return success response
return GetSuccessResponse(ref response);
}
3. Crea métodos privados con nombres claros para cada pieza de la lógica del dominio. Esto ayuda a simplificar la lógica y hacer el código mantenible. El método Withdraw(...) se vería así:
public TransactionResponse Withdraw(WithdrawRequest request)
{
TransactionResponse response = null;
//Check if the request is null
if (request != null)
{
using (TransactionScope transactionScope = new TransactionScope())
{
//Get account
Account account = AccountRepository.GetAccount(request.AccountNo);
//Check if account is null
if (account != null)
{
//Check if amount is less than the account balance
if (request.Amount < account.Balance)
{
response = PerformWithdrawal(request, response, account);
}
else
{
response = ProcessAsOD(request, response, account);
}
}
else
{
throw new ArgumentException("Invalid Account No");
}
//Mark the transaction to be commited
transactionScope.Complete();
}
}
else
{
throw new ArgumentNullException("request");
}
return response;
}
private TransactionResponse ProcessAsOD(WithdrawRequest request, TransactionResponse
response, Account account)
{
if (CanIssueODBasedOnCreditHistory(account))
{
//Process OD
decimal withdrawalLimit = GetODLimitWithoutApproval(account);
//check if the amount is less than the withdrawal limit
if (account.Balance - request.Amount > withdrawalLimit)
{
response = PerformWithdrawal(request, response, account);
}
else
{
response = StartWithdrawApprovalWorkflow(request, response);
}
}
else
{
//Return request failed response
response = GetFailedResponse(response);
}
return response;
}
4. Refactoriza usando las reglas indicadas:
- Valida la entrada y retorna una excepción si hay error.
- Valida los datos del negocio y retorna una excepción si hay error.
- Ejecuta la operación de negocio en si.
- Comenta lo que creas prudente.
public TransactionResponse Withdraw(WithdrawRequest request)
{
TransactionResponse response = null;
//Validate input parameters
if (request != null)
{
throw new ArgumentNullException("request");
}
//Perform business logic in a transaction
using (TransactionScope transactionScope = new TransactionScope())
{
//Get account
Account account = AccountRepository.GetAccount(request.AccountNo);
if (account != null)
{
throw new ArgumentException("Invalid Account No");
}
if (request.Amount < account.Balance)
{
//If the requested amount is less than the balance then
//the withdrawal can be performed directly
response = PerformWithdrawal(request, response, account);
}
else
{
//Otherwise the request has to be performed as an OD
response = ProcessAsOD(request, response, account);
}
//Mark the transaction to be committed
transactionScope.Complete();
}
return response;
}
private TransactionResponse ProcessAsOD(WithdrawRequest request, TransactionResponse
response, Account account)
{
//Check if the requested customer can receive and OD based on his credit history
//Reject the request if he has a bad credit history
if (CanIssueODBasedOnCreditHistory(account))
{
return GetFailedResponse(response);
}
//Check if the requested amount is less than the OD limit for the account type
//If so perform the request
//Else initiate the approval workflow to get branch managers approval
decimal withdrawalLimit = GetODLimitWithoutApproval(account);
if (account.Balance - request.Amount > withdrawalLimit)
{
return PerformWithdrawal(request, response, account);
}
else
{
return StartWithdrawApprovalWorkflow(request, response);
}
}
private TransactionResponse PerformWithdrawal(WithdrawRequest request,
TransactionResponse response, Account account)
{
//Reduce the amout from the balance
account.Balance -= request.Amount;
AccountRepository.Save(account);
CreateWithdrawalHistoryEntry(request, account);
return GetSuccessResponse(ref response);
}
5. Aplica las siguientes reglas:
- Declara las variables sólo cuando las usarás y no al inicio del método.
- No uses la misma variable 2 o más veces.
Ejemplo, si tenemos un código correctamente escrito:
public double FindEffortInManDays(RequirementDefinition requirements,
ComplexityFactors complexity, EnvironmentFactors environment)
{
double functionPoints = CalculateFunctionPoints(requirements);
double adjustedFunctionPoints = CalculateAdjustedFunctionPoints(functionPoints, complexity);
double effortInManDays = CalculateEffort(adjustedFunctionPoints, environment);
return effortInManDays;
}
Si aplicamos lo de declarar las variables al inicio:
public double FindEffortInManDays3(RequirementDefinition requirements,
ComplexityFactors complexity, EnvironmentFactors environment)
{
double functionPoints = 0.0;
double adjustedFunctionPoints = 0.0;
double effortInManDays = 0.0;
functionPoints = CalculateFunctionPoints(requirements);
adjustedFunctionPoints = CalculateAdjustedFunctionPoints(functionPoints, complexity);
effortInManDays = CalculateEffort(adjustedFunctionPoints, environment);
return effortInManDays;
}
Mira lo que pasa si cambias de orden una línea como se se ve abajo, el resultado será totalmente distinto, sin que el compilador lo note:
public double FindEffortInManDays31(RequirementDefinition requirements,
ComplexityFactors complexity, EnvironmentFactors environment)
{
double functionPoints = 0.0;
double adjustedFunctionPoints = 0.0;
double effortInManDays = 0.0;
adjustedFunctionPoints = CalculateAdjustedFunctionPoints(functionPoints, complexity);
functionPoints = CalculateFunctionPoints(requirements);
effortInManDays = CalculateEffort(adjustedFunctionPoints, environment);
return effortInManDays;
}
Un impacto similar es reutilizando una misma variable. En el ejemplo de abajo, se usa una misma variable para mantener dos cantidades que lógicamente son distintas:
public double FindEffortInManDays2(RequirementDefinition requirements,
ComplexityFactors complexity, EnvironmentFactors environment)
{
double functionPoints = CalculateFunctionPoints(requirements);
functionPoints = CalculateAdjustedFunctionPoints(functionPoints, complexity);
double effortInManDays = CalculateEffort(functionPoints, environment);
return effortInManDays;
}
El siguiente código tiene otro impacto, ya que se reutiliza un misma variable y cambia un orden del código, dando un resultado erróneo sin que el compilador lo delate:
public double FindEffortInManDays21(RequirementDefinition requirements,
ComplexityFactors complexity, EnvironmentFactors environment)
{
double functionPoints = CalculateFunctionPoints(requirements);
double effortInManDays = CalculateEffort(functionPoints, environment);
functionPoints = CalculateAdjustedFunctionPoints(functionPoints, complexity);
return effortInManDays;
}
Otros tips
1. No intentes bajar el número de llamados a métodosEl compilador C# puede optimizar el performance a medida que recorre el código, reduciendo el número de llamadas a métodos no ganas performance. Sin embargo, dividiendo un gran método en múltiples llamados a métodos chicos, haces el código mas mantenible y reusable. Lo que si, asegúrate de nombrarlos apropiadamente.
2. Si usas using (var x = new StreamReader) no hagas adentro un x.Close() ya que este se gatilla automáticamente con el mismo Dispose() del using.
using( var reader = new StreamReader(file) )
{
variable = reader.ReadToEnd();
reader.Close(); // Esta linea está de más!!!
}
Una alternativa es cerrarlo manualmente pero sólo si no usas using:
reader = new StreamReader(file); variable = reader.ReadToEnd(); reader.Close();
3. No uses las siguientes formas de validad igualdad
if (10 == n)
if (n.Equals(10))
Escribe las condiciones como:
if (n == 10)
Sin embargo, a pesar del impacto en la lectura, debemos comparar los string con string.Equals como se muestra abajo:
if (string.Equals(string1, string2, StringComparison.Ordinal))
Esto porque .Net aplicará las reglas de cultura específicas para comprar la igualdad de los strings, algo que no pasa si sólo nos limitamos a usar el símbolo "==".
Ahora, StringComparison.Ordinal podría darle resultado inesperado, ya que se basa en la cultura actual del usuario (navegador del usuario, en caso de una aplicación web).
Por lo tanto siempre tenemos que ser específicos en el método de comparar cadenas. StringComparison.Ordinal ve los caracteres escondidos incluso los que no se ven en el navegador (como caracteres especiales ejemplo U+200E) y los compara, en cambio StringComparison.InvariantCultureIgnoreCase los ignora.
Los tipos de comparación son:
StringComparison.CurrentCulture StringComparison.CurrentCultureIgnoreCase StringComparison.InvariantCulture StringComparison.InvariantCultureIgnoreCase StringComparison.Ordinal StringComparison.OrdinalIgnoreCase
Una buena explicación está de cada caso en MSDN:
http://msdn.microsoft.com/en-us/library/system.stringcomparison(v=vs.110).aspx
4. Usa "+" en vez de string.Concat
Esto es lo correcto:
a = b + c + d;
Ya que si usas esto:
a = string.Concat(a, b, c);
Es una optimización que el compilador mismo se encarga de hacer por lo que no es necesario hacerlo de nuevo. Además el símbolo "+" es mas legible que "Concat".
Este artículo lo hice basándome en este http://ruwandotnet.wordpress.com/2012/03/13/59/ (en inglés)
