Lo mejor para lo último, el más popular de los code smell. Si tenemos un método que tenga más de 10 líneas de código seguramente el método es muy largo. Seguramente estarás pensando que tienes muchos con más de 50 líneas de código. No significa que esté mal, pero cuando las líneas son muchas hace difícil entender que hace, difícil de cambiar y difícil de reutilizar.
Métodos Largos
Si tenemos muchas líneas, seguramente nuestro código no está especializado en algo y hace muchas cosas. Debemos buscar que nuestros códigos sean especializados. Que hagan solo una cosa a la vez y que la hagan bien. Por ejemplo, si tenemos un código de 250 líneas, seguramente muchas de las líneas pertenecen a otros lugares como métodos en la misma clase o en una clase diferente.
La pregunta del millón es: ¿Cómo sabemos dónde deben pertenecer? Para esto existe el principio de cohesión. Esto significa que las cosas que están relacionadas deben estar juntas y las cosas que no están relacionadas deberían estar por separado. Este principio no solamente puede usarse con métodos, también, podemos usarla con clases.
En el caso de las clases tenemos “Single Responsability Principle”. Este principio dice que cada clase o método debe hacer solo una cosa y debe hacerlo bien convirtiéndolo en expertos. Veamos algunos ejemplos.
protected void Page_Load(object sender, EventArgs e)
{
System.IO.MemoryStream ms = CreateMemoryFile();
byte[] byteArray = ms.ToArray();
ms.Flush();
ms.Close();
Response.Clear();
Response.ClearContent();
Response.ClearHeaders();
Response.Cookies.Clear();
Response.Cache.SetCacheability(HttpCacheability.Private);
Response.CacheControl = "private";
Response.Charset = System.Text.UTF8Encoding.UTF8.WebName;
Response.ContentEncoding = System.Text.UTF8Encoding.UTF8;
Response.AppendHeader("Pragma", "cache");
Response.AppendHeader("Expires", "60");
Response.ContentType = "text/comma-separated-values";
Response.AddHeader("Content-Disposition", "attachment; filename=FooFoo.csv");
Response.AddHeader("Content-Length", byteArray.Length.ToString());
Response.BinaryWrite(byteArray);
}
En este fragmento de código podemos ver que se llamará a la base de datos y convertirá los datos en un archivo CSV. El método CreateMemoryFile() será el encargado de esto.
public System.IO.MemoryStream CreateMemoryFile()
{
MemoryStream ReturnStream = new MemoryStream();
try
{
string strConn = ConfigurationManager.ConnectionStrings["FooFooConnectionString"].ToString();
SqlConnection conn = new SqlConnection(strConn);
SqlDataAdapter da = new SqlDataAdapter("SELECT * FROM [FooFoo] ORDER BY id ASC", conn);
DataSet ds = new DataSet();
da.Fill(ds, "FooFoo");
DataTable dt = ds.Tables["FooFoo"];
//Create a streamwriter to write to the memory stream
StreamWriter sw = new StreamWriter(ReturnStream);
int iColCount = dt.Columns.Count;
for (int i = 0; i < iColCount; i++)
{
sw.Write(dt.Columns[i]);
if (i < iColCount - 1)
{
sw.Write(",");
}
}
sw.WriteLine();
int intRows = dt.Rows.Count;
// Now write all the rows.
foreach (DataRow dr in dt.Rows)
{
for (int i = 0; i < iColCount; i++)
{
if (!Convert.IsDBNull(dr[i]))
{
string str = String.Format("\"{0:c}\"", dr[i].ToString()).Replace("\r\n", " ");
sw.Write(str);
}
else
{
sw.Write("");
}
if (i < iColCount - 1)
{
sw.Write(",");
}
}
sw.WriteLine();
}
sw.Flush();
sw.Close();
}
catch (Exception Ex)
{
throw Ex;
}
return ReturnStream;
}
Este es un método muy largo tiene más de 60 líneas de código. Nuestro objetivo será reducir el método a menos de 10 líneas de código. Comencemos, pensemos nuevamente en la cohesión, las cosas que deben estar juntas y cuáles no.
Veamos el primer bloque de código:
string strConn = ConfigurationManager.ConnectionStrings["FooFooConnectionString"].ToString();
SqlConnection conn = new SqlConnection(strConn);
SqlDataAdapter da = new SqlDataAdapter("SELECT * FROM [FooFoo] ORDER BY id ASC", conn);
DataSet ds = new DataSet();
da.Fill(ds, "FooFoo");
DataTable dt = ds.Tables["FooFoo"];Esta sección de código no tiene nada que ver con la generación de archivo CSV o la creación de la tabla. Entonces, podemos separarlo en un método. Pero antes de eso, nuestro método CreateMemoryFile() en realidad no pertenece a WebPage. Así que primero lo sacaremos a una clase.
Ahora, miremos un poco más en detalle este código. Tiene algunas líneas que están relacionadas. Como lo están, vamos a separarlas y extraerlas en métodos. Las primeras 4 líneas están relacionadas porque tienen que ver limpiar el response. Las siguientes 4 están relacionadas con el Cache. Las últimas están relacionadas con el contenido de la página web.
Response.Clear();
Response.ClearContent();
Response.ClearHeaders();
Response.Cookies.Clear();
Response.Cache.SetCacheability(HttpCacheability.Private);
Response.CacheControl = "private";
Response.AppendHeader("Pragma", "cache");
Response.AppendHeader("Expires", "60");
Response.Charset = System.Text.UTF8Encoding.UTF8.WebName;
Response.ContentEncoding = System.Text.UTF8Encoding.UTF8;
Response.ContentType = "text/comma-separated-values";
Response.AddHeader("Content-Disposition", "attachment; filename=FooFoo.csv");
Response.AddHeader("Content-Length", byteArray.Length.ToString());
Response.BinaryWrite(byteArray);
Nos quedarían los siguientes métodos.
protected void Page_Load(object sender, EventArgs e)
{
System.IO.MemoryStream ms = CreateMemoryFile();
byte[] byteArray = ms.ToArray();
ms.Flush();
ms.Close();
ClearResponse();
SetCacheability();
WriteContentToResponse(byteArray);
}
public void ClearResponse(){
Response.Clear();
Response.ClearContent();
Response.ClearHeaders();
Response.Cookies.Clear();
}
public void SetCacheability(){
Response.Cache.SetCacheability(HttpCacheability.Private);
Response.CacheControl = "private";
Response.AppendHeader("Pragma", "cache");
Response.AppendHeader("Expires", "60");
}
public void WriteContentToResponse(byte[] byteArray){
Response.Charset = System.Text.UTF8Encoding.UTF8.WebName;
Response.ContentEncoding = System.Text.UTF8Encoding.UTF8;
Response.ContentType = "text/comma-separated-values";
Response.AddHeader("Content-Disposition", "attachment; filename=FooFoo.csv");
Response.AddHeader("Content-Length", byteArray.Length.ToString());
Response.BinaryWrite(byteArray);
}
Finalmente, nos queda solamente el fragmento de código donde el stream es convertido en un array de bytes. Lo cambiaremos para que directamente sea pasado al siguiente método:
protected void Page_Load(object sender, EventArgs e)
{
ClearResponse();
SetCacheability();
WriteContentToResponse(GetCsv());
}
Ahora veamos el método que movimos a la clase MemoryFileCreator que tiene una gran cantidad de líneas. Empezaremos a trabajar con esta clase. Como antes, buscaremos los fragmentos de código relacionados. El primer fragmento que podemos separar:
string strConn = ConfigurationManager.ConnectionStrings["FooFooConnectionString"].ToString();
SqlConnection conn = new SqlConnection(strConn);
SqlDataAdapter da = new SqlDataAdapter("SELECT * FROM [FooFoo] ORDER BY id ASC", conn);
DataSet ds = new DataSet();
da.Fill(ds, "FooFoo");
DataTable dt = ds.Tables["FooFoo"];
Crearemos un método que se llama GetDataTable().
public class TableReader
{
public DataTable GetDataTable(){
string strConn = ConfigurationManager.ConnectionStrings["FooFooConnectionString"].ToString();
SqlConnection conn = new SqlConnection(strConn);
SqlDataAdapter da = new SqlDataAdapter("SELECT * FROM [FooFoo] ORDER BY id ASC", conn);
DataSet ds = new DataSet();
da.Fill(ds, "FooFoo");
DataTable dt = ds.Tables["FooFoo"];
return dt;
}
}
La siguiente línea vemos:
//Create a streamwriter to write to the memory stream
StreamWriter sw = new StreamWriter(ReturnStream);
Tiene un comentario innecesario. Es bastante obvio lo que hace, solo debemos eliminarlo. Siguiente fragmento:
int iColCount = dt.Columns.Count;
for (int i = 0; i < iColCount; i++)
{
sw.Write(dt.Columns[i]);
if (i < iColCount - 1)
{
sw.Write(",");
}
}
Vemos que estar armando las columnas a partir de los encabezados. Vamos a extraerlo a un método. Tenemos iColCount podemos cambiarla por un variable de línea ya que es utilizada muchos lugares. Ahora podemos seleccionar el mismo bloque y extraerlo en un método WriteColumnsName(dt, w). Para la siguiente línea vemos que no se usa la eliminaremos:
int intRows = dt.Rows.Count;Nuevamente tenemos un comentario que está explicando lo que hace el código y no está indicando que podemos extraerlo en un método. Lo llamaremos WriteRows.
public void WriteRows(){
for (int i = 0; i < iColCount; i++)
{
if (!Convert.IsDBNull(dr[i]))
{
string str = String.Format("\"{0:c}\"", dr[i].ToString()).Replace("\r\n", " ");
sw.Write(str);
}
else
{
sw.Write("");
}
if (i < iColCount - 1)
{
sw.Write(",");
}
}
sw.WriteLine();
}
Lo siguiente que deberíamos cambiar es el try…catch porque lo único que estamos haciendo es tomar la excepción y volverla a lanzar. No tiene sentido.
catch (Exception Ex)
{
throw Ex;
}
Directamente lo eliminaremos y nuestro código quedaría de la siguiente manera:
public MemoryStream CreateMemoryFile(DataTable dataTable){
MemoryStream ReturnStream = new MemoryStream();
StreamWriter sw = new StreamWriter(ReturnStream);
WriteColumnNames(dataTable, sw);
WriteRows(dataTable, sw);
sw.Flush();
sw.Close();
return ReturnStream;
}
Lo siguiente es,
var dt = GetDataTable();Este método trata de acceso a datos. No trata de crear un archivo CSV. La responsabilidad de nuestra clase es WriteColumnNames y WriteRows que tratan de crear un CSV. Si dejamos este método aquí estaremos perdiendo cohesión así que deberemos moverlo. Además, si lo movemos podremos reutilizarlo. ¿Pero no lo necesitamos? si, por eso nuestro método CreateMemoryFile ahora debe recibir este parámetro:
public MemoryStream CreateMemoryFile(DataTable dataTable){
MemoryStream ReturnStream = new MemoryStream();
StreamWriter sw = new StreamWriter(ReturnStream);
WriteColumnNames(dataTable, sw);
WriteRows(dataTable, sw);
sw.Flush();
sw.Close();
return ReturnStream;
}
Ahora lo separamos en otra clase que llamaremos TableReader. Ahora nuestra clase CreateMemoryFile trata de crear solamente un CSV. Por último, cambiamos el nombre por algo más descriptivo como DataTableToCsv y el método lo llamaremos Map.
public MemoryStream Map(DataTable dataTable){}Volvamos a nuestra webpage y modificamos para que podamos utilizar los cambios.
public partial class Download : System.Web.UI.Page
{
private readonly DataTableToCsvMapper _dataTableToCsvMapper = new DataTableToCsvMapper();
private readonly TableReader _tableReader = new TableReader();
protected void Page_Load(object sender, EventArgs e)
{
System.IO.MemoryStream ms = CreateMemoryFile();
byte[] byteArray = ms.ToArray();
...
Volvamos a GetDataTable. Tiene hardcoded la consulta. No es muy reutilizable. Podemos modificar el método para que reciba como parámetro el nombre de la tabla para rearmar la consulta y que sea reutilizable. Pero por el momento lo dejaremos así.
Ahora revisemos el método WriteRows. Veamos, tenemos varios if, un for dentro de un foreach también. Tomemos el fragmento del foreach y lo sacaremos dentro de un método WriteRow:
public void WriteRows(DataTable dr, StreamWriter sw){
for (int i = 0; i < iColCount; i++)
{
WriteRow(dt, sw, dr);
}
}
public void WriteRow((DataTable dataTable, StreamWriter sw, DataRow dr){
if (!Convert.IsDBNull(dr[i]))
{
string str = String.Format("\"{0:c}\"", dr[i].ToString()).Replace("\r\n", " ");
sw.Write(str);
}
else
{
sw.Write("");
}
if (i < iColCount - 1)
{
sw.Write(",");
}
sw.WriteLine();
}
En nuestro método WriteRow, dentro del foreach extraemos el primer if y llamaremos el método WriteCell:
public void WriteCell(){
if (!Convert.IsDBNull(dr[i]))
{
string str = String.Format("\"{0:c}\"", dr[i].ToString()).Replace("\r\n", " ");
sw.Write(str);
}
else
{
sw.Write("");
}
}
Luego el siguiente if lo extraemos con el nombre WriteSeparatorIfRequired:
public void WriteSeparatorIfRequired(){
if (i < iColCount - 1)
{
sw.Write(",");
}
}
Nos queda métodos mucho más pequeños:
for (int i = 0; i < iColCount; i++)
{
WriteCell(sw, dr, i);
WriteSeparatorIfRequired(DateTime, sw, i);
}
Aún podemos hacer más ajustes, pero se los dejaré para que lo piensen. El objetivo principal era reducir la cantidad de líneas por método y lo hemos logrado.
Conclusión
Podemos ver que nuestros métodos no deben ser tan largos y podemos ir dividiendo nuestro código para hacerlo más entendible.
En esta serie de post vimos una gran cantidad de temas sobre Clean Code. Espero que lo hayan disfrutado.
