Question

Look at this code:

Console.WriteLine(user.name);
Console.WriteLine("**********"); // 10 stars
//...

Tomorrow, I needed to use 20 stars instead 10 stars. Okay I would change it:

Console.WriteLine(user.name);
Console.WriteLine("********************"); // 20 stars
//...

It gives me an idea to write a method for this work:

public void DisplayStars()
{
   Console.WriteLine("********************");
}

Console.WriteLine(user.name);
DisplayStars();
//remaining code

But when I'm writing code, I think to myself: isn't that a better solution to add a new parameter for getting number of repetition of character like int count? How about adding another parameter to get repeating character like char whichChar?

public void DisplayStars(int count, char whichChar)
{
   string s=new String(whichChar, count);
   Console.WriteLine(s);
}

Why int? It seems uint is better type. No No, byte is best. What if user passes an empty character, What if he\she passes too big (or small) number for count? And these goes on...

I often add some code because I think that I might need it in the future. These is my day-to-day programming challenges. Am I Over-Engineering? Am I perfectionism? If not, what is Over-Engineering?

Was it helpful?

Solution

What if they want "/\/\/\/\"? that is not repeating a single character, but a combination of two.

How about "◀◼◼◼◼▶"? Oh, no! you need to allow a different starting and ending characters too.

What if the client wants to sell it as advertisement space? Ok, that is risible, let's step back.


Yes, you are over-engineering. First of all, I would argue that these changes actually make it hard to maintain.

If you keep "**********", then changing it is easy. When the moment comes that a new requirement arrives, you can just replace the string.

I would argue that if you have "**********" in multiple places, then making it a constant or extracting it a method could make sense. That is a single source of truth※. If you have reason to believe that all these strings will change togheter, then do that. Why? Bacause it makes it easy to change them all at once, there is no risk you forgot one, or anything like that.

※: Some people will invoke Don't repeat yourself instead. But then you can say that you are repeating yourself by using the constant or calling the method... look, DRY is a tool that helps us get to a single source of truth. It is a means, not a goal. Do not take to the extreme.

However, beyond that, you are making argument about what the client will want. You do not know. Every change you make to ease a possible future change, will actually make hard to do other changes... and since you do not know what the changes are, you could be setting yourself up for a situation in which you need to undo your work to actually satisfy the new requirements.


Alright, there could be an argument for possible changes. Certainly, changing from "**********" to "----------" makes more sense than selling it as advertisement space. I have two things to say about that idea...

  • It has an opportunity cost. You could be using the time you are using to implement these hypothetical future requirements in implementing other currently actual requirements.
  • You should not venture in speculation wildly. Once you are done with the actual requirements, you might aswell - and I would argue you should - ask the client if they like the product and if they want to do any changes.

Let us take a lesson from functional programming...

You can take this:

public void DisplayStars(int count, char whichChar)
{
   string s=new String(whichChar, count);
   Console.WriteLine(s);
}

And turn it into a method with the following signature:

public void DisplayStars()

Addendum: In fact, extract that dependency to Console, so you have a method that works as a good old pure function (asumming you are not taking the value from an external system), with the following signature:

public static string DisplayStars()

I would argue that this is equivalent to a function that maps void to a single string. And, no, don't tell me void, I mean unit, does not exist.

By setting the values of its parameters.

Then, when the day comes, and it turns out all this you did is wrong, you can just change the call to DisplayStars.

That is right, there is a colorary to all this: make something easy to destroy.


And, yeah, yeah, YAGNI.

OTHER TIPS

Yes, you're over-engineering this. Build what you need today, or at most what you know you're going to need in the near future. Otherwise as you've discovered, you spend all your time building abstractions which you never use.

Or the short version: You Ain't Gonna Need It (YAGNI).

Licensed under: CC-BY-SA with attribution
scroll top