Spartan Programming

@michael:
Of course in Python the for loop IS a foreach loop and nobody cares about performance since it’s already interpreted.

Please get your facts right. First point : Python is byte-compiled (at least CPython, StacklessPython, Jython and IronPython are). Second point: yes, there are people that actually care about Python’s performances.

@Daanish Rumani, I’ve always preferred to put ‘change’ comments in source control rather than litter my code with them, otherwise the code just gets harder to read. That way a diff between versions, along with the comment helps to identify and cross reference any changes.

Agree with you about the braces though, I like to see them even for single line blocks, much easier to read.

I think spartan is important, I code not organized and I face problems when I want to change my code :frowning:

Baited by a troll? Perhaps.

Sorry @Yossi, I disagree 100%, but then I use languages with objects as first class citizens, and most of your one-liners can be removed by using a class and better use of object oriented techniques.

/// xmldoc comments deleted.
public SendMessageResult SendMessage( HtmlMailMessage htmlMailMessage)
{
// check preconditions
if( htmlMailMessage == null ) throw new ArgumentNullException(htmlMailMessage);
if( !htmlMailMessage.HasRecipients() ) throw new Exception(no recipients in message);
if( !htmlMailMessage.HasSender() ) throw new Exception(no sender specified);

// create SmtpMessage from our own HtmlMailMessage
SmtpMessage message = new SmtpMessage();

message.Type = MessageType.Html;
message.Body = htmlMailMessage.HtmlBody;
message.Sender = CreateSmtpSenderFor(htmlMailMessage.Sender);
SetMessageRecipientsTo(message,htmlMailMessage.Recipients);

try 
{
	SmtpClient client = new SmtpClient(mail.mycompany.com);
	SentMessageIdentifier messageId = client.Send(message);
	return new SuccessSendMessageResult(messageId);
}	
catch( SmtpClientException ex )
{
	// this method does not propagate smtp client exceptions.
	return new ExceptionSendMessageResult(ex);
}

}

None of this rubbish with s, m, t, etc. It’s readable even if you don’t know C#, and even if you don’t know what the SendMessageResult, ExceptionSendMessageResult, SuccessSendMessageResult or HtmlMailMessage objects look like, and you don’t have to look at the methods CreateSmtpSenderFor and SetMessageRecipientsTo to figure out what they’re supposed to do.

Maybe I’ve evolved, but I have to keep scanning up to see what s, m, and t are. My brain does a great job of pattern recognition, and there’s a cognitive penalty to single character variable names when they’re not universally accepted e.g. ‘ex’ is accepted for exception (in C#), but s and m are not session (or was that sender) and message universally. It’s easier to read if the variable is session, not s.

The only thing I see is an attempt to round up a few half-baked refactoring techniques in a few pages of text and label it ‘spartan’. You can only get out what you’ve put in. Read Fowlers Refactoring book, which I might add will take a few days, as it’s more than a few hundred pages.

Go to http://www.refactoring.com/

@TED I’d reckon that child.Surname = father.Surname is a little clearer.

For property-disabled languages, child.set_Surname( father.get_Surname() ); (or whatever your language’s convention is)

child.surname() by itself implies that surname is an action, and is not just setting a property. I surname() you? Huh? Property get and set’s are best done with a convention that implies they are just properties, and their only side-effect is a property set (or get).

Maybe to a western, english reader’s perspective it’s obvious, but even we can’t agree what to call it.

Oh, I didn’t say that you claimed it as your own. However, you are reaping a significant benefit out of its use, which in the form you apply it is nearly a word for word copy.

Basically, the concept is like this. Suppose someone quoted 90% of a novel, interjecting comments every few pages and copiously mentioning the title and author of the novel. It would still be plagiarism and unfair use because the original content dramatically outweighs the new contribution.

The only place I did misspeak is that it’s possible the wiki page has licensing that allows this type of use. Which means it is not exactly wrong. But still, the central point is that you are here deriving benefit from content which is primarily someone else’s work. And I don’t like it.

Your article here is barely better than a link to the page along with a picture and your disclaimer in the final paragraph. You’re free to do what you please with this, but since you provide a forum for comments, I felt I should voice my objection.

Bothered to look up the definition of plagiarism: I used it incorrectly. For that I am sorry. However! I still believe the use you put this wiki article to is unfair because, whether you claim academic credit for it or not, you are still deriving financial benefit from the work of another. Mere citation does not absolve you of this; original content would.

I figured modern compilers are usually smarter than the programmer writing them. So, in all likelihood, it’s better to write clear and concise code and let the compiler sort out the optimization. But, in the interest of science, the compiled bytecode is different (for now). Although the JIT’er is free to do it’s own optimizations.

Typically I’d prefer code in Cone_A because it’s more easily debugged and easier to understand that it’s been coded correctly.

public class Cone_A
{
public double Radius { get; set; }
public double Height { get; set; }
public double ComputeVolume()
{
double result = Math.PI * Math.Pow(Radius, 3) * Height;
result /= 3.0;
return result;
}
}
public class Cone_B
{
public double Radius { get; set; }
public double Height { get; set; }
public double ComputeVolume()
{
return ( Math.PI * Math.Pow( Radius, 3) * Height) / (3.0);
}
}

The result, Cone_B::ComputeVolume() is much smaller in release mode. Execution time wasn’t tested.

I apologise for the long post, but it’s the only way to shut people up.

//In .net, DEBUG version of code. Cone_A::ComputeVolume
.method public hidebysig instance float64
ComputeVolume() cil managed
{
// Code size 57 (0x39)
.maxstack 3
.locals init ([0] float64 result,
[1] float64 CS$1$0000)
IL_0000: nop
IL_0001: ldc.r8 3.1415926535897931
IL_000a: ldarg.0
IL_000b: call instance float64 Inlining.Cone_A::get_Radius()
IL_0010: ldc.r8 3.
IL_0019: call float64 [mscorlib]System.Math::Pow(float64,
float64)
IL_001e: mul
IL_001f: ldarg.0
IL_0020: call instance float64 Inlining.Cone_A::get_Height()
IL_0025: mul
IL_0026: stloc.0
IL_0027: ldloc.0
IL_0028: ldc.r8 3.
IL_0031: div
IL_0032: stloc.0
IL_0033: ldloc.0
IL_0034: stloc.1
IL_0035: br.s IL_0037
IL_0037: ldloc.1
IL_0038: ret
} // end of method Cone_A::ComputeVolume

//DEBUG Cone_B::ComputeVolume
.method public hidebysig instance float64
ComputeVolume() cil managed
{
// Code size 53 (0x35)
.maxstack 3
.locals init ([0] float64 CS$1$0000)
IL_0000: nop
IL_0001: ldc.r8 3.1415926535897931
IL_000a: ldarg.0
IL_000b: call instance float64 Inlining.Cone_B::get_Radius()
IL_0010: ldc.r8 3.
IL_0019: call float64 [mscorlib]System.Math::Pow(float64,
float64)
IL_001e: mul
IL_001f: ldarg.0
IL_0020: call instance float64 Inlining.Cone_B::get_Height()
IL_0025: mul
IL_0026: ldc.r8 3.
IL_002f: div
IL_0030: stloc.0
IL_0031: br.s IL_0033
IL_0033: ldloc.0
IL_0034: ret
} // end of method Cone_B::ComputeVolume

// And again, in RELEASE mode. Cone_A::ComputeVolume
.method public hidebysig instance float64
ComputeVolume() cil managed
{
// Code size 52 (0x34)
.maxstack 3
.locals init ([0] float64 result)
IL_0000: ldc.r8 3.1415926535897931
IL_0009: ldarg.0
IL_000a: call instance float64 Inlining.Cone_A::get_Radius()
IL_000f: ldc.r8 3.
IL_0018: call float64 [mscorlib]System.Math::Pow(float64,
float64)
IL_001d: mul
IL_001e: ldarg.0
IL_001f: call instance float64 Inlining.Cone_A::get_Height()
IL_0024: mul
IL_0025: stloc.0
IL_0026: ldloc.0
IL_0027: ldc.r8 3.
IL_0030: div
IL_0031: stloc.0
IL_0032: ldloc.0
IL_0033: ret
} // end of method Cone_A::ComputeVolume

// RELEASE Cone_B::ComputeVolume
.method public hidebysig instance float64
ComputeVolume() cil managed
{
// Code size 48 (0x30)
.maxstack 8
IL_0000: ldc.r8 3.1415926535897931
IL_0009: ldarg.0
IL_000a: call instance float64 Inlining.Cone_B::get_Radius()
IL_000f: ldc.r8 3.
IL_0018: call float64 [mscorlib]System.Math::Pow(float64,
float64)
IL_001d: mul
IL_001e: ldarg.0
IL_001f: call instance float64 Inlining.Cone_B::get_Height()
IL_0024: mul
IL_0025: ldc.r8 3.
IL_002e: div
IL_002f: ret
} // end of method Cone_B::ComputeVolume

@Joe:

Your example is exactly why I don’t like early returns.
Four different return points - yikes! I definitely don’t find that easier to read.

Comparisons aren’t free, you loose some milliseconds on them.

Granted, but measure it. Really. Call it a ten thousand times and measure it.

Try comparing:

bool statusOk = func1();
if (statusOk)
____statusOk = func2();
if (statusOk)
____statusOk = func3();

if ( statusOk )
____dostuff();

return statusOk;

…to the original…

if (func1() == false)
____return false;
if (func2() == false)
____return false;
if (func3() == false)
____return false;

dostuff();
return true;

My version has 3 comparisons - the early return version has 3 comparisons.
The only advantage of the early return version is that it is quicker when hits an error, but the normal success path will take roughly the same time for both of them.

@T.E.D.: Thanks for that. I confess I’ve had one too many beers to go through it tonight, but I’ll prepare a suitable rebuff tomorrow.

For me, the example under ‘horizontal complexity’ is terrible; I always try to write such constructs as:

int abs(int x) {
return x 0 ? -x : x;
}

The shorter the code, the less the cognitive strain, the less chance of bugs, etc. I became a far more efficient coder once I started making full use of the ternary operator.

@Steven Bey

You mean only someone with !hasBrain, of course :wink:

Here’s some succinct code I wrote just now that caused a stackoverflow :slight_smile:

public double LargestYVal
{
get{ return LargestYVal;}
}

An unusual retrograde fitting scenario (short version legacy code pre the coding standard in our team) caused me to use a not ideal naming convention (the private member was already labelled LargestYValue :wink: ) Which meant intelisense ( a tool to aid lazy programming) allowed me to inadvertantly code a tight loop, which the compiler swallowed without complaint.

So in my attempt to avoid renaming all occurances of LargestYValue to largestYValue (to save time Ha!), I lost having to go through a second build and test !

The shorter the code, the less the cognitive strain, the less chance of bugs
Not really. Code that’s been deliberately compressed in terms of the lines or the speed requires more cognitive strain than slightly more verbose code that performs in an easily understood manner.

@bobby

I like your comment but it was a conscious decision to write it as hasBrain = false (for the guys who can’t read !hasBrain), although my point was muted, slightly, as it should really have been hasBrain == false.

I’ll give a second to this response:

Character count should be a concern, but not at the expense of readability. Ever.
–Kris on July 8, 2008 01:03 PM

I’m the only one to think that by replacing this

String curr = i.next();
InternetAddress[] temp = InternetAddress.parse(curr, false);
toAddresses.addAll(Arrays.asList(temp));

whit this:

toAddresses.addAll(Arrays.asList(InternetAddress.parse(r, false)));

you make the code 10x less readable?

And many of rules from Spartan programming remove readability of the code?

Yup, this pretty much looks exactly like my own personal coding philosiphy.

At this moment I’m in the process of rewriting some code that isn’t working. I spent weeks trying to figure out what it was doing. I traced the main execution path on a whiteboard, and found it goes through 10 layers of function calls, including a callback (but not including possible extra layers from one recursive routine). The source file is over 10,000 lines long. But if I insert a return statement at the top of the first routine there is no difference in results.

So far I’ve rewritten the equivalent of the first 5 layers (including the callback) in one routine that takes about 80 lines (comments included). That’s actually still a bit large for my tastes, but a major improvement. I may find more I can remove later.

Spartan comment.

@Tom
LONGER IS NOT MORE DESCRIPTIVE

Your example shows two long poorly named variables. I’d wager to say that two short poorly named variables are just as bad.

A longer, more clear name is always better than something short and cryptic, but both are going to fall apart when named almost identically.

So, anyhow, yes - longer is more descriptive.

I think the point is that going with either too terse or too verbose causes problems. Avoid extremism, be a moderate coder.