C# Code Smells

Pokemon Exception Handling



try
{

}
catch
{
   // I'm catching everything as I don't really 
   // know what my application is doing.
}

Clases demasiado largas


  • Más de 30 líneas de código en un solo método
  • Código repetido en más de un método de
    la misma clase

Código que no se muere


// No estoy seguro si se usará todavía
// int result = command.Setup();
// Command.WriteLine(result)

Si tienes código que no se usa, no lo comentes,

BORRALO!

Para eso tenemos source control

Separación de responsabilidades


  • Un archivo debe contener solo una clase
  • Intenta separar las responsabilidades de:
    • Acceso a datos
    • Manipulación/transformación de objetos
    • Presentación 
    • Interacción con el usuario

Programando Manualmente


Por que hacer esto:

 SqlConnection connection = new SqlConnection(connectionString);
 SqlCommand command = connection.CreateCommand();
 command.CommandText = "SELECT * FROM Tabla WHERE Campo = @param";
 command.Parameters.Add(new SqlCommand(SqlDbType.String, "prueba");
 var reader = command.ExecuteReader();  ... // Etc...

Cuando puedes hacer esto
var db = new sistemaContext();
var ordenes = db.Ordenes.Where(order => order.Id == 45); 



Doscientos mil parámetros:


public int InsertarNuevaOrden(DateTime fecha, string nombreUsuario, FormasDePago formaDePago, 
int cantidadItems, int noReferenciaVendedor, Address Localidad, List<Items> itemsComprados, 
decimal subTotal, decimal impuestos, decimal costoShipping, decimal total, 
TipoShipping tipoShipping) {
        //Código para almacenar aquí
    }  

Por que no hacerlo así mejor:

 public int InsertarNuevaOrden(Orden orden) {
        //Código para almacenar aquí
 }  

Comentarios

No abuses de los comentarios. 

Describe métodos y propiedades, pero no hagas esto:

public void DoSomething(string param){   //Create dbContext   var context = new MyDbContext();   //Get Customers
var customers = context.Customers.ToList(); //Update Customers
customers.ForEach(c => c.PropertyX == param); //Save Changes
context.SaveChanges();}

DRY

(Don't repeat yourself)

private void button1_click(object sender, EventArgs e)
{
   //Save Customer Info
   txtName.Text = "";
   txtAddress.Text = "";
   //After Refactor, just one line of code
   RefreshAndClearControls();
}
private void button2_click(object sender, EventArgs e)
{
   txtName.Text = "";
   txtAddress.Text = "";
   //After refactor
   RefreshAndClearControls();
}
private void RefreshAndClearControls()
{
   txtName.Text = "";
   txtAddress.Text = "";  
}
 

Pregunta por NULL

(y duerme más tranquilo)

public void DoSomething(int customerID)
{
   var customer = context.Customers.FirstOrDefault(c =>      
                     c.CustomerID == customerID);
   var newCustomer = new Customer();
   newCustomer.Name = customer.Name;     // el objeto customer puede estar nulo si el id 
    // del usuario no existe } 

No escribas código en dos idiomas


Hazlo en Inglés (preferible) o en español, pero no en Spanglish

public List<Cliente> GetClientesPorCategory(string categoria)

{}



Utiliza un patrón de arquitectura


  • WinForm, WebForm => MVP (Model View Presenter)
  • WPF => MVVM (Model View ViewModel)
  • ASP.NET MVC => pues claro que Model View Controller ;)


Tu nuevo mejor amigo



Referencias


C# Code Smells

By Ahmed Ayub

C# Code Smells

  • 2,142