Recently I was browsing through YouTube, and it kept suggesting this video about programming mistakes, with one of those paper-cutout like CG thumbnails. My first impression was that it was one of those "let me show you how to code" videos, like so many of YouTube's suggestions to me are. The fact of the matter is, while YouTube tried to get me to watch beginning coder videos, like normal, they cycled through my feed pretty quickly, this one stuck around for a few days.
I don't pretend to know the algorithm YouTube uses to display this stuff, but somehow it figured I would like that video. Intrigued, I clicked it and braced myself for what would surely be someone explaining why using single letters as variable names is bad.
That's not what it was.
One of the odd things I like to watch there is lectures. It doesn't have to be computer science related, but I like watching people talk about something they're passionate about. Every time I learn something new about the subject which I hadn't previously even thought about.
This video, despite it's code-school thumbnail, was one of those videos.
Up until I watched it, I was a hard line fan of Stroustrup indentation. Functions like this one below were nice to me, I was used to them, and they were rather standard.
int do_a_thing(int this, string that, bool the_other_thing){
if(the_other_thing){
cout<<this<<endl;
}
else{
cout<<that<<endl;
}
for(int i = 0; i<this; i++){
do_something_with_i(i, this, that);
}
return 0;
}
I'm not going to get too deep into the topics in the video, I'll link to it at the end, but my takeaway was basically a whole bunch of stuff I had figured out before, but not really internalized. Here's the shortlist:
- Comments get out of date fast, often conveying wrong information (In the words of the presenter,
comments are lies waiting to happen
) - It's OK to put a line break in an argument list
- Variable names should be human-readable, not Java-human-readable or abbreviated to the point of meaninglessness
- This means no networkedAntiTheftVisualRecorder, just use securityCamera
- Something out of my code to this effect: menubhoverout is bad, menuButtonMouseLeave is better
- Layout matters, and consitent predictable layout makes for readable, debuggable, sustainable code
These aren't exactly the points that were made, but again, not trying to repeat what was in the video in its entirety here.
This prompted me to look at my largest, and arguably most worked on free time project, the FSU CS Club site. I've discussed a little about the site in my first post, and here I'm going to show the change in code style since I watched the video.
function movewindow(currentwindow, increasex, increasey){
//client window bounaries: get the current dimensions of a window
var cwbounds = currentwindow.toplevel.getBoundingClientRect();
//screen boundaries: get the current dimensions of the screen
var scbounds = document.body.getBoundingClientRect();
//new X position (from top left corner)
var newx = cwbounds.left + increasex;
//new Y position (from top left corner)
var newy = cwbounds.top + increasey;
//now we make sure we're not running off the screen in the horizontal direction
if(newx>0 && cwbounds.right+increasex < scbounds.right){
currentwindow.toplevel.style.left = newx + "px";
}
//and try to make sure we don't run off the screen in the vertical direction
//though the code for the bottom doesn't work right, not sure why.
if(newy>0 && cwbounds.bottom + increasey < scbounds.bottom){
currentwindow.toplevel.style.top = newy + "px";
}
}
Note that most of these comments are obvious, and a number of them would be with better variable names. Also note that the code is largely horizontal, and can make for some long lines.
Now the new code:
function movewindow(
currentwindow,
increasex,
increasey)
{
var currentWindowBounds = currentwindow.toplevel.getBoundingClientRect();
var screenBounds = document.body.getBoundingClientRect();
var newx =
currentWindowBounds.left +
increasex;
var newy =
currentWindowBounds.top +
increasey;
if(
newx>0 &&
currentWindowBounds.right + increasex < screenBounds.right)
{
currentwindow.toplevel.style.left =
newx +
"px";
}
if(
newy>0 &&
currentWindowBounds.bottom + increasey < screenBounds.bottom)
{
currentwindow.toplevel.style.top =
newy +
"px";
}
}
When trying to debug the second version of this otherwise identical code, it's much easier to find specific variables, troubleshoot problems, and even understand to an extent. Also note that as parts of this program expand, the general format is not broken, and horizontal scrolling is not an issue. Switching to this style virtually eliminated >80 character lines, making it readable alongside not one more document, but two more on my 1080p monitor. With the old style, some functions would become unreadable if they didn't have at least 2/3 of the screen to display their enormous lines on.
And this comes to the final part of this post: why any of this matters.
I've seen a lot of lectures where someone will recommend something only for it to turn out to not be very useful. There are plenty of people who use some K&R derived indentation scheme like I was, and to good effect, it's a solid style. This time, however, the change in something as trivial as style has made the code arguably neater and easier to not only read, but expand upon down the line without breaking the readability of the code.
I've been using GNU Emacs for my coding of late, and I do want to mention that it makes all of this really easy. A lot of my code had bad indentation, or hadn't been changed when a new level of code was inserted above another existing level, and it made for some trouble. When formatting all of this, if a mistake crops up, Emacs is intelligent enough that simply selecting the region where things went wrong and typing C-M-\ will solve indentation problems, and even go so far as to place the braces on the correct indentation level to separate code from parameter lists properly. Emacs will also let you align arguments to a function right at the end of the function name, where the list would run horizontally otherwise in Stroustrup style.
Here's the original code sample in the new format:
int do_a_thing(
int this,
string that,
bool the_other_thing)
{
if(the_other_thing)
{
cout<<this<<endl;
}
else
{
cout<<that<<endl;
}
for(
int countsUpToThis = 0;
countsUpToThis<this;
countsUpToThis++)
{
do_something_with_i(
countsUpToThis,
this,
that);
}
return 0;
}
Much neater, right?
Original lecture video (Edit: Appears to be broken?)