Seriously what do these numbers mean to anyone else looking at your code, or in fact to yourself when you come back in a week or two (in my case a day)?
Damn magic numbers and random variable names, awesome tactics if you want to spend time looking at the same code day in and day out trying to figure out what they mean, especially when the same number means two different things in the database or even three. Genius!
Use an enum damn it! Give those numbers meaning! Tell me what they are! Tell yourself what they are! Rather than me/you hunting through the damn code to try and work out what they mean!
Name your variables something useful, and your methods too!
not so nice way...
15 public void CheckEngineManagement()
16 {
17 //Call something to get the data of the engine.
18 var engineData = GetEngineData();
19
20 if (engineData.State > 0)
21 {
22 if (engineData.State == 1)
23 {
24 //Do some engine checks for oil,
25 //seatbelt checks etc, blah blah,
26 //or something else
27 }
28 }
29 }
I seem to be stuck in the car world at the moment, I will have to think of some better examples.
So there are the different types of EngineData coming back from the database, all in the same table with a column to state the different type. Maybe bad maybe not, but I am trying to find an example.
I get my engine data back for a particular car, ferrari engineData, superb, then I do a check on the State if it's less than zero don't do anything, if it's 1 then do something. IF IT'S 1, IF IT'S 0 WTF does that mean? The door is open, ignition is on, car started, what!? Especially for different engine data 1 could mean something else and it probably does when written magically!
nicer way hopefully...
8 public enum CarState
9 {
10 CarLocked,
11 PassengerOpen,
12 DriverDoorOpen,
13 KeyInIgnition,
14 }
23 public void CheckEngineManagement()
24 {
25 //Call something to get the data of the engine.
26 var engineData = GetEngineData();
27
28 if (engineData.State != CarState.CarLocked)
29 {
30 if (engineData.State == CarState.DriverDoorOpen)
31 {
32 //Do some engine checks for oil,
33 //seatbelt checks etc, blah blah,
34 //or something else
35 }
36 }
37 }
Ignore the nasty if nesting, I will have to think of a better example, but I can't at the moment I am fuming!
I have taken away all the nasty numbers, now you can read that the car is locked or the driver door is open and see what its trying to do and why its doing its checks for oil, seatbelt checks etc. not having to spend a larger amount of time understanding the whole code to work out what the numbers are and why they are there!
Sooner or later those ifs would have ended up all over the place nested like crazy with a mix of the engine data type amongst it all no doubt, can you imagine trying to read all that lot with just numbers?
Give me a slap for nesting if's like that, probably could have used a switch or probably even something else. Or suggest a better idea for my demonstrations. Or just a slap anyway. I am just trying to get some of the simpler things across at the moment.
No comments:
Post a Comment