Skip to content

Instantly share code, notes, and snippets.

@nothings
Last active August 29, 2015 14:04
Show Gist options
  • Save nothings/28d163780c35a8608271 to your computer and use it in GitHub Desktop.
Save nothings/28d163780c35a8608271 to your computer and use it in GitHub Desktop.
One of the primary benefits of using a fixed-width
programming font is that things line up in columns
if they are the same length in characters. (Please
note that monospacing is not just for indentation;
indentation works fine with proportional fonts. It
is only once you have a non-blank character on the
line that proportional vs. monospace differ much.)
In my experience, lining code up so features align
vertically makes it a lot easier to avoid the bugs
described here: http://www.viva64.com/en/b/0260/ .
I've lined up all the attributed examples shown on
that page. There are two considerations. First, is
the bug immediately obvious? Second, does scanning
vertically down the "changing" columns turn up the
bug with little mental effort (i.e. mechanically)?
Of the 15 attributed examples, my reading is that:
3 are not helped by column-scanning;
1 becomes very obvious once aligned
6 are obvious when column-scanned (but 4 were already aligned)
5 are obvious when column-scanned, but require multiple-statements per line or very long lines
Given that there were 4 examples that were already
aligned where the bug was easily found by scanning
columns, it's clear that aligning vertically isn't
a panacea: programmers must still scan the columns
to find the bug. So I firmly recommend they do so!
Note that this reasoning is one of the two primary
reasons that you can find multiple statements on a
single line in the stb_ libraries. (De-emphasizing
error-control-flow is the other important reason.)
Here is the sample code reformatted for alignment:
== Examples that are obvious once aligned.
// MongoDb
return _id == r._id
&& votes == r.votes
&& h == r.h
&& priority == r.priority
&& arbiterOnly == r.arbiterOnly
&& slaveDelay == r.slaveDelay
&& hidden == r.hidden
&& buildIndexes == buildIndexes;
== Obvious on column-scanning
// SlimDX
//| |
sliders[i] = joystate.rglSlider [i]; // if aligning is a priority, rename this to be same length
asliders[i] = joystate.rglASlider[i];
vsliders[i] = joystate.rglVSlider[i];
fsliders[i] = joystate.rglVSlider[i];
// Qt
// | |
qreal x = ctx->callData->args[0].toNumber();
qreal y = ctx->callData->args[1].toNumber();
qreal w = ctx->callData->args[2].toNumber();
qreal h = ctx->callData->args[3].toNumber();
if (!qIsFinite(x) ||
!qIsFinite(y) ||
!qIsFinite(w) ||
!qIsFinite(w) )
// |
== Obvious, but only if you break the one-statement-per-line rule
// Chromium
if (access & FILE_WRITE_ATTRIBUTES) output.append(ASCIIToUTF16("\tFILE_WRITE_ATTRIBUTES\n"));
if (access & FILE_WRITE_DATA) output.append(ASCIIToUTF16("\tFILE_WRITE_DATA\n" ));
if (access & FILE_WRITE_EA) output.append(ASCIIToUTF16("\tFILE_WRITE_EA\n" ));
if (access & FILE_WRITE_EA) output.append(ASCIIToUTF16("\tFILE_WRITE_EA\n" ));
break;
== Obvious on column-scanning, but only if you break the one-statement-per-line rule
// Qt
// | | |
if (repetition.isEmpty()) { pattern->patternRepeatX = true ; pattern->patternRepeatY = true ; }
else if (repetition == QStringLiteral("repeat") { pattern->patternRepeatX = true ; pattern->patternRepeatY = true ; }
else if (repetition == QStringLiteral("repeat-x")) { pattern->patternRepeatX = true ; }
else if (repetition == QStringLiteral("repeat-y")) { pattern->patternRepeatY = true ; }
else if (repetition == QStringLiteral("no-repeat")) { pattern->patternRepeatY = false; pattern->patternRepeatY = false; }
else {
//TODO: exception: SYNTAX_ERR
}
// OpenSSL
// | | |
if (!strncmp(vstart, "ASCII" , 5)) arg->format = ASN1_GEN_FORMAT_ASCII;
else if (!strncmp(vstart, "UTF8" , 4)) arg->format = ASN1_GEN_FORMAT_UTF8;
else if (!strncmp(vstart, "HEX" , 3)) arg->format = ASN1_GEN_FORMAT_HEX;
else if (!strncmp(vstart, "BITLIST", 3)) arg->format = ASN1_GEN_FORMAT_BITLIST;
// Requires some additional work beyond just naive column-analysis;
// nevertheless, columnizing encourages you to work down sequentially
// through every number and check it, in an organized way which is
// harder when it's multiline, so I'm going to count it.
== Obvious on column-scanning, but only if you allow long lines
// Source Engine SDK
// | | |
intens.x=OrSIMD(AndSIMD(BackgroundColor.x,no_hit_mask),AndNotSIMD(no_hit_mask,intens.x));
intens.y=OrSIMD(AndSIMD(BackgroundColor.y,no_hit_mask),AndNotSIMD(no_hit_mask,intens.y));
intens.z=OrSIMD(AndSIMD(BackgroundColor.y,no_hit_mask),AndNotSIMD(no_hit_mask,intens.z));
// Clang
// | | | |
return ContainerBegLine <= ContaineeBegLine
&& ContainerEndLine >= ContaineeEndLine // | | | |
&& (ContainerBegLine != ContaineeBegLine || SM.getExpansionColumnNumber(ContainerRBeg) <= SM.getExpansionColumnNumber(ContaineeRBeg))
&& (ContainerEndLine != ContaineeEndLine || SM.getExpansionColumnNumber(ContainerREnd) >= SM.getExpansionColumnNumber(ContainerREnd));
// this requires such long lines that I would probably break these evals into
// temporary variables that I could line up while having shorter lines.
// Also: don't use both Container and Containee. It's like using 'l' as a var.
== Examples that were already aligned in columns,
== column-scanning would catch:
// Source Engine SDK
// inline void Init
// SeqAn
// inline typename Value<Pipe>::Type const & operator*
// Quake-III-Arena
// if (fabs(dir[0])
// Unreal Engine 4
// static bool PositionIsInside(
== Examples that were already aligned in columns,
== but column-scanning wouldn't catch:
// ReactOS
// *ScanString
// (noisy, unalignable lexemes -- a reason to make #defines for them?)
// Trans-Proteomic Pipeline
// void setPepMaxProb
// (intervening code between lines makes it impossible to make adjacent)
// Mozilla Firefox
// if (protocol
// (column scanning only catches single-char patterns and adjacent-line repeats;
// note that ordering the comparisons alphabetically would help with this)
== Omitted from analysis because it's
== not clear what the bug actually is.
// Multi Theft Auto
// class CWaterPolySAInterface
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment