Clean methods

If last week was about what does clean code means and how you write and use clean variables, today I will give some examples of how to think, write and clean your methods. (click here if you didn’t get the chance to read last week’s post or if you just want a refresh ūüôā )

Lets have an example:

void parseText(string inputFilePath, string outputFilePath)
{
       int linesToParse = 0;
       string line;
       List<string> result = new List<string>();
       List<string> auxiliar = new List<string>();

       System.IO.StreamReader file =
              new System.IO.StreamReader(inputFilePath);
       while ((line = file.ReadLine()) != null)
       {
              auxiliar.Add(line);
              linesToParse++;
       }

       file.Close();

       for (int i=0; i<linesToParse; i++)
       {
              line = auxiliar.ElementAt(i);
              result.Add(line.ToUpper());
       }

       System.IO.StreamWriter output =
              new System.IO.StreamWriter(outputFilePath);
       for (int i=0; i<linesToParse; i++)
              output.WriteLine(result.ElementAt(i));
}

Sure, from the name only you can tell that¬†the purpose is to parse text but honestly, don’t you see anything wrong there? I mean, it’s quite big (the function, not [only] the purpose). But is has less than 80¬†lines, so it shouldn’t be a problem, after this:

Known rule: No function should have more than 80 lines.

Of course, a function that has more than 80 lines is horrible, awful and should never be written, as important as it is, the main reason beeing that no one will ever understand it fully and completely only if she or he really has to. And why do we struggle to write good code if not so it’s beeing read and understood?


 

First things first: naming convention:

Rule: C# applies¬†Upper Camel Case principle on method’s and function’s names.

As you can see the function written above is written using Lower Camel Case so that’d be one of the first things that is wrong. Also, do you think the name of the function is propper for what it actually does? We’ll get back to this later.

Use descriptive names. You want to fully understand the whole function/method by reading the header, right? That’d sure save a lot of time.

Now that¬†the function’s name is changed and applies C#’s rule in method’s naming convention, what does it do? Does it only parse some text?

void ParseText(string inputFilePath, string outputFilePath)

Looking only at the function’s header, we see that it recieves two strings¬†accounted of¬†two file paths. So the function parses this strings? Or it parses the text in them? Or in both of them? Aa, yes, (I think) it actually parses the data from one file and writes it to another file. Yes, I believe that could be it’s meaning.

Ok, you can see what it actually does but… honestly, would you think of that in the first place or would you first ask yourself at least one question like I did? Yes, you probably would. And why would you? Clean code should be straightforward, as this is the word that best defines good code.

Functions should do one thing. They should do it well. They should do it only.

 

Yet by looking at the way our function¬†was initially written we can see it doesn’t do only one thing. And that is confusing – and it shouldn’t be.

Actually, it first reads from a file (there the inputFilePath recieved as a parameter), after which it uppercases all the lines in it, and after that writes the outcome into another file (and the outputFilePath role comes in).

The function’s¬†name describes only one thing that is beeing done, when actually recieves two files paths – therefore the confusion. But what if it’ll have what I like to call “subfunctions” that’s only do one thing? And would do it well?

Yes, but writing more functions would only add more lines to my code. Yes, it would, but no one said that’s wrong –¬†lets be serious, no one will only write a class or a program just to uppercase some string, would they? Would you? What’ll¬†be the purpose? You would write a program to do more than only one thing – so why not apply the last convention? Let your code do one thing at a time – it’ll do it well.¬†Not only that, but you could then reuse your code and as you would advance in your career you will see why this is important – because no code should be longer than it should.

Going back to the 28 lines of code above, considering the “do one thing” convention, I would write it like this:

void ReadFromFile(string filePath, List<string> result)
{
       string line;
       System.IO.StreamReader file =
              new System.IO.StreamReader(filePath);
       while ((line = file.ReadLine()) != null)
              result.Add(line);
       file.Close();
}

void WriteInFile(string filePath, List<string> text)
{
       System.IO.StreamWriter output =
              new System.IO.StreamWriter(filePath);
       for (int i = 0; i < text.Count(); i++)
              output.WriteLine(text.ElementAt(i));
}

void UpperCast(List<string> text, List<string> result)
{
       for (int i = 0; i < text.Count(); i++)
              result.Add(text.ElementAt(i).ToUpper());
}

void ParseText(string inputFilePath, string outputFilePath)
{
       List<string> result = new List<string>();
       ReadFromFile(inputFilePath, result);
       UpperCast(result, result);
       WriteInFile(outputFilePath, result);
}

You’re probably thinking “look at that, so many lines of code”. Yes, I’d give you that, but don’t you understand it better now? “ParseText first reads in a List all the data from the input file, and then it uppercases that data, after which it writes it in the output file.” See how it can be understood without even looking into the functions? Try that. If it works, your functions or methods¬†complete the “do one thing” rule. And it makes a huge difference. Just imagine how much time you could save by understanding a function so easily.

Another plus would be to put the¬†ParseText function first – so you can just scroll to actually see how¬†the functions ReadFromFile and WriteInFile are implemented (note that this applies in modern objected oriented programming languages –¬†C#, C++ and not in procedural ones – like C). This and also using regions gives code a much more clean texture.


 

But what about the parameters? Are they specific? Are they enough or too much? Before judging, lets better understand parameters’ best practice:

No function should have 763…29 parameters.

I think you would agree on that – who would memorize the order of 6 parameters, for example? Ok, anyone probably would if they really had to – but do we have to?

Ok, sure, IDEs like Visual Studio would give you a warning if instead of a string you would parse an integer to a function – but what if your function recieves int l, int t? (Maybe the person who wrote it didn’t apply clean code) and instead he meant¬†int numberOfLabeledGifts, int totalAmountOfGifts? Not even Visual Studio would help you in that case, and you would have to go to the function, figure out which argument is what, then get back to your function¬†call. Imagine doing that for 7 parameters. Yes, it would hurt, indeed.

Forthunately, you could avoid dealing with these kind of functions in your code: first step would be to have functions that only do one thing – and as our example does that, we’ll move on to the second step:¬†if you are doing only one thing, how could you have that many parameters? Cool, right? We managed to make our life easier in two ways by only writing more functions – which we were already were doing, just didn’t manage to gain everything from that approach!

Triads Functions: functions or methods that recieve three parameters.

Dyadic Functions: functions or methods that recieve only two parameters.

Triads are sometimes a bit too much – why? Even with only three parameters, you would have to be careful not to swich them by accident. (especially if you are a begginer – consider the case you code in C),plus the fact that you have to consider the return function – it’s still a bit too much – you most probably could break the function in more functions as it probably won’t do just one thing only. Don’t jump to conclusions before you are sure you absolutely cannot break it. ūüôā

As of dyadic functions, this was found to be the more used approach – two is enough but not too much – just the right amount of parameters.

You can see that our example has only dyadic functions. But as I mentioned above, what do they return?

void ReadFromFile(string filePath, List<string> result)
void UpperCast(List<string> text, List<string> result)
void ParseText(string inputFilePath, string outputFilePath)

Nothing? Oh, well, why not use the return parameter? It’s there to help us and also to make our code even more cleaner. Here’s how:

List<string> ReadFromFile(string filePath)
{
       List<string> result = new List<string>();
       string line;
       System.IO.StreamReader file =
              new System.IO.StreamReader(filePath);
       while ((line = file.ReadLine()) != null)
              result.Add(line);
       file.Close();
       return result;
}

void WriteInFile(string filePath, List<string> text)
{
       System.IO.StreamWriter output =
              new System.IO.StreamWriter(filePath);
       for (int i = 0; i < text.Count(); i++)
              output.WriteLine(text.ElementAt(i));
}

List<string> UpperCast(List<string> text)
{
       List<string> result = new List<string>();
       for (int i = 0; i < text.Count(); i++)
              result.Add(text.ElementAt(i).ToUpper());
       return result;
}

void ParseText(string inputFilePath, string outputFilePath)
{
       List<string> result = new List<string>();
       result = ReadFromFile(inputFilePath);
       result = UpperCast(result);
       WriteInFile(outputFilePath, result);
}

Doesn’t that makes it even more easier to understand? “ReadFromFile returns a list with the file information, while UpperCast returns the list given as a parameter with all the characters uppercased”.

Observe that now two of the written function recieve only one parameter.

Rule: The perfect function recieves only one parameter and returns the result of its applience.


 

Maybe at the first version of these functions you thought “but all those opperations could be done in only one – for example, you could UpperCase and write the output in a single opperation”. Yes, you could, but what if your program would have another function LowerCase whose input text would be read from a file and whose¬†result would also have to be written in a output file? In that case, you would have to write duplicate code for writing in the file. And I think we agreed last week that¬†clean code has no duplicate code.


 

Lets take another look at the functions’ headers – they all do contain a verb: read, write, cast, parse. Why is that? Because:

Functions (should) do something – as so their name should contain a verb which best describes what every function does.

Besides that, they are searchable this way and it is also easier to distinguish them from variables.

Also:

Use keywords.

And stay consistent – this will give you generality. Completely avoid using synonyms (at least in the same program).For example: don’t use get,¬†fetch, retrieve in the same program – you will forget what synonym you used in the class method’s when you’ll want to call them – and even if you can remeber them, why struggle when you could stay consistent and only use get?

Note that the right synonym describes perfectly the action so do take some time to consider the best option – it’s worth it. Also, if you believe you found a better synonym, just¬†change all it’s appearences.


 

Spacing and indenting are also very important – obviously you can set your own coding style in spacing and newlining, I will just make some modifications on how I think these functions should be spaced:

List<string> ReadFromFile(string filePath)
{
       List<string> result = new List<string>();
       string line;

       System.IO.StreamReader file =
              new System.IO.StreamReader(filePath);

       while ((line = file.ReadLine()) != null)
              result.Add(line);
       file.Close();

       return result;
}

void WriteInFile(string filePath, List<string> text)
{
       System.IO.StreamWriter output =
              new System.IO.StreamWriter(filePath);

       for (int i = 0; i < text.Count(); i++)
              output.WriteLine(text.ElementAt(i));
}

List<string> UpperCast(List<string> text)
{
       List<string> result = new List<string>();

       for (int i = 0; i < text.Count(); i++)
              result.Add(text.ElementAt(i).ToUpper());

       return result;
}

void ParseText(string inputFilePath, string outputFilePath)
{

List<string> result = new List<string>(),
inputFileData = new List<string>();

       inputFileData = ReadFromFile(inputFilePath);

       result = UpperCast(inputFileData);

       WriteInFile(outputFilePath, result);
}

Separation:

  • separate the declarations from the rest of the function;
  • use newline between different opperations (opening the inputFile and reading from it);
  • separate the returning of the function so it can be better seen;

Besides that, notice that I declared another List<string>¬†inputFileData to save the initial data from the file – do I need to do that? I think not, because even if I maybe wanted to save the file’s content I can anytime call the¬†ReadFromFile function again. ūüôā (Afterall, I wrote it to use it – preferably not only once)

 

As for the perfect function, word is the best function only has one line. Take this for example:

bool isListEmpty(List<string> text)
{
       return (text.Count() == 0) ? true : false;
}

It looks awesome, if you ask me – even though it only does a check, it is pretty amazing. If you don’t think that, compare it with the following:

bool isListEmpty(List<string> text)
{
       if (text.Count() == 0)
              return true;

       return false;
}

Or:

bool isListEmpty(List<string> text)
{
       if (text.Count() == 0)
              return true;
       else return false;
}

 

Write smart code, don’t look just like the others programmers.

 

As I said earlier, do you think ParseText is a good name for the main function? Comment¬†with a better ideea. ūüėÄ

3. Comments

To be continued… here.

Leave a Reply

Your email address will not be published. Required fields are marked *