Sunday, December 22, 2013

"DONE" - Thoughts on Software Estimation

When developers are asked to estimate the effort & time required for a change request, or fixing a bug, they give some numbers. And starts working on it and says its done. But actually there is more to do in it.

For example:
-------------
1. It should be tested, integrated, and pass the QA
2. The code also needs to be reviewed & refactored (if the developer/company does't follow this practice as part of their regular development)
3. Finally it should be accepted by the customers

But developers don't consider all of these and gives estimates. But the Parkinson's Law applies and the project automatically pulls the effort and time actual required from the developers.

I usually tell the developers that you have the right to give the actual time required for it. Need to shrink and give false estimates which are less. Take all of those into consideration and tell the time required for it.

Let me give an example of what DONE means. If we want to buy an Air-conditioner for our bed room, when we are done? We are done only when cool air is flowing out of the A/c and our room is getting cool.
To complete the above task, we need to take all of the following steps into consideration for estimating the cost and time required.

1. Search (online or visit a nearest store) for the needed capacity, model, brand, of the affordable budget, quality, service availability etc.
2. Go and place and order for it.
3. Get it transported to our house.
4. Get the required electrical wiring & man-power to do it.
5. Actually mount the A/c and connecting it the mains.
6. Finally the A/c need to be tested and accepted/approved and we then pay the bills

There could be few more or less steps involved in getting the room cool then only we can say it is DONE.

Saturday, December 21, 2013

More on Dead Code

There is one more type of dead code which is sometimes automatically detected by today’s IDEs those are like condition based. A code fragment runs only when a certain condition is met. This gets created based on the developer's initial thoughts in the first-time. Latter the code gets modified by the same developer after few weeks/months or by some other developers.

This happens because of the same reason as I mentioned in earlier post that they fear to remove and refactor the code base. The moment it starts working they just close it and say it’s done!

Example:

var customerIsPreferred = true;
if (customerIsPreferred)
{
     DoSomething();
}
else
{
     DoSomethingElse();
}


Write less code to produce less bugs

I often see the developers write more code which leads to more bugs. My previous post was regarding a code-smell in that I talked about unnecessary lines of code fed into the logic even it is not necessary. Those lines of code and steps were brought as part of building the initial algorithm/logic to make it work. But fail to remove/compact it by refactoring.

This leads to more code to be maintained and leads to creep more bugs. The principle is writing less code hence it will have less bugs. Writing more code, producing more bugs and spending more time in fixing them over and over are a bad practice.

Every method in the class, and every line in the method should have a strong reason to exist in there and should scream for its existence. -- You get the point.

So write less code, keep it short, simple, and produce & fix less bugs, save time!


"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.

On Giving Intention Revealing Names

As I see and reviewing more code (Written by an amateur programmer to very senior developer) I noticed why they cannot come up with good names even though they are willing to do so and be able to spend little time and effort too.

They are two things:

1. They are not well-versed with the domain terms and vocabulary. They spend very little time on understanding it. They very soon bring the technical jargon in to their mind and starts thinking in implementing with it. In the initial phase of the project where the business requirements are discussed, they spend very less time in those discussions and starts thinking about tools and technologies can be used.

They eventually jump into the development and use the names of whatever at the time they know. Latter even if they become aware of the full business domain terms they don't really dare to touch the code and change the names. Then they forget it or ignore it and come under schedule pressure to add more code.

2. In some countries (e.g. India), English is taught as second or third language. Even somebody study in full English-medium still it is seldom used in day-to-day conversation and communication. They study it just for passing the exams. The problem is again with the strong vocabulary hence it becomes very hard to come up with good names very fast or immediately while they are programming.

They don't spend time in thinking for good, short, straight-forward, condensed names and they use whatever words they know and abbreviate the names by prefixing and suffixing with numbers, or omitting vowel (e.g. for customer to cstmr).  They make lot of spelling mistakes also.