C++ Coding Standard: The If Block

Almost two months ago I went to a CocoaHeads meeting during Macworld. They had Mike Lee talk. His presentation was about “Pimping Your App”. There were a bunch of interesting points, but one thing really stuck in my head. Mike was talking about how is a messy person in his life. His car is messy. His room is messy. His desk is messy. Everything is messy, except his code. HIs code is crystal clean, squeaky even. As a programmer you need to make sure your code is consistent and clean. Ever since I have been thinking about standardizing the way I write code. This is the first post in hopefully a stream of posts about quality code.

Yesterday, there was a discussion in the office about code quality. There were many points discussed, topics like line length, white space, and my personal favorite “if” statements. I have a track record of being incredibly inconsistent with my “if” block. The basic if block is the following:

if(foo) {
  //...
}

The “if” statement by itself is really not that big of a deal. It’s when you start adding “else if” and “else” clauses that it becomes complicated. The problem for me is twofold. Firstly, I’m inconsistent. The type of blocks I use on larger sections of code is different from the blocks I use on smaller sections of code. Secondly, my desire for consistency is at odds with my crazy, cooky desire to have code look aesthetically pleasing.

The Condensed If Block

I sometimes use the following style of the if statement:

if( foo ) {
   //...
} else if( bar ) {
   //...
} else {
   //...
}

This block is very condensed. You throw the braces for each clause on it’s own line. I feel that it this type of statement makes it clear and easy to really make the if statements a smaller part of the code. On trivial if blocks, I really like this approach. Where it suffers is in more complex if statements. If the ifs and else ifs fit really just blend into the code, sometimes making it easy to miss them. Okay, so if I’m looking at the code in detail, not a big ideal, but if I’m just giving it a quick glance over I might miss something. Also, if the “if” statement line is long enough, it could easily blend into the line below it.

The Almost Condensed If Block

This one is a take on the Condensed If block. It is actually just really poorly formatted “if” statements, but I often find myself using this one:

if( foo ) 
{
   //...
} else if( bar ) {
   //...
} else {
   //...
}

All we are doing here is moving the opening brace from the end of the if statement to the next line. The rest of the code follows the condensed. So, I really like this approach because the first “if” block is clearly separated from the rest of the code. It’s clear that we are are entering an “if” statement line. The remainder of the statements don’t take up too much space. The closing brace, the else if/else, and the following opening brace are all on the same line.

The downside of this approach is that it looks inconsistent. Why does the initial “if” statement get one extra return, and all the subsequent lines statements get jammed into one line? It’s not functionally different, and it might in general be more appealing to me, but consistency is also important. I have started to shift away from using this style.

The Intermediate If Block

This one has more white space:

if( foo ) {
   //...
}
else if ( bar ) {
   //...
}
else {
   //...
}

This approach has gives you a little bit more separation of the control statements from the code. For some reason, though, I just feel it looks weird to write the close brace on its own line but then incorporate the opening brace on the same line as the control statement. Still feels inconsistent.

White Space Heaven If Block

The following block is the last if style I’m going to talk about:

if( foo ) 
{
   //...
}
else if( bar )
{
   //...
}
else 
{
   //...
}

This block takes up a very large amount of space, everything gets its own line. It is probably the clearest of all the examples above, but the trade off is that your code is now three lines longer for each else statement. This extra space means that you can theoretically fit less code in the same amount of screen space.

Additional Concerns

There are a couple of additional concerns when working with if blocks. For example, if you are chaining “if” blocks, how much space should you provide.

// What I don't like
if(foo)
{
   //...
}
if(bar)
{
   //...
}

// What I prefer
if(foo)
{
   //...
}

if(bar)
{
   //...
}

I have run into some people who prefer the first option above with no space between each block. The reason I dislike this so much is that depending on the method of “if” syntax you use, you second if block could look identical to your else blocks. As your code gets large and complex, it is important to make it as easy to discern different control paths that might be executed, and in this case, white space is really your friend.

Another concern is the ternary operator. This operator is basically a simple “if/else” block which fits on one line.

double number;
if(foo)
    number = 3.1415;
else
    number = 2.71828183;

// As opposed to
double number = foo ? 3.1415 : 2.71828183;

Ternary operators are great for doing assignments as in the one line above, but be careful with trying to do too much in one line. My general rule is the line is getting really long, or I’m doing multiple clauses in my condition I tend to shift away from ternary operator.

Conclusion

The problem with all of this is that my mind changes depending on where I am in, what I’m working on and what I plan to be doing with my code. I’m learning towards using the condensed form of the “if” block in the future for the simple reason that I can fit more lines of code in less space (not to mention that some pointed out it was the K&R way, too). Bottom line, whatever method you choose, you should really stick to it as best you can for each project. If you open up a file in a project that was edited by someone else, you should probably follow their precedence.

Stupid std::vector Class

The Standard Template Library in C++ is nice to provide us with a bunch of different container classes so we don’t have to re-invent the wheel every time we write new code. One of the classes is called “Vector”, if you aren’t familiar with it, you might not get too much out of this post. But basically, it is a dynamically growing array. Meaning it has contiguous memory and can be indexed like a regular array. It’s a great class and I use it all over the place, but for the second time in one week I find it lacking.

I’m working on some code where I have two vectors of the same type. I want to concatenate one on the end of the other.

  std::vector firstVector;
  std::vector secondVector;

  // I want to do the following, but its not legal
  firstVector.append(secondVector);

This doesn’t work. There is no append method for std::vector. It turns out that the correct code for this is:

  std::vector firstVector;
  std::vector secondVector;

  firstVector.insert(firstVector.end(), secondVector.begin(), secondVector.end());

This frustrates me. Append seems like a logical function to include. I’m relatively new C++ developer, having been working in the language intensely for only about a year. My guess is I will reverse my opinion over time, but it just seems like an append() makes sense.

There is also the potential benefit, depending on the implementation of std::vector. Insert requires you to pass in three iterators. Because an append function would have access to the internals of each vector you wouldn’t necessarily need to do the look up of each iterator, which might save a few lines of code. These lines of code might be dwarfed by the amount of code you would need to check the parameter being passed to an append function, I’m not sure.

Regardless of whether or not it is more efficient, I still feel like append() makes sense, conceptually and I would like to see it in future versions of the STL.

(again, I reserve the right to change my opinion in the not too distant future)

Leap Year Spells Trouble for Zune Users

On December 31st, all 30GB Zune users woke up to their music players not working. In a rarity for Microsoft problems, the source for for the problem was found. There is a good explanation of the problem here.

There are two lessons to be learned from this: 1) be careful of your looping conditions. 2) Try and write your code in small snippets that are testable, and write tests! A simple iteration through the total amount of days including a leap year would have caught this bug.

Just FYI, I’m not saying I would have been good enough to catch this, but it is worth writing down so I try to remember for myself.

The Address of Monkey

Have you seen the following C code sample:

	char x = 1;
	char c = x["monkey"];

Do you know what it the value of c is? Don’t read on unless you want to know the answer and why. The value of c is ‘o’. Why? Well, I wrote some code to start playing around with this. The answer seemed simple, but here were some suggestions about why the answer is ‘o’:

  • 1 is the index into the string “monkey”
  • There is some magic with math of memory on the stack for the compiler used
  • Something else is happening

Okay, it seems relatively trivial now, but when I looked at it it wasn’t. Other people were putting up ideas so I tested them out. Here is my silly test code:

#include 

void initial_test()
{
	char x = 1;
	char c = x["monkey"];
	
	printf("What is c:%cn", c);
}

void different_index()
{
	char x = 2;
	char c = x["monkey"];
	
	printf("What is c:%cn", c);
}

void space_allocation()
{
	char x = 1;
	char v = 'd';
	char c = x["monkey"];
	
	printf("What is c:%cn", c);
}

int main(int argc, char** argv)
{
	initial_test();
	different_index();
	space_allocation();
	
	return 0;
}

/* Output:
What is c:o
What is c:n
What is c:o
*/

What is actually going on here is really just the associative property of addition. I was telling a friend that I would understand “monkey”[x], but not the other way around. This is the quote from my friend (who wishes to remain nameless):

I mean, technically it’s base_address + sizeof(datatype)*index. since sizeof(char) == 1, it’s just base_address+index. 1+addressof(“monkey”) or addressof(“monkey”) + 1.. they both work

In the end it was just a fun little exercise.

MFC’s Radio Button Hack

Disclaimer: I am a mac user, but a windows programmer.

MFC is Microsoft’s old Window framework. Basically it is an object oriented wrapper around the traditional Win32 programming environment presented by Microsoft to help develop windows. Win32 is many years old, and so is MFC. Microsoft’s new frameworks, .NET and WPF (Windows Presentation Framework) are supposedly better than MFC, but I have yet to play with them.

MFC has tools for many different types of controls, from buttons to dialogs, windows, and menus. MFC allows the user to create the button, override some basic functionality, provide message callbacks and otherwise manipulate the application. Buttons are particularly interesting because the base class for buttons actually provides a ton of functionality for many different types of buttons. From this one class, you can get push buttons, check boxes, radio buttons, owner draw buttons (the programmer handles the rendering of these buttons), etc.

A Group of Radio Buttons
A Group of Radio Buttons

I have several problems with this class design, but today I just want to talk about my gripe with Radio Buttons. The term radio button comes from the buttons on old car radios, where only one button could be pushed at any one time. Radio buttons on a computer form, are by definition grouped with other radio buttons so that only one in the group can be selected at any one time. Any time a user selects another button in the group, the previously selected button should become unselected.

Taking this even further, logically, you should only use a radio button in certain situations. You have several options, usually less than 10, and you want the user to select from one of them. You should be able to have a default option set, and this choice should somewhat make sense. This functionality is very similar to a drop down box. In a drop down, you have a bunch of options (please put them in some order), where the user should select only one item. The difference in use between radio and drop downs depends on your application, but in general, you can put more items into a drop down. Drop downs will take up less screen real estate, but not all the choices may be obvious to the user, and sometimes the user may select the first option that seems relevant rather than looking through the whole list. In a radio group, all the options are present at the start.

This brings me to my gripe. MFC radio buttons are just the same as any other CButton. The way you define that a radio button is a radio button is by passing a style flag that is either BS_RADIOBUTTON or BS_AUTORADIOBUTTON. The difference is that auto radio buttons will look to be part of a group. This group is defined by ORing the BS_AUTORADIOBUTTON style with WS_GROUP for the first element of a group. All subsequent radio buttons will be part of that group until you create another WS_GROUP.

This upsets me because radio buttons in a group are associated with the other buttons in that group. They shouldn’t just be loosely coupled like this. It puts a lot more responsibility on the programmer to understand how the grouping is done. If you look at the picture above, you will notice it is from my Mac. In Interface Builder, Apple does not provide you with individual radio buttons, it instead provides an object called a “Radio Group”. This group is a collection of radio buttons that handles all the magic I wish existed in MFC. To be fair, Apple’s implementation is pretty new, they have redefined the way to create code on the Mac no less than 8 years ago with release of OS X. Microsoft’s MFC is much older than that and they have new technologies out there which probably better handle this problem. My issue is simply that I am working with legacy code here, and am incredibly frustrated by the lack of UI thought that went into designing this library in the beginning.

Wanting To Find A Bug

This happened to me last week. Yet another lesson learned. I was working on debugging some code at work. Someone was complaining about some functionality in an element we draw on screen. In order to better diagnose the problem, I wanted to create an example and see if I couldn’t get the problem to reproduce. Not a bad first step.

The problem was that as I wrote out the code I made a typo on the code I was writing. I wrote something like the following snippet:

     // ... 
     int x = pixels2OtherUnitsX(20);
     int y = pixels2OtherUnitsX(23);

     // ...

If you look closely you will see that I’m using pixels2OtherUnitsX() in both cases, where I should probably be using pixesl2OtherUnitsY() in the second case. Oops.

What do you know, my image didn’t render correctly on screen. I had recreated the bug that someone said was out there. Now all I had to do was figure out where in our production code the bug was. I spent way too long looking around for the problem.

Lesson don’t be so blinded by your desire to find a bug you miss one that is right in front of your face.

Why 2 can sometimes equal 1

Ran into a fun situation today where I was writing some code, and I came across an interesting situation in C++. Now, before I get to the end of this post, I’ll give you the punch-line, Developer stupidity.

So I was working on a exercise where I needed to write some sort of state machine.

enum STATES{
  STATE_1 = 0,
  STATE_2,
  STATE_3,
  ...
};

/* ... some other code ... */

switch(state) {
case STATE_1:
    // Do something
    state = STATE_2;
    break;
case STATE_2:
    // Do some other stuff
    state = STATE_3;
    break;
case STATE_3:
    // Do the last state of stuff
    // This code never gets called.
    break;
}

In this code STATE_3 is never reached. The code for the enum was working fine, but the state wasn’t being reached. I went over this for a while, until I found out the problem.

bool state = STATE_1;

The assignment for the state variable had been left over from a previous implementation of the code, and when you set the Boolean value for state = 2;, you get 1, which is true.

Now I don’t expect anyone to be as silly as me and make this mistake, but just in case, learn from my lesson.

TOC on all Books Please

I shop for mostly non-fiction books. Most of these books tend to be on new and emerging technologies. When I’m purchasing a book on a technology, I really want to know the content of the book before I purchase it. What I really want is a table of contents. Just a list of the chapter lists and maybe even a more detailed list of the topics covered in each chapter, but at least some form of the table of contents would be very helpful in my purchasing decision.

The sad thing is, not many of these books actually publish a table of contents online. At the time when I was first searching for books, one of the notable series that didn’t publish this was Head First Labs. They have since changed their ways and publish a PDF version of each books TOC on the individual book page. Another good example is the new Hillegass book. While this is a great book, and I was going to buy it, with, or without a look at the TOC it would still be great to see the TOC, so I knew what I was purchasing.

There is another one that really drove me up the edge. I purchased the first edition of Bullet Proof Web Design, and thought it was a GREAT book. While Dan’s site now has an updated TOC, it didn’t when the book first came out, and the publishers book page does not list the table of contents, even though they will give you chapter 1 for review. The other problem with this book, is that even at the time of this post, there is no real mention about what has been updated in the second edition of this book. Now, I definitely see the cause for someone new web design to buy this book, but why should “I”, a reader of the previous edition shell out another $25 for this one?

This is just another instance of not giving the customers what they need. How hard is it to publish your TOC? It is a simple list element on your webpage. You aren’t giving out anything that special, just giving your customers an idea of what they will be purchasing. That is rule number 1 in sales: give the customers what they want.

Cheer Resource Intro

Cheer Resource is the name of my current web project. The whole goal of this project is to contribute to the cheerleading community by providing a centralized resource for cheerleaders, parents, and coaches alike to share knowledge about their programs and experiences. There is much work to be done on this project, including refining our goals and design, but most of that will be posted on the project page:

https://zacharyc.com/projects/cheer-resource/

Keep in mind that this is still a rough sketch, and this not what I do for full time employment. This is a “project”, or something I work on for fun. More details to come in the future. For now, I’m just excited to have set up a basic code base and database structure.