Saturday, December 21, 2013

"Dead Code": An interesting code-smell

Over the time, as I work more closely with the development team, I saw an interesting code-smell again and again and it is called "Dead Code"

In a given method there are many lines which are not just required. I guess we can call them dead-code but it is not that those are not executing. Those lines are executing but just required at all. The developers are just not aware of those lines. They are not interested in giving a closer look into it). They built them in the code in the process of initial thoughts in building a particular functionality (as part of the trial and error of a particular logic implementation). The developers stopped and leave them as it is once the method gives the intended result even though the other steps are not utterly needed.

It seems the developers are afraid of touching the code once it starts working. They seem bit lazy in cleaning up the code by removing non required lines. I see this happens even in the case of existence of some automated unit tests also.

Example-1: The following code fragment I copied and pasted from the actual project.

public static List<string> GetColumnsList(string columnnames)
{
    List<string> ListColumnNames = new List<string>();

    if (columnnames != string.Empty)
    {
        if (columnnames.Contains(';'))
        {
            string[] arrColumnNames = columnnames.Split(';');
            ListColumnNames = arrColumnNames.ToList();
        }
        else if (columnnames != string.Empty)
        {
            ListColumnNames.Add(columnnames);
        }
    }

    return ListColumnNames;
}

NOTE: There are many code-smells in the above code (even some formatting issues which I corrected for the sake of this post) but I'm interested in showcasing the particular problem that I am discussing about in this post.

After refactoring the above code (by removing the extra code) it came to only one line as given below.

return columnnames.Trim().Split(';');

Example-2: Below is the method from a class I copy-pasted from one more project.
NOTE: Don't look at the quality of the following method. It is horribly written.

private bool PrintFile(byte[] bytes)
{
    bool result = false;
    try
    {
        MemoryStream objstream = new MemoryStream(bytes);
        string currentText = ""; //Convert.ToString((char)29) + " " + Convert.ToString((char)40) + " L" + (char)1;
        StringBuilder text = new StringBuilder();
        PdfReader pdfReader = new PdfReader(objstream);
        List<byte[]> ListByteArray = new List<byte[]>();
        for (int page = 1; page <= pdfReader.NumberOfPages; page++)
        {
            string currentPage = string.Empty;
            byte[] pageContent = pdfReader.GetPageContent(page);
            ListByteArray.Add(pageContent);
            ITextExtractionStrategy strategy = new SimpleTextExtractionStrategy();
            currentPage = PdfTextExtractor.GetTextFromPage(pdfReader, page, strategy);
            currentPage = Encoding.UTF8.GetString(ASCIIEncoding.Convert(Encoding.Default, Encoding.UTF8, Encoding.Default.GetBytes(currentPage)));
            currentText = currentText + currentPage;
            text.Append(currentText);
        }
        if (!string.IsNullOrEmpty(currentText))
        {
            string GS = Convert.ToString((char)29);
            string ESC = Convert.ToString((char)27);
            string cutCommand = "";
            cutCommand = ESC + "@";
            cutCommand += GS + "V" + (char)1;
            currentText += "\n" + "\n" + "\n" + "\n" + "\n" + cutCommand;

        }
        pdfReader.Close();
        int nlength = bytes.Length;
        PrintDialog pd = new PrintDialog();
        pd.PrinterSettings = new PrinterSettings();
        pd.PrinterSettings.PrinterName = PrinterName;
        ErrorLogger.WriteLog(ErrorLogger.LOGTYPE.INFO, "Before Send Text" + currentText);
        if (RawPrinterHelper.SendStringToPrinter(pd.PrinterSettings.PrinterName, currentText) == true)
        {
            ErrorLogger.WriteLog(ErrorLogger.LOGTYPE.INFO, "After Printing Return value is true");
            result = true;
        }
        else
        {
            ErrorLogger.WriteLog(ErrorLogger.LOGTYPE.INFO, "After Printing Return value is false");
            result = false;
        }
    }
    catch (Exception ex)
    {
        ErrorLogger.WriteLog(ErrorLogger.LOGTYPE.ERROR, "Before Send to Printer", ex);
    }
    return result;
}


In the above method the following lines were declared and filled but never used after that.

StringBuilder text = new StringBuilder();
List<byte[]> ListByteArray = new List<byte[]>();



Example:-3: I copy-pasted the following
method from a class in another project.





public List<string[]> Read(string filePath)
{
    List<string[]> result = null;
    StreamReader reader = null;
    try
    {
        result = new List<string[]>();
        reader = new StreamReader(filePath);

        string line = string.Empty;
        int index = 0;
        while ((line = reader.ReadLine()) != null)
        {
            if (index == 0)
            {
                line.Skip(1);
            }
            else
            {
                var columns = line.Split(',');
                result.Add(columns);
            }
            index++;
        }

    }
    catch (Exception ex)
    {

        ErrorLogger.ErrorLogger.WriteLog("Read", ex);
        if (ex.InnerException != null)
            // ErrorLogger.ErrorLogger.WriteLog("Read", ex.InnerException);
            return null;
    }
    finally
    {
        reader.Close();
    }
    return result;
}

In the above method, the variable named 'line; is used only in case of index is zero. The line is never used but the value of the line is processed.

No comments:

Post a Comment