Comment On Self Documenting

Look at the output running nm on a library or executable on a linux system. You'll see all your classes, methods, enums and so much more!

It's a Cock! It's a Fist! It's Zoonerman!:

Fields in Fantom

Why did it take so long to get properties right?

What do you mean by right? Fantom only generates the basic setters and getters for you if you don't write them yourself, so it still follows the same idea. They do this "So if you add one later you won't have to recomoile everything" whuich is why a lot of us provide the setter and getter even though it isn't really doing much.

I will still stand on my previous words though, if all you have is that in a business object where are your rules for proper values? A phone number setter should at least check for proper formatting, a quantity should at least check for proper range limits, etc.

KattMan:

CAPTCHA:sino:

Sorry that was an offtopic rant. Also I hate setter and getter functions that only consist of "this.value = value" or "return this.value". Anyone who creates a language where they are necessary is a dickweed arsehole.

Look into encapsulation and the "law" of demeter and you will know why these simple get/set methiods exist. It's not the language that requires them, you can make your variables public, but then the thing that owns them has no control over when and how they are updated.

No, this is an artifact of the languages you use, and you're so used to it that you can't imagine it any other way. Look at the way Python, for instance, implements "properties". You have a public variable, and then later, if you need access to that public variable to go through a method, you can change it to do that transparently to the code that uses that class doesn't need to change. Only in C++ and the languages derived from it (e.g. Java) do you see everyone "defensively" creating getters and setters for everything right off the bat.

KattMan:

It's a Cock! It's a Fist! It's Zoonerman!:

Fields in Fantom

Why did it take so long to get properties right?

What do you mean by right? Fantom only generates the basic setters and getters for you if you don't write them yourself, so it still follows the same idea. They do this "So if you add one later you won't have to recomoile everything" whuich is why a lot of us provide the setter and getter even though it isn't really doing much.

I will still stand on my previous words though, if all you have is that in a business object where are your rules for proper values? A phone number setter should at least check for proper formatting, a quantity should at least check for proper range limits, etc.

By "right", I mean you're not required to right any more code than is necessary, whether you want to validation/calculation code at first or not.

C# disappointed me with their attempt at auto-implemented properties which often still require you to declare a separate, private field.

Oh, my $_DEITY, they're Greenspunning COBOL (the language in which a short novel about the program is the program)!

Rob C.:

Depends on the consumer of the code. For someone who is likely to be modifying the code itself, comments as documentation tend to become stale and eventually misleading. Lazy coders will modify the code without touching the comments, and the next thing you know the comment is talking about something dozens of lines away, and is inaccurate due to implementation changes.

True, but it's not clear how that is an argument in favor of code like the example here. It is true that a programmer may change what a function does and forget or not bother to change the comment. But it is surely even more true that a programmer would be unlikely to change function or variable names. That would require finding all the references and updating them.

I have fond memories of a function I once wrote that I called "validateStockNumber". It accepted a stock number as a parameter, looked it up on the database, and returned true if it was found and active, false otherwise. Than another programmer came along and changed this function to also do a bunch of additional work and update the database. I really disliked the fact that a function whose name implied it just did a validation in fact also did database updates. Later someone made another change that removed the validation. But he left the function name unchanged. It was now totally misleading. Some time later another programmer had to build a VB interface to the Java code, mostly consisting of pass-thru functions. He called the pass thru on this one, "ValidateStockNumberButReallyUpdateShippingManifest".

Correct me if I'm wrong, but (in the case of .NET) doesn't compiling to CLR make it so this doesn't make it take extra memory?

adiener:

Controller talking directly to (or instantiating?) the view? Not the most ideal application of MVC either.

In ASP.NET the Controller does not talk to the view, it returns an ActionResult to the ASP.NET MVC framework. So the line "return View("Index", form);" just means: "After processing my Action, I want the framework to open the "Index" view and pass in this 'form' ViewModel object."

You can easily write a unit test against this Controller Action. After calling an Action, you can test if the ActionResult is typeof(View) with the ViewName ("Index") and ViewModel ('form') you were expecting

This embodies the highly questionable idea that longer names are always better. Who says?

I agree that extremely short names, like "x" or "i1", are likely to be uninformative.

But extremely long names mean that figuring out whether two names are the same is now a chore. If I have a variable called "AmountBilledToTheCustomerForShippingChargesOnPackagesSentOverseas" and another called "AmountBilledToTheClientForShippingChargesOnPackagesSentOverseas" and another called "AmountBilledToTheCustomerForShippingChargesOnMailersSentOverseas", etc, it is very easy to get confused when trying to make sense of the program.

No name you give is going to tell the reader exactly what the function does unless the name repeats all of the code contained in that function. At which point, why bother to write a function at all? Just write the code in-line. At best, a name gives the reader a clue what the function is for. At a minimum, it should be meaningful enough that once the reader has figured out what it does, when he sees the name again he can recognize it.

Sure, suppose I create a function to calculate sales tax and I call it "f32". Then I have a function to calculate shipping charges that I call "f33", a function to apply discounts that I call "f34", etc. When I come back to this program in a few months it's unlikely I'll remember which was which. Even after I figure it out again, it will be easy to get confused working my way through the code.

At the other extreme, if I call them "FunctionToCalculateTheSalesOnASaleMadeToACustomerFromOneOfOurStoresOrOverTheInternet", "FunctionToCalculateTheShippingCharesOnASaleMadeToACustomerFromOneOfOurStoresOrOverTheinternet", etc, that will be almost as painful to read and comprehend.

But if I call them "calcSaleTax", "calcShipping", and "applyDiscount", the names are reasonably descriptive, easy to remember, and easy to read.

Let me add that if two functions are similar, the name should tell me how they are similar and how they are different. Like "calcDomesticShipping" and "calcInternationalShipping". Then I can clearly see that both have to do with shipping but one is about domestic and the other is about international. It drives me nuts when programmers give two functions similar names with no clue about how they are different. Like just adding a digit, so we have calcShipping and calcShipping1. Or deliberately mis-spelling a word, like calcShipping and calcShppng. Then I have to guess how they're different.

Okay, I'm off on a tangent of at least somewhat serious discussion rather than the simple ridicule and meme-repetition. Sorry.

Geoffrey T. Buchanan:

I suppose modern businesses tolerate the complete waste of resources today because computers have got more powerful and the customer rarely complains. But you have to wonder how much faster typical software would run if it was written as per the old ways. There's also a slippery slope in play - out there now are a load of upstart "programmers" who are making all the software we use but they have no idea about how computers work let alone basic compiler theory. And people wonder why there are so many security holes and viruses in software these days...

/rant

It's written for the .NET framework so it will have terrible performance anyway compared to an unmanaged natively compiled executable. Any further performance penalty caused by terrible coding practices will probably come out in the wash.

Jay:

Okay, I'm off on a tangent of at least somewhat serious discussion rather than the simple ridicule and meme-repetition.

Frist! Irish Girl! Bring back MFD! Oh....sorry.

Jay:

Let me add that if two functions are similar, the name should tell me how they are similar and how they are different. Like "calcDomesticShipping" and "calcInternationalShipping".

Or better yet, have a DomesticOrder class and a InternationalOrder class that both inherit from an abstract base Order class and implement their own CalcShipping functions.

That way the code becomes self-documenting without adding blot to function names. One is DomesticOrder.CalcShipping and the other is InternationalOrder.CalcShipping. If you're looking at CalcShipping inside the InternationOrder class, it should be obvious that it's calculating international shipping! And you don't have to mess around with lots of:

if (isDomesticOrder)
order.CalcDomesticShipping();
else
order.CalcInternationShipping();

Now it's just

order.CalcShipping();

big picture thinker:

Geoffrey T. Buchanan:

I suppose modern businesses tolerate the complete waste of resources today because computers have got more powerful and the customer rarely complains. But you have to wonder how much faster typical software would run if it was written as per the old ways. There's also a slippery slope in play - out there now are a load of upstart "programmers" who are making all the software we use but they have no idea about how computers work let alone basic compiler theory. And people wonder why there are so many security holes and viruses in software these days...

/rant

It's written for the .NET framework so it will have terrible performance anyway compared to an unmanaged natively compiled executable. Any further performance penalty caused by terrible coding practices will probably come out in the wash.

Obvious troll is obvious.

Anon:

Or better yet, have a DomesticOrder class and a InternationalOrder class that both inherit from an abstract base Order class and implement their own CalcShipping functions.

Not been here long have you. NEVER assume someone might actually do the sane thing.

Say what you will about long object names. I'm working with a database designed by a guy who grew up in an era when every fucking byte was precious. If he could still pack his number fields, he would. I've spent (not spended) a week trying to follow his cryptic mess, and I'm ready to kill someone.

Jay:

Rob C.:

Depends on the consumer of the code. For someone who is likely to be modifying the code itself, comments as documentation tend to become stale and eventually misleading. Lazy coders will modify the code without touching the comments, and the next thing you know the comment is talking about something dozens of lines away, and is inaccurate due to implementation changes.

True, but it's not clear how that is an argument in favor of code like the example here. It is true that a programmer may change what a function does and forget or not bother to change the comment. But it is surely even more true that a programmer would be unlikely to change function or variable names. That would require finding all the references and updating them.

That's an interesting conundrum. Personally I'm pretty anal about changing things like this everywhere, but in part that's because the barrier to doing so is pretty small with an IDE. Maybe one isn't inherently better than the other, I'm just so accustomed to one approach it seems intuitively easier.

Jay:

I have fond memories of a function I once wrote that I called "validateStockNumber". It accepted a stock number as a parameter, looked it up on the database, and returned true if it was found and active, false otherwise. Than another programmer came along and changed this function to also do a bunch of additional work and update the database. I really disliked the fact that a function whose name implied it just did a validation in fact also did database updates. Later someone made another change that removed the validation. But he left the function name unchanged. It was now totally misleading. Some time later another programmer had to build a VB interface to the Java code, mostly consisting of pass-thru functions. He called the pass thru on this one, "ValidateStockNumberButReallyUpdateShippingManifest".

Someone in that chain of failure deserved a slap. Probably the dev who added side effects to a setter.

My eyes hurt.

Not a WTF at all.

Actually, the code IS self-documenting. A "manager" can read it, and determine the business cases that the code implements. A modern COBOL!

About the only thing I would change (in a code review) would be
"ThereIsAtLeastOneSharedActivityTypeBetweenTheEventAndCurrentUser"

That function is not a declaration, but a predicate, and should be named

"IsThereAtLeastOneSharedActivityTypeBetweenTheEventAndCurrentUser"

Now, personally, I would prefer underscore separators for readability, but that is secondary. (I would also like the ability to use ? in names if this sort of thing is being done).

("Is_there_at_least_one_shared_activity_type_between_the_event_and_current_user?" would be better, because it wouldn't slow down reading as much)

The wrapping of "true" and "false" is pure syntactic sugar. The compiler should be capable of optimizing these completely away, anyway.

The logic is approachable by a reader who is not capable of abstract thought. It embodies the business cases. It is maintainable (by use of IDE tools). There are no comments to get out of date. The path of least resistance is to properly update the code, and the "manager" will be able to tell if it hasn't been done correctly.

Really, for most business logic, this is almost perfect. If this style could have been conceived when COBOL was designed, it would have been.

Lurch:

Not a WTF at all.

Actually, the code IS self-documenting. A "manager" can read it, and determine the business cases that the code implements. A modern COBOL!

About the only thing I would change (in a code review) would be
"ThereIsAtLeastOneSharedActivityTypeBetweenTheEventAndCurrentUser"

That function is not a declaration, but a predicate, and should be named

"IsThereAtLeastOneSharedActivityTypeBetweenTheEventAndCurrentUser"

Now, personally, I would prefer underscore separators for readability, but that is secondary. (I would also like the ability to use ? in names if this sort of thing is being done).

("Is_there_at_least_one_shared_activity_type_between_the_event_and_current_user?" would be better, because it wouldn't slow down reading as much)

The wrapping of "true" and "false" is pure syntactic sugar. The compiler should be capable of optimizing these completely away, anyway.

The logic is approachable by a reader who is not capable of abstract thought. It embodies the business cases. It is maintainable (by use of IDE tools). There are no comments to get out of date. The path of least resistance is to properly update the code, and the "manager" will be able to tell if it hasn't been done correctly.

Really, for most business logic, this is almost perfect. If this style could have been conceived when COBOL was designed, it would have been.

A person who is not capable of abstract thought should not be allowed anywhere near code.

Jay:

This embodies the highly questionable idea that longer names are always better. Who says?

I agree that extremely short names, like "x" or "i1", are likely to be uninformative.

But extremely long names mean that figuring out whether two names are the same is now a chore. If I have a variable called "AmountBilledToTheCustomerForShippingChargesOnPackagesSentOverseas" and another called "AmountBilledToTheClientForShippingChargesOnPackagesSentOverseas" and another called "AmountBilledToTheCustomerForShippingChargesOnMailersSentOverseas", etc, it is very easy to get confused when trying to make sense of the program. [...]

In .NET, it's called Namespace. In Java, it's called Package. In English, it's called context.

Truly, such long method names are not necessary iff* the path and class are named properly as well. Architecting properly encapsulated features will lead to extremely easy to read, modify, and maintain code bases.

Example: CustomerOperations.Sales.ShippingCalculator()
If I am already within the Sales package/namespace, then it is obvious I what the ShippingCalculator refers to. If I am in the MerchandiseOperations.Inventory.Warehouse package/namespace, then a call to such a distant package/namespace should raise a code smell.

* as in if and only if, not merely a typo

LANMind:

Say what you will about long object names. I'm working with a database designed by a guy who grew up in an era when every fucking byte was precious. If he could still pack his number fields, he would. I've spent (not spended) a week trying to follow his cryptic mess, and I'm ready to kill someone.

I've kilt (not killded) many ones for far less.

Anyone else read:

private bool TheFormIsInvalid()

{

    return ModelState.IsValid == false;

}

as:

private bool TheFormIsValid()

{

    return ModelState.IsValid == false;

}

the first time they saw it? Because obviously that's much more semantically clear and "documenting" than a negative boolean test:

if (!ModelState.IsValid)

But now I'm just regurgitating the original article.

Only two reasons exist to move logic into a separate method/function:
1) It's used in multiple places.
2) It needs to be invoked separately from the code block it's being removed from (which is really just #1 in reverse).

Otherwise all you're doing is wasting your time and, more importantly, the time of any poor soul having to look at your code abortion after you've moved on.

If you're not capable of processing more than one logical operation per line of code, then you should really go back to creating refrigerator art with your crayons.

I remember when software guys were not good enough for the engineer title and double E's would rant when ever someone used software and engineer in the same sentence. LOL

TheSi:

You mean 'spelt'

No, he means 'spelled', as in casting a spell.

Lost in Code:

I remember when software guys were not good enough for the engineer title and double E's would rant when ever someone used software and engineer in the same sentence. LOL

As a Mechanical Engineer who finds himself in the software business because Boeing found it easier to teach programming to Engineers than to teach engineering to CS grads, I have to say it's not just the EE's who feel that way. IMHO software guys may never be good enough for the Engineer title. Engineering is a discipline few CS grads follow.

me:

I remember when Engineers had to have degrees and built tangible things.

We still do. CS grads are NOT engineers any more than working at McDonalds makes you a chief.

Engineering is a discipline few CS grads follow.

So, in your opinion, could the principles of engineering be applied to the construction of software if they were taught and practiced?

mott555:

public string TextToDisplayWhenThisCommentIsFrist()
{
return "FAIL";
}

FTFY.

Geoffrey T. Buchanan:

Back then 16kb was considered a LUXURY and we had to learn to write highly efficient and compact code so that it would PERFORM.

You got to *write* your code? LUXURY! LUXURY! Back in MY day we had to chisel our code into stone tablets! Without our bare hands!

The Great Neckbeardio:

Geoffrey T. Buchanan:

Back then 16kb was considered a LUXURY and we had to learn to write highly efficient and compact code so that it would PERFORM.

You got to *write* your code? LUXURY! LUXURY! Back in MY day we had to chisel our code into stone tablets! Without our bare hands!

You got to chisel your code? Paradise! Back in MY day, we had to erode our code out of the stone with precisely-placed water - when we had even that! I kilt many a developer to get the necessary blood when water was scarce.

Nickster:

Engineering is a discipline few CS grads follow.

So, in your opinion, could the principles of engineering be applied to the construction of software if they were taught and practiced?

There are people out there doing precisely that. They're the decent end of our industry, and should be encouraged.

There is discussion in the more professional end of our industry about certification/chartership, and there's a common feeling that it's more "when" than "if".

Rob C.:

Jay:

Rob C.:

Depends on the consumer of the code. For someone who is likely to be modifying the code itself, comments as documentation tend to become stale and eventually misleading. Lazy coders will modify the code without touching the comments, and the next thing you know the comment is talking about something dozens of lines away, and is inaccurate due to implementation changes.

True, but it's not clear how that is an argument in favor of code like the example here. It is true that a programmer may change what a function does and forget or not bother to change the comment. But it is surely even more true that a programmer would be unlikely to change function or variable names. That would require finding all the references and updating them.

That's an interesting conundrum. Personally I'm pretty anal about changing things like this everywhere, but in part that's because the barrier to doing so is pretty small with an IDE. Maybe one isn't inherently better than the other, I'm just so accustomed to one approach it seems intuitively easier.

It's not an argument in favour of code like in the article - there's a huge difference between that insanity and naming functions/variables to make it clear what's going on while using a minimum of comments.

Yes, when you change what a function does, you have to rename it. Refactoring is normal - that's what you've got your test suite for, to allow you to do it safely.

If you can't rename your function when you change what it does, you've got serious problems with your codebase or processes anyway.

A bit like Coco !

minitech:

I kilt many a developer to get the necessary blood when water was scarce.

Why would putting those developers in skirts help? Well, assuming they're not simultaneously Mel Gibson.

Some Damn Yank:

CS grads are NOT engineers any more than working at McDonalds makes you a chief.

Of course not. One becomes a chief by proving oneself brave and victorious in battle, repeatedly. Also by poisoning one's predecessor's bear claw soup.

stew:

Only two reasons exist to move logic into a separate method/function:
1) It's used in multiple places.
2) It needs to be invoked separately from the code block it's being removed from (which is really just #1 in reverse).

3) It's long and/or complex enough to be worth black-boxing, e.g.

PerformMainTask() {

  if (GuardCondition()) return;

  DoSomethingWithHeaderRecord(header);

  for each detail in details {

    DoSomethingWithDetailRecord(detail);

  }

}

where each of the three functions is, say, about 50 lines long.

Paul Neumann:

I've kilt (not killded) many ones for far less.

Oh, so, "wearing a kilt" actually means showing off your prey?

I am really surprised some of the commenters think that this is not a WTF. However, it explains where the WTFs like this come from.

dkf:

minitech:

I kilt many a developer to get the necessary blood when water was scarce.

Why would putting those developers in skirts help? Well, assuming they're not simultaneously Mel Gibson.

It's not a skirt, ye bloody Sasunnach!

Nickster:

Engineering is a discipline few CS grads follow.

So, in your opinion, could the principles of engineering be applied to the construction of software if they were taught and practiced?

I think of it kind of like plumbing. One one hand you have physicists designing piping in a liquid fuel rocket, on the other someone is gluing PVC pipe fittings together. In traditional engineering there is also a lot of beating on things until they work.

It really depends. In aircraft design, every excess ounce comes out of the payload. So you pay to carry it around. Which means management is motivated to pay to get rid of it. Other fields extra weight is a one time cost that may not justify the NRE to get rid of it.

clive:

amis:

What is so wrong with comments? I see this kind of crap and wonder what anybody has against comments.

I'm guessing you've not been doing dev for long enough to learn from experience. Comments in theory are great. In real life they're often wrong or useless.

Comments are code, and if they are there should be treated to the same quality control process. Unfortunately they're invisible to much of that process - the computer ignores them so there's no compile or test failure, so you're left with stuff like peer review, and too many people simply don't understand the problem.

If you're one of the few % who maintains your comments as well as your code, I'll congratulate you. The rest get me grumbling at them.

That's one problem with comments, but often they still give you an idea of what was going on. Even better is if you check the file history to see the comment & code at the time it was inserted.

However I see a greater issue with this particular alternative. If one of the Self Documenting functions changes, so that the name is no longer accurate, the developer would have to rename the function and all calls to it (sometimes that's easy, but other times it can be a nightmare). Or more likely they'll just leave it with the same name and you'll get a similar issue to obsolete comments, only with it being an obsolete function name (which personally I'd find far more misleading).

private static bool NoActivityTypesAreAttachedToThisEvent(IEnumerable<ActivityType> activityTypes)
{
return activityTypes.Any() == false;
}

or

private static bool NoActivityTypesAreAttachedToThisEvent(IEnumerable<ActivityType> activityTypes)
{
return !activityTypes.Any();
}

Exactly my point - I'm so happy not to be the only one with this experience.

Picking up ten year old code with no change history or source repository means that you benefit from every bit of information you can glean whether code or comment.

Where is my deer stalker hat, Watson?

When reading existing code, I tend to completely ignore any comments. Mainly because comments are mostly wrong because they have been correct only at the exact moment when they were written (maybe not even then), and they almost never get updated, so much of the time, they are incorrect if not misleading.

I prefer well-named identifiers (at all levels, be they function names, variable names, class names or whatever) at any time. But the identifiers shown in this daily-WTF take the cake. These names are less than helpful.

The identifier names in this code are a little wordy, but worse things happen in code... much worse!

I would take issue however with the TrueBecause... and FalseBecause... methods, whose names appear to expose their implementation. What if I wanted to subclass, and change the return value from true to false? That would generate a real WTF.

Good program design is about preventing future WTFs, and therefore saving the cost of refactoring them away (refactoring works, but it is expensive).

In review, one might suggest toning down the wordiness a little, but one should not let that distract attention from more substantial isses.

If one of the Self Documenting functions changes, so that the name is no longer accurate, the developer would have to rename the function and all calls to it

Seems to me this would be an occasion to take a step back, and ask "why are we breaking this function?" If it no longer does what it says, perhaps the structure of the program has changed enough to warrant refactoring.

It may also be possible to divide the new functionality between 1) a new function with a helpful name and 2) a subfunction called by the now "incorrectly" named function.

Shouldn't self documenting code be about what it does rather than why?
Knowing what a method does by the method name is a great but the "TrueBecauseThisEventDoesNotRegistrictBasedUponActivityType()" being a a ramble over why you return a true value.

Maybe AddCommentToArticleBecauseItIsMyLunchTimeAndAContributionMayBeHelpful()
is the new Article.Comments.Add()?

ObiWayneKenobi:

Actually, writing code like that (correctly though, since the method shouid be "TheFormIsValid") is a good thing. The Ruby community in particular is fond of creating little wrapper functions around calls simply to make them read better.

This developer had his priorities backwards, however.

"... simply to make them read better." : I used to work in a military bureaucracy. That means a two line letter (not counting salutation, etc.) took 3 weeks to "make it read better." (a true fact! (pun intended)) At some point someone has to say "the goal is understanding, not the Pulitzer for literature."

If Marie Antoinette were a programmer, she would say "Let them code COBOL". If one want's to read code as if proper English, then I suggest COBOL which has the concepts of "paragraphs" and sentences"; and COBOL-68 in particular where operators had to be spelled out: "COMPUTE X EQUALS Y MULTIPLY X PLUS A." (Don't forget the period!! There. That will make those morons happy. I understand 'zactly what that calculation is about!!

Isn't this just how most average programmers write their code? When I look at other people's code, it usually looks about like this to me: abusively camel-cased, with everything wrapped in stupid little functions.

I feel quite certain that this code conforms to the misguided standards of the group tbat produced it.