Golden rules in programming

[AndrewB]

- Never have one visual component access another directly.
For example, when putting a "Submit" button on a form, never have the code for the "onClick" event of the button directly access a checkbox (for example, an "Automatically parse URLs" checkbox) to see what its value is. The value should be stored in a central location that both the checkbox and the submit button can access. If the Submit button directly accesses the checkbox, then you have a problem. The problem is that you are "cementing" your program. If you ever decide to change the checkbox to, say, a pull-down (yes/no) menu, then not only do you have to redesign the checkbox, you also have to go into the Submit button and change its code. Not only that, you have to go into EVERY component that accesses the checkbox value and change the way they work.

People almost NEVER follow this rule. While technically, it WILL work to have one visual component check the value of another, there is ZERO added benefit to doing it this way, and it only makes it harder to change your program in the future. NEVER allow your visual components to talk to each other.


- Never do repetitively what you can do in a loop or in a group
If you are designing a game, and you want it to allow for four players, do not design for four players. Design for n players. Do NOT engage in the practice of writing methods like player1Win(), player2Win(), player3Win(), etc... What you SHOULD do is something like playerWin(playerNumber).

Likewise, if you want every player to see a "Game over" message pop up on their screen, do NOT do this:

player1.ShowMessage("Game over!");
player2.ShowMessage("Game over!");
player3.ShowMessage("Game over!");
player4.ShowMessage("Game over!");

What you SHOULD do is something like this:

Player[] listOfPlayers = {player1, player2, player3, player4};
For Each currentPlayer As Player In listOfPlayers
    currentPlayer.ShowMessage("Game over!");
Next

And that will work regardless of how many players you have. If you ever want to add more players, then there is very little that needs to be changed.


- Never repeat code
If two objects, let's call them "Imp" and "Cacodemon" have the exact same method for, let's say, getHitPoints(), then don't paste the same function into both of their code files. Since all monsters' getHitPoints() function work exactly the same way, then create a "Monster" object with that method and have all monsters use that method.


These are just some of the important things to learn and remember.

[Kaiser]

I find that first rule pretty interesting..

[AndrewB]

Fredrik, I know you have something to say, so spit it out.

[Fredrik]

Thank you for going through such an effort to state the completely obvious.

[AndrewB]

You might call it obvious if weren't for the fact that around 98% of software breaks these rules.

[Lord FlatHead]

Mine doesn't.

[Fredrik]

That's not surprising, considering that around 98% of programmers, not to use harsh words, are in the wrong business.

[AndrewB]

Umm, so we're in agreement.. I think.

But as sensible as all of this sounds, don't consider it to be "obvious." These are concepts that apply to data processing, not to ordinary life.

[Fredrik]

Good. You might want to have a look at this brilliant site: http://thedailywtf.com

This might be the one that made me laugh the hardest: http://thedailywtf.com/ShowPost.aspx?PostID=26368

[Linguica]

I code like shit but then no one pays me to do it so I don't care!

The Doomworld updating scripts are pieces of crap that I wrote in a few hours one day in 2000 but I don't care!

[insertwackynamehere]

I'm lost. At first I thought that code on fredrick's site (second link) was C++ but then they start talking about VB O_O

[AndrewB]

switch ( Time.GetYear() )  
{
       case 2000: m_TagDate.Year = 0x00; break;
       case 2001: m_TagDate.Year = 0x01; break;
       case 2002: m_TagDate.Year = 0x02; break;
       case 2003: m_TagDate.Year = 0x03; break;
       case 2004: m_TagDate.Year = 0x04; break;
       case 2005: m_TagDate.Year = 0x05; break;
       case 2006: m_TagDate.Year = 0x06; break;
       default : m_TagDate.Year = 0x00; break;
} 

Wow. This is almost malicious. The guy KNEW that he was only designing it to work until 2006 and then 'kaput!' (always think it's 2000) when 2007 came along. He figured that he would be long gone by then and that it would no longer be his problem. Way to drop a timebomb.

[AndrewB]

insertwackynamehere said:

I'm lost. At first I thought that code on fredrick's site (second link) was C++ but then they start talking about VB O_O

The excerpt was C++. They were proving that that, yes, C++ can have horrible code too. Any language can have horrible code. I could open Visual Studio .NET right this second and write a program that doesn't work. That's the whole point; blame the programmer, not the language.

[Fredrik]

Also note what happens near the end of m_SetMinute:

      case 56: m_TagTime.Minute = 0x38;break; 
      case 57: m_TagTime.Minute = 0x39;break; 
      case 58: m_TagTime.Minute = 0x3f;break; 
      case 59: m_TagTime.Minute = 0x3a;break; 
      case 60: m_TagTime.Minute = 0x3b;break;

[AndrewB]

I noticed that.

What the heck was this designed for, anyway? Surely, that f..a..b thing would throw something into turmoil. What were the end results of this coding disaster?

[DOOM Anomaly]

AndrewB said:

player1.ShowMessage("Game over!");
player2.ShowMessage("Game over!");
player3.ShowMessage("Game over!");
player4.ShowMessage("Game over!");

I have a bad habit of doing that. Everytime I try to make something like that into a nice package with the player list dee-lee it goes bonkos and I get hurled futher into confusion. :D
Though, I should try it more, I'm bound to get it right some day. :D

[AndrewB]

Tell me what language you're trying to use, because I melted about three languages together in my example above.

[Remilia Scarlet]

While I did notice the last sentence saying, "These are just some of the important things to learn and remember," I would have thought surely that useful comments would have been included.

I believe that it's the O'Riley series which stresses that bad code with good comments is better than good code with bad comments.

[spank]

I don't know... hard coding stuff like that is such a hack it shouldn't even be considering actual programming.

On the bright side, notice how the program is inherently O(1) :D

[AndrewB]

Oh yes, I almost forgot... Always echo every line of your code, describing in comments what that line does. (Don't forget to comment your braces.)

[Szymanski]

Also remember to add insults in your comments, this builds moral.

[Fredrik]

/**
  @author Fredrik
  @description In case 1, set m_TagTime.Minute to 0x01 and exit the switch statement
  @purpose Set the value of m_TagTime.Minute to 0x01
*/
case 1: m_TagTime.Minute = 0x01;break;

/**
  @author Fredrik
  @description In case 2, set m_TagTime.Minute to 0x02 and exit the switch statement
  @purpose Set the value of m_TagTime.Minute to 0x02
*/
case 2: m_TagTime.Minute = 0x02;break;

...

[AndrewB]

As for that "time" thingy, wouldn't this be the equivalent of what the moron is trying to do?

void m_SetYear ( CTime &Time ) 
{ 
   m_TagDate.Year = (Time.GetYear() - 2000);
} 

void m_SetMonth ( CTime &Time ) 
{
   int month = Time.GetMonth();
   if (month >= 1 && month <= 12)
   {
      m_TagDate.Month = Time.GetMonth();
   }
}

void m_SetDay ( CTime &Time ) 
{
   int day = Time.GetDay;
   if (day >= 1 && day <= 31)
   {
      m_TagDate.Day = Time.GetDay();
   }
} 

void m_SetHour ( CTime &Time) 
{ 
   int hour = Time.GetHour();
   if (hour >= 0 && hour <= 24)
   {
      m_TagTime.Hour = hour;
   }
} 

void m_SetMinute( CTime &Time ) 
{ 
   int minute = Time.GetMinute();
   if (minute >= 0 && minute <= 60)
   {
      m_TagTime.Minute = time;
   } 

} 

void m_SetSecond( CTime &Time ) 
{ 
   int second = Time.GetSecond();
   if (second >= 0 && second <= 60)
   {
      m_TagTime.Second = second;
   } 
} 

[Fredrik]

That's still an awful lot of code to do absolutely nothing.

Without knowledge of how the functions were used elsewhere in the program, we'll never know for sure.

[Cyb]

Linguica said:

I code like shit but then no one pays me to do it so I don't care!

The Doomworld updating scripts are pieces of crap that I wrote in a few hours one day in 2000 but I don't care!

tell me about it. if marv decides to turn off register_globals off there will be a lot of fun to be had :P

edit: what the fuck. I just looked at that example fredrik posted... that's the most retarded thing I've ever seen. First of all the CTime::CTime object (which is what I assume he is using; there's also a similar struct avaliable in time.c) stores all the fucking digits for mins, hours, days, seconds and so on, that what he did was not only 100% retarded but 100% needless.

edit again: btw you could compress every one of those functions to be two lines of code, though it would make sense to stick them all in one function, there's really no need to do bounds checking with CTime since I believe it gets all those values via a unix timestamp... though why he is using his own time structure which appears to be identical to CTime (yet is also using CTime) is a mystery

[insertwackynamehere]

@cyb: time.c or time.h ?

[Cyb]

.h is the header, I mean I should have said time.h since that's what you need to include, and that's the custom for saying those types of things, but if you want to get technical all the functions are probably in time.c, but I meant time.h :P

including time.c (or any .cpp/.c file), btw, is another thing you can do wrong