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) {
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
- Andy Hunt
The pragmatic programmer -
Martin Folwer
Refactoring: Improving the design of Existing code
-
Jeff Attwood
Coding Horror: Code Smells
C# Code Smells
By Ahmed Ayub
C# Code Smells
- 2,142