4

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?

Ayub
  • 169
  • 10
  • 1
    In such a simple case, very little is at stake, and it will be a matter of preference - I wouldn't write a separate method when you could have just inlined `Console.WriteLine(new String("*", 10))`. It is the overall architecture where the stakes become extremely high, and you ideally want some flexibility to change and adapt in future without radical rewrites or total replacement. But it also pays not to bite off more than you can chew - complex, hand-rolled frameworks that bear no relation to any known future requirement, may well inflict more development effort than it was designed to save. – Steve Aug 03 '19 at 15:57
  • 1
    This is a reasonable generalization based on your understanding of what the program needs to do, and a small enough thing that it can be rewritten if need be, assuming some care had been taken to contain usage (i.e., it's not called all over the codebase). Over-engineering is when you do this on a somewhat larger scale, resulting in more complicated dependency structures, and certain tradeoffs being made, before you have enough evidence to suggest that the future you have designed for (i.e., the kinds of changes you made easy to do) is the future that will actually happen. – Filip Milovanović Aug 03 '19 at 16:06
  • 2
    If you want to future proof stop hard coding output to console. – candied_orange Aug 03 '19 at 20:42

2 Answers2

9

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.

Theraot
  • 8,921
  • 2
  • 25
  • 35
  • Thanks a lot. Are you saying that we're authorized to write Spaghetti code some time? – Ayub Aug 03 '19 at 13:19
  • 3
    @kokabi That is taking a loan. I mean, that is getting technical debt. It could take you out of a tight spot in the short run. However, you will have to pay for it, eventually. You need to learn to refactor. Once you know what the required change is, you can use refactoring to change the code into equivalent code that is easier to change in the required way. That will keep spaghetti down. However, please notice, it depends on knowing what the required change is to know what refactor to do. Edit: by tight spot I was thinking of hitting a dead/live line. – Theraot Aug 03 '19 at 13:23
  • That's not a functional programming function. That's a good old side effect loving subroutine. – candied_orange Aug 03 '19 at 20:45
  • @candied_orange I was trying to apply and idea from functional programming. However, sure I can make it more function-like. – Theraot Aug 04 '19 at 02:21
  • @Theraot I'm not insisting on how you do it. Just want it called what it is. – candied_orange Aug 04 '19 at 02:23
4

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

Philip Kendall
  • 22,899
  • 9
  • 58
  • 61