I like to make comments complete sentences.
Code teases itself right out of the text's stream when comments are indented.
# Lorem ipsum dolor sit amet, consectetur adipisicing elit, # sed do eiusmod tempor incididunt ut labore. my $lorem = @ipsum; # Yes, this is a comment. $nesquiat[0] = 42;# Lorem ipsum dolor sit amet, consectetur adipisicing elit, # sed do eiusmod tempor incididunt ut labore. my $lorem = @ipsum; # Yes, this is a comment. $nesquiat[0] = 42;
sub new { # Determine if we were called via an object-ref or a classname. my $this = shift; my $class = ref($this) || $this; # Any remaining arguments are treated as initial values for the # hash that is used to represent this object. my $ctpath = clearcase_config('CLEARTOOL'); # The versions also vary in their formatting of the following. my @defaults = ( qw( -output -chomping ), '-abort' => undef, '-execpath' => $ctpath ); my $execopts = execopts_parse(@defaults, @_); my $self = { '-execopts' => $execopts }; # Bless ourselves into the desired class and perform any # initialization. bless $self, $class; $self->initialize(); return $self; }sub new { # Determine if we were called via an object-ref or a # classname. my $this = shift; my $class = ref($this) || $this; # Any remaining arguments are treated as initial values for # the hash that is used to represent this object. my $ctpath = clearcase_config('CLEARTOOL'); # The versions also vary in their formatting of the following. my @defaults = ( qw( -output -chomping ), '-abort' => undef, '-execpath' => $ctpath, ); my $execopts = execopts_parse(@defaults, @_); my $self = { '-execopts' => $execopts }; # Bless ourselves into the desired class and perform any # initialization. bless $self, $class; $self->initialize(); return $self; }
# Don't request or allow negotiation of any options for LCP and IPCP # (use default values). #-all # Disable Address/Control compression negotiation (use default, i.e. # address/control field disabled). -ac # Disable asyncmap negotiation (use the default asyncmap, i.e. escape # all control characters). #-am # Don't fork to become a background process (otherwise pppd will do so # if a serial device is specified). -detach # Disable IP address negotiation (with this option, the remote IP # address must be specified with an option on the command line or in # an options file). #-ip# Don't request or allow negotiation of any options for LCP # and IPCP (use default values). #-all # Disable Address/Control compression negotiation (use # default, i.e. address/control field disabled). -ac # Disable asyncmap negotiation (use the default asyncmap, i.e. # escape all control characters). #-am # Don't fork to become a background process (otherwise pppd # will do so if a serial device is specified). -detach # Disable IP address negotiation (with this option, the remote # IP address must be specified with an option on the command # line or in an options file). #-ip
The '#' towers appear wasteful of vertical spacing. Simple indenting of the comments gets the job done.
# # startDaemon # # Fork and detach from the parent process # sub startDaemon { # # Fork and detach from the parent process # Proc::Daemon::Init(); if ($@) { dienice("Unable to start daemon: $@"); } # # Get a PID file # dienice("Already running!") if hold_pid_file($PIDFILE); }sub startDaemon { # Fork and detach from the parent process. Proc::Daemon::Init(); if ($@) { dienice("Unable to start daemon: $@"); } # Get a PID file. dienice("Already running!") if hold_pid_file($PIDFILE); }
I prefer this unusual formatting:
if (MaxMemory == 0) {
/* TODO move to one module per platform. */
#ifdef DOS
#if defined(__TURBOC__)
MaxMemory = (unsigned long) coreleft();
#else
/* DOS-default is 256 KB. */
MaxMemory = (unsigned long) 256 * 1024;
#endif
#else
#if defined(WIN16)
MaxMemory = (unsigned long) 1024 * 1024;
#else
#if defined(WIN32)
/* Get physical memory amount. */
MEMORYSTATUS Mem;
Mem.dwLength = sizeof(MEMORYSTATUS);
GlobalMemoryStatus(&Mem);
MaxMemory = Mem.dwAvailPhys;
#ifdef WIN98
/* WIN98 cannot handle more than 768MB. */
if (MaxMemory > (unsigned long) 700 * 1024 * 1024) {
MaxMemory = (unsigned long) 700 * 1024 * 1024;
}
#endif
#else
/* UNIX-default is 2 MB. */
MaxMemory = (unsigned long) 2048 * 1024;
#endif
#endif
#endif
}
Usually presented like this:
if (MaxMemory == 0) {
/* TODO move to one module per platform. */
#ifdef DOS
#if defined(__TURBOC__)
MaxMemory = (unsigned long) coreleft();
#else
/* DOS-default is 256 KB. */
MaxMemory = (unsigned long) 256 * 1024;
#endif
#else
#if defined(WIN16)
MaxMemory = (unsigned long) 1024 * 1024;
#else
#if defined(WIN32)
/* Get physical memory amount. */
MEMORYSTATUS Mem;
Mem.dwLength = sizeof(MEMORYSTATUS);
GlobalMemoryStatus(&Mem);
MaxMemory = Mem.dwAvailPhys;
#ifdef WIN98
/* WIN98 cannot handle more than 768MB. */
if (MaxMemory > (unsigned long) 700 * 1024 * 1024) {
MaxMemory = (unsigned long) 700 * 1024 * 1024;
}
#endif
#else
/* UNIX-default is 2 MB. */
MaxMemory = (unsigned long) 2048 * 1024;
#endif
#endif
#endif
}
Some (all? most?) C preprocessors have an 'elif'. With it, it would look like this (did I break the logic?); also, with interleaved indentation (doesn't work for me at all):
if (MaxMemory == 0) {
/* TODO move to one module per platform. */
#ifdef DOS
#if defined(__TURBOC__)
MaxMemory = (unsigned long) coreleft();
#else
/* DOS-default is 256 KB. */
MaxMemory = (unsigned long) 256 * 1024;
#endif
#elif defined(WIN16)
MaxMemory = (unsigned long) 1024 * 1024;
#elif defined(WIN32)
/* Get physical memory amount. */
MEMORYSTATUS Mem;
Mem.dwLength = sizeof(MEMORYSTATUS);
GlobalMemoryStatus(&Mem);
MaxMemory = Mem.dwAvailPhys;
#ifdef WIN98
/* WIN98 cannot handle more than 768MB. */
if (MaxMemory > (unsigned long) 700 * 1024 * 1024) {
MaxMemory = (unsigned long) 700 * 1024 * 1024;
}
#endif
#else
/* UNIX-default is 2 MB. */
MaxMemory = (unsigned long) 2048 * 1024;
#endif
}
pudge showed me a sample of his code, that uses the '?' and ':' at the beginning of lines:
my $mode =
(ref $hash->{CALLBACK} eq 'CODE'
? kAEQueueReply
: (exists $hash->{REPLY} # check event setting
? $hash->{REPLY}
: exists $self->{REPLY} # check global setting
? $self->{REPLY}
: 1 # default is to wait
)
? kAEWaitReply
: kAENoReply)
| (exists $hash->{MODE}
? $hash->{MODE}
: exists $self->{MODE}
? $self->{MODE}
: (kAECanInteract | kAECanSwitchLayer));
I have trouble seeing the top level structure of the assignment, and I used to format that like this:
my $mode = (
ref $hash->{CALLBACK} eq 'CODE' ?
kAEQueueReply : (
# Check event setting.
exists $hash->{REPLY} ?
$hash->{REPLY} :
# Check global setting.
exists $self->{REPLY} ?
$self->{REPLY} :
# Default is to wait.
1
) ?
kAEWaitReply :
kAENoReply
) | (
exists $hash->{MODE} ?
$hash->{MODE} :
exists $self->{MODE} ?
$self->{MODE} :
(kAECanInteract | kAECanSwitchLayer)
);
But pudge's example makes the ternary conditional much easier to read, so nowadays, I use a combination of those styles:
my $mode = (
ref $hash->{CALLBACK} eq 'CODE'
? kAEQueueReply
: (
# Check event setting.
exists $hash->{REPLY}
? $hash->{REPLY}
# Check global setting.
: exists $self->{REPLY}
? $self->{REPLY}
# Default is to wait.
: 1
)
? kAEWaitReply
: kAENoReply
) | (
exists $hash->{MODE}
? $hash->{MODE}
: exists $self->{MODE}
? $self->{MODE}
: (kAECanInteract | kAECanSwitchLayer)
);
I find clarity is improved with "one true style" blocks, indented comments, and first-level-only indented, prefixed-with-operator, ternary conditional clauses. (Note that I've also moved up to a 4-space shift-width.)
clintp once showed us this code, which he had to maintain:
if (((BIG_LE_ZERO(&impamt) || !draftdate ||
(((imptype == TF_MANUAL || imptype == TF_CLI_WIRE ||
imptype == TF_WIRE) ? (cleardate = draftdate) :
(cleardate = add_and_check_weh(draftdate, 3, YES, YES))) >
(cur->duedate <= TFP_TOMORROW ? TFP_TODAY :
((!memcmp(cur->taxkey, "FICA", 4) || !memcmp(cur->taxkey, "FUI ", 4)) ?
add_and_check_weh(cur->duedate, -1, NO, YES) : cur->duedate)))) &&
(BIG_CMP_BL(big_sub_bb(balamt,
(BIG_LT_ZERO(&impamt) ? BIGNUM_ALWAYS_ZERO : impamt)),
-500l, DECIMAL2) < 0)) || (!clibase || (BIG_CMP_BL(balamt, -500l, DECIMAL2) < 0)))
{ ...
Reformatted, I can almost understand it, but it's so big that I'd probably try to factor out some of the interior predicates as maintenance proceeded.
if (
(
(
BIG_LE_ZERO(&impamt) ||
! draftdate || (
(
(
imptype == TF_MANUAL ||
imptype == TF_CLI_WIRE ||
imptype == TF_WIRE
)
? (cleardate = draftdate)
: (cleardate = add_and_check_weh(draftdate, 3, YES, YES))
) > (
cur->duedate <= TFP_TOMORROW
? TFP_TODAY
: (
(
! memcmp(cur->taxkey, "FICA", 4) ||
! memcmp(cur->taxkey, "FUI ", 4)
)
? add_and_check_weh(cur->duedate, -1, NO, YES)
: cur->duedate
)
)
)
) && (
BIG_CMP_BL(
big_sub_bb(
balamt,
(BIG_LT_ZERO(&impamt) ? BIGNUM_ALWAYS_ZERO : impamt)
),
-500l,
DECIMAL2
) < 0
)
) || (
! clibase || (BIG_CMP_BL(balamt, -500l, DECIMAL2) < 0)
)
) { ...
def self.search(query) Person.all('$where' => "function() { return this.diaspora_handle.match(/^#{query}/i) || this.profile.first_name.match(/^#{query}/i) || this.profile.last_name.match(/^#{query}/i); }") enddef self.search(query) Person.all( '$where' => "function() { return this.diaspora_handle.match(/^#{query}/i) || this.profile.first_name.match(/^#{query}/i) || this.profile.last_name.match(/^#{query}/i); }" ) end
#Person.rb class Person ... key :url, String key :diaspora_handle, String, :unique => true key :serialized_key, String #This is your public/private encryption key pair. OOPS. key :owner_id, ObjectId #You don't want me changing this one either... one :profile, :class_name => 'Profile' many :albums, :class_name => 'Album', :foreign_key => :person_id belongs_to :owner, :class_name => 'User' #... because it lets me own you! Literally! end #User.rb one :person, :class_name => 'Person', :foreign_key => :owner_id #Oh dear.# Person.rb class Person ... key :url, String key :diaspora_handle, String, :unique => true # This is your public/private encryption key pair. OOPS. key :serialized_key, String # You don't want me changing this one either... key :owner_id, ObjectId one :profile, :class_name => 'Profile' many :albums, :class_name => 'Album', :foreign_key => :person_id # ...because it lets me own you! Literally! belongs_to :owner, :class_name => 'User' end # User.rb # Oh dear. one :person, :class_name => 'Person', :foreign_key => :owner_id
$clickers_html .= $self->top_bot_clickers_html( $left + $half_width + ( 2 * ( $self->wac_prop('box_border_width') + $self->wac_prop('box_padding_horiz'))), $half_width, 'c55lugnut', $box);$clickers_html .= $self->top_bot_clickers_html( $left + $half_width + ( 2 * ( $self->wac_prop('box_border_width') + $self->wac_prop('box_padding_horiz') ) ), $half_width, 'c55lugnut', $box, );
my $height = ($box->{height} - (2 * ( $self->wac_prop('box_border_width') + $self->wac_prop('box_padding_verti')))) / 2;my $height = ( $box->{height} - ( 2 * ( $self->wac_prop('box_border_width') + $self->wac_prop('box_padding_verti') ) ) ) / 2;
#users_controller.rb
def update
# Uh oh, no auth check.
@user = User.find_by_id params[:id]
prep_image_url(params[:user])
# Pass untrusted input to @user.
@user.update_profile params[:user]
respond_with(@user, :location => root_url)
end
#user.rb
def update_profile(params)
# Insert input directly to DB.
if self.person.update_attributes(params)
...
end
end
window.location.replace(
((lang == 'en') ? 'cows' : 'vaches') + '.html' +
"?" +
"pen1=" + this.pens.pen1.box.id + "&" +
"pen2=" + this.pens.pen2.box.id + "&" +
"last_pen=" + this.last_pen_moved_ndx + "&" +
"last_box=" + this.last_move_box_id + "&" +
"is60on=" + this.rule60on +
""
);
var border_color = (this.id == 'b60' && appl.wac.rule60on)
? "[% p.c_border60_on %]"
: (NPH == 1)
? (this.pens_held[1]
? "[% p.c_border_pens10 %]"
: "[% p.c_border_pens02 %]"
)
: (NPH == 2)
? "[% p.c_border_pens12 %]"
: "[% p.c_border_pens00 %]"
;