This Is Why We Test...

...to make sure our crap stuff actually works.

Somewhere, deep down inside the SQL Anywhere query engine, the promise that RAND() "is treated as a non-deterministic function" is being broken... perhaps it's because of all the GROUP BY and COUNT(*) stuff that was added to the SELECT.

Or maybe it's a feature... in this case, it doesn't matter, we just need it to work.

And here's how: Bite the bullet, save the million RAND() results in a simple temporary table, then run the query:

Also: Just because an older version isn't "fully supported" doesn't mean you can't download files (or ask questions, or get help), it just means there won't be any more new EBFs released. For a complete list of everything available for download see Downloads - EBFs / Maintenance.

the WINDOW ORDER BY clause determines what "PRECEDING" means (the preceding row by t.entry_date) and

the ROWS clause determines the size of the window (always two rows).

The expression AVG ( t.value ) OVER two_days on line 19 refers to the WINDOW clause by name, and it tells SQL Anywhere to compute the average of the two values of t.value that exist in the 2-row sliding window, for each row in the result set.

So, for '2012-02-02' the average of 10 and 20 is 15.000000,

for '2012-02-03' the average of 20 and 10 is 15.000000,

for '2012-02-04' the average of 10 and 30 is 20.000000,

... and so on ...

for '2012-02-10' the average of 10 and 60 is 35.000000.

Oops, what about the first row?

The '2012-02-01' row doesn't have a PRECEDING row, so what is the average over the moving window?

According to Glenn Paulley's white paper "in the case of a moving window, it is assumed that rows containing Null values exist before the first row, and after the last row, in the input."

That means when the moving window has '2012-02-01' as the "CURRENT ROW", the "1 PRECEDING" row contains NULL values... and when SQL Anywhere computes an AVG() that includes a NULL value, it doesn't count the NULL at all... not in the numerator or in the denominator when computing the average. Here's proof:

Monday, February 20, 2012

This question made me realize just how lucky I've been for several years, not having to deal with any Security Schemas Gone Wild!

Security is one of those areas of life that knows no physical bounds. People can eat only so much food, travel to only one place at a time, and are generally limited in their activities when those activities are based on real-world resources.

(even multitasking nutbars have their limits)

Security is an entirely different matter, especially computer system security: There is no limit to the rules and regulations that can be imposed by a database schema gone crazy... or if there is a limit, it is far beyond the point where administrators, end users and developers have been driven 'round the bend.

Examples? You want examples? ...not from me, I don't want anyone to get ideas.

In fact, I'm have second thoughts about this article, but here goes...

Here's the question: How do I provide insert, update, delete and select access to all tables and columns, to all users, but limit each user to a specific subset of rows in each table?

In other words, how do I provide row-level security?

Let's start with a simple base table that contains a primary key, some data columns plus a special "access_code" column that will be used to implement row-level security:

The CREATE VIEW ... SELECT on lines 1 through 7 sets up the basics: a view which looks exactly the same as the underlying base table. The "looks exactly the same" part can be important when it comes to updating base_table via the view... more on that later.

The WHERE clause on line 8 implements row-level access control: the end user can only see base_table rows that have a access_code value that matches what's in the @access_code variable.

(and where does @access_code come from? more on that later, too)

The WITH CHECK OPTION lets your end users INSERT, UPDATE and DELETE data via the view without affecting any base_table rows that a SELECT on the view wouldn't return in the first place; in other words, the WHERE clause is applied to the data that's being inserted, updated or deleted.

Creating an updatable view is a bit more difficult that a SELECT-only view. Most of the rules for updatable views are described in the Help, but not all the rules: A view can only be INSERT-able and DELETE-able if there is only one table in the FROM clause, no matter what the ansi_update_constraints option is set to. Of course, that's no problem here, the access control predicate is coded in a WHERE clause rather than an INNER JOIN ON clause.

(but it can become a problem, later on... that one-table-FROM-clause rule)

There is also a common-sense rule that an updatable view SELECT must contain all the columns required to code the INSERT, UPDATE or DELETE you actually want. For example, the view SELECT should probably include the underlying primary key columns for an INSERT, and for UPDATE and DELETE statements as well... unless you want to update/delete all the rows. That's why "looks exactly the same" as the base tables can make these views a lot easier to deal with.

Here's where the @access_code variable comes from, it's a connection-level variable that is created and assigned a value depending on the user id, whenever that user id connects to the database:

Connection-level variables like @access_code are visible to all the SQL run on the connection, including every reference to view_base_table from anywhere in the application or any stored procedure or trigger... plus, each connection has its own copy of @access_code that is inaccessible (invisible, in fact) to SQL code running on other connections.

The GRANT CONNECT and GRANT GROUP statements on lines 1 and 2 create a user group called "usergroup"... this is optional, but it does make administration easier when you have a lot of tables and a lot of users.

The GRANT GROUP and GRANT MEMBERSHIP on lines 4 and 5 provide name visibility on all the objects created by DBA to all the members of usergoup. Name visibility simply means you can refer to table t instead of having to code DBA.t; it doesn't actually give you the right to do anything useful like INSERT or SELECT, that's done separately. And being a member of the group called DBA doesn't confer any special powers: the DBA privilege is not inherited through group membership (whew!)

The GRANT EXECUTE on statement 7 lets everyone execute the connection_start procedure when they log in.

The GRANT statement on line 9 sets up each member of usergroup with all the necessary privileges on the view. This statement is the reason usergroup exists: Only one such GRANT is required for each view, not one GRANT for each view for each user... on a large system that brings the number of GRANT statements down to the hundreds from the thousands.

The three pairs of GRANT statements on lines 11 through 18 are all that is required to set up three users with (1) the ability to connect, (2) name visibility to all the stuff DBA owns and (3) INSERT, UPDATE, DELETE and SELECT privileges on the view.

GRANT CONNECT TO usergroup;
GRANT GROUP TO usergroup;
GRANT GROUP TO DBA;
GRANT MEMBERSHIP IN GROUP DBA TO usergroup;
GRANT EXECUTE ON connection_start TO usergroup;
GRANT INSERT, UPDATE, DELETE, SELECT ON view_base_table TO usergroup;
GRANT CONNECT TO user_a IDENTIFIED BY SQL;
GRANT MEMBERSHIP IN GROUP usergroup TO user_a;
GRANT CONNECT TO user_b IDENTIFIED BY SQL;
GRANT MEMBERSHIP IN GROUP usergroup TO user_b;
GRANT CONNECT TO user_c IDENTIFIED BY SQL;
GRANT MEMBERSHIP IN GROUP usergroup TO user_c;

Here's what works...

Here are some INSERT, UPDATE, DELETE and SELECT statements that work OK for user_a:

So what's all the fuss about?

The problem with row-level security is that CREATE VARIABLE is a gateway drug that only satisfies security cravings for a very short time... the next step is CREATE TABLE, followed by many-to-many relationships among multiple security tables implementing hierarchical access rules... by department, by job description, by time-of-day, by range-of-dollar-value... bwaaa ha ha ha!

Eventually, each single-table CREATE VIEW involves hidden joins to several other tables, and every five-way join among views becomes a performance nightmare involving twenty, thirty or more tables.

Why won't CREATE VARIABLE satisfy?

Because each user gets only one access code, thus limiting the relationship between users and rows, not to mention the different security relationships between one user and several tables.

But never mind that, consider this loophole: There's nothing stopping a user with an adhoc reporting tool from changing @access_code on the fly:

So no, CREATE VARIABLE makes a nice demo but the security high doesn't last long at all.

Moving on, from crack cocaine to CREATE TABLE

Here's a workaround to close the CREATE VARIABLE loophole. It involves a single-row connection-level table to replace the CREATE VARIABLE, plus modifications to the CREATE VIEW and connection_start procedures:

The end user is now prevented from changing the access code assigned in the connection_start procedure because the user doesn't have permission to UPDATE the connection_settings table.

Thus it starts: a security table is introduced, and the view WHERE clause introduces an implicit join with a second table... just to close one loophole. Not to increase flexibility in setting up security rules, for that you need more columns, more rows, more tables, more joins...

Friday, February 17, 2012

Question: How do I create a table that can contain only a single row for each connection, and that row exists only as long as that connection exists, and is visible only on that connection?

Answer: Start with a non-shared CREATE GLOBAL TEMPORARY TABLE statement to ensure that rows inserted by each connection are only visible on that connection and are automatically deleted when the connection finishes.

Then, use a CHECK constraint to ensure that each connection can insert only one row.

Here's an example of a connection settings table; "non-shared" is the default when the SHARE BY ALL clause is omitted:

The CHECK ( one_row_per_connection = 1 ) constraint enforces the "single row for each connection" requirement by limiting the primary key to a single value... since primary keys must be unique, that implies there can only be one row.

Tip: Don't forget to code NOT TRANSACTIONAL or ON COMMIT PRESERVE ROWS on all your GLOBAL TEMPORARY TABLE statements... unless you want your data in those tables to disappear every time you run a COMMIT. The default is ON COMMIT DELETE ROWS, one of those rare decisions that violates the rule "Watcom does things the way they should be done."

Neither the SELECT nor the UPDATE needs a WHERE clause because there's only one row.

Also, the UPDATE doesn't need a COMMIT, and it isn't affected by a ROLLBACK, because of the NOT TRANSACTIONAL clause. If you want your data to be involved in transaction processing, use ON COMMIT PRESERVE ROWS instead (or the default, ON COMMIT DELETE ROWS, if you're in that 0.01% who needs that kind of strangeness functionality :)

...and if you do use ON COMMIT PRESERVE ROWS instead of NOT TRANSACTIONAL, you going to need a COMMIT when you update your table... just sayin'.

Also: Just because an older version isn't "fully supported" doesn't mean you can't download files (or ask questions, or get help), it just means there won't be any more new EBFs released. For a complete list of everything available for download see Downloads - EBFs / Maintenance.

Friday, February 10, 2012

They both work exactly the same way, they both perform a useful function (return the first non-null argument), and they both have stupid names, so why the duplication?

Why not have just one function, with a decent name, like FIRST_NON_NULL()?

Answer: Here's a guess: It might have something to do with "Transact SQL compatibility" which is a historical requirement that by its very nature breaks the fundamental rule "WATCOM does things the way they should be done."

In this case, however, "compatibility" is not absolute: In Microsoft SQL Server the two functions do not work exactly the same way. For one thing, Transact SQL ISNULL() accepts only two arguments whereas Transact SQL COALESCE() accepts two or more.

But it gets worse...

When calling ISNULL() in Microsoft SQL Server the second argument "must be of a type that is implicitly convertible to the type" of the first argument.

That sounds OK, until you think about it.

Even after you think about it, it probably still sounds OK... ISNULL() could return either argument so they have to have compatible types, right?

It sounds OK until you actually run a test using Microsoft SQL Server 2008:

Wednesday, February 8, 2012

Way back in January I made a New Year's Resolution to "Post More Examples Here", so here we go (and I'm set for another year, right?)...

Question: How do I write a SQL Anywhere function that accepts a SELECT statement in a string parameter and returns a string containing the result set with each row delimited by [square brackets] and separated by commas, and adjacent columns separated by tilde '^' characters?

Answer: Phrases like "row delimited by" and "columns separated by" should make every SQL Anywhere developer think of the UNLOAD and OUTPUT statements... in fact, if you look up DELIMITED BY in the Help that's what you'll find.

In this case, OUTPUT won't work because it's unique to the ISQL utility and can't be embedded inside a function, so UNLOAD it will have to be.

Step 1: Build A Prototype UNLOAD SELECT

UNLOAD
SELECT * FROM Person ORDER BY first_name
TO 'c:/temp/result.txt'
ROW DELIMITED BY '],['
DELIMITED BY '^'
ESCAPES OFF
HEXADECIMAL OFF
QUOTES OFF;

The UNLOAD and TO clauses on lines 1 and 3 are wrapped around the SELECT to tell SQL Anywhere to send the entire result set to a text file.

The ROW DELIMITED BY clause on line 4 really should be called "ROW TERMINATED BY" because it tells SQL Anywhere to place the character string '],[' at the right end of each output row including the last. This isn't exactly what we want, but it's the only row-separator-or-terminator clause that UNLOAD gives us... so we'll take it for now and deal with it later.

The DELIMITED BY clause on line 5 really should be called "COLUMNS SEPARATED BY" because it tells SQL Anywhere to put the tilde '^' between adjacent columns... in this case, that's exactly what we want.

The remaining three clauses on lines 6 through 8 tell SQL Anywhere to write all the column values as simple strings with no extra formatting: no backslash-escape sequences, no \0x hexadecimal formatting and no 'single quotes' around string values.

The INTO VARIABLE clause on line 5 puts the output from UNLOAD into a local variable instead of a file, and the SELECT STRING statement fixes the two problems by tacking a '[' onto the front and chopping off the trailing ',['.

Now the formatting is OK:

result
'[Joe^2009-11-10^2012-02-01^1],[Mary^2009-11-08^2012-01-31^45],[Tom^2009-11-11^2012-01-30^12]'

Step 3: Turn It Into A General-Purpose Function

The UNLOAD statement expects a SELECT statement, not a string containing a SELECT statement like the function's supposed to use...

...what to do, oh, what to do?

Answer: Turn the whole UNLOAD statement into a string and use EXECUTE IMMEDIATE to run it!

The SET statement on lines 7 through 13 builds the entire UNLOAD statement and the EXECUTE IMMEDIATE on line 14 runs it.

The input SELECT is added to the UNLOAD on line 9.

The delimiter strings on lines 11 and 12 have their single quotes ''doubled up'' because they are embedded in the outer 'string literals'.

The magic of EXECUTE IMMEDIATE allows you to embed a reference to a local variable inside the statement to be executed (INTO VARIABLE @result on line 10) and then refer to that variable again afterwards: the LEFT ( @result, ... on line 15.

Also: Just because an older version isn't "fully supported" doesn't mean you can't download files (or ask questions, or get help), it just means there won't be any more new EBFs released. For a complete list of everything available for download see Downloads - EBFs / Maintenance.

Friday, February 3, 2012

The easiest kind of comment to write is Modification History: A comment which identifies the "What?" and "When?" of each change made to the program code and serves to answer questions like "Did I remember to make some change?"

Too bad Modification History comments are also be the least important kind of comment, certainly less important than Exhortations and Explanations, probably less important than Section Titles.

Nevertheless, Modification History comments can be helpful when they provide a concise overview of changes made from the developer's point of view... with the emphasis on "concise" and "developer's point of view" since version control utilities and even source code comparison programs can provide all the excruciating syntactical detail you would ever need.

Example 1

The first few lines of Foxhound's most important and most heavily-modified stored procedure looks like this:

Each Modification History comment is limited to one line, and includes the date, developer's initials and the build number which first included the change.

Some changes are eyecatchers, like "Published: Foxhound GA 1.2.4025a" which server to identify big events in the lifespan of the application.

Other changes include snippets of code to facilitate future searches: "Added: SELECT ... rroad_global_options.enable_schedules INTO ... @enable_schedules FROM rroad_global_options;"

When whole sections of code are affected, the Section Title comment is included, as in "Added: Code section: -- Check the connection count."

Generally speaking, only the most recent Modification History entries stand a chance of being important to the developer, which is why the older entries are moved out of the way: "(see bottom of file for earlier modification history)". In the case of this module there are 387 lines of Modification History comments at the bottom of the module... someday, they might be deleted, but they're not in the way and from time to time they're useful, so the effort required to choose the cutoff-point for each module has never seemed worthwhile.

Example 2

It is tempting to code Modification History comments down in the body of the code, close to the affected lines, especially when the effort required to make the changes was very high... but

Here's an example from inside a SQL script that contains CREATE TABLE and other definitions for tables that are subject to the "Data Upgrade" process when an old copy of the application database is upgraded to a new version:

-- ***********************************************************************
-- ***** EVERY CHANGE DOCUMENTED HERE MUST ALSO BE DOCUMENTED IN *****
-- ***** 015c_rroad_data_upgrade_startup.sql EVEN IF IT IS *****
-- ***** NOT SIGNIFICANT TO THE DATA UPGRADE PROCESS. *****
-- ***********************************************************************

The "MUST ALSO BE DOCUMENTED" exhortation is put into effect with a series of Modification History comments copied from the first module and embedded down in the code of the second:

Ugly, but easy to do, and easy to check later on to see if anything got missed... and yes, "checking later" is a frequent task.

The Bottom Line

If you're not going to do a thorough job of writing Modification History comments, then don't bother trying... nothing is more wasteful than a poorly executed task that results in a work product nobody uses.

"Thorough" doesn't mean "provide the same level of detail as a source code comparison program."

It doesn't even mean "document every trivial syntax error" that you fixed... but it does mean "list every non-trivial maintenance task you performed on the module"... on the code and on the other comments.

What? All of a sudden comments aren't important? Well, if that's how you feel, see "don't bother trying"... most don't.

Wednesday, February 1, 2012

The code comes first: If there's a choice to be made between writing clear, understandable program code and writing comments to explain less-than-readable code, comments should take a back seat.

The reason is simple: It's the code that determines what the program does, not the comments. Comments aren't even compiled, let alone executed. So it shouldn't come as a surprise that maintenance programmers concentrate on the code, often skipping over the comments altogether.

Sadly, however, even readable, understandable program code tells only part of the story: It only answers the question, "How is this program accomplishing its tasks?" Very few executable statements directly address the question, "What is this program doing?", with the possible exception of a CALL statement where the procedure name has been carefully crafted to answer the "What?"

There are no exceptions, however, when it comes to answering the questions, "Why is the program doing this thing?" or the much harder "Why isn't the program doing this other thing?"

So, comments come second, but

Explanations Are Important

Explanation: A concise comment describing the "What" and "Why" of a section of program code while avoiding the "How" which should be apparent from reading the code itself.

Example 1

Here is a short section of simple code (basically a SELECT FROM DUMMY) with several explanations, some good, some not so good:

------------------------------------------------------------
-- Perform the basic heartbeat or canarian query.
-- This is done to calculate the heartbeat time as well check the connection.
-- Note: The target database can be stopped and restarted, and the monitor should just keep trying to connect.
-- Note: When Foxhound itself is is stopped and restarted, reconnection to target databases *should* be automatic.
SET @sql = STRING ( 'SELECT dummy_col INTO @dummy_col FROM ', @proxy_owner, '.proxy_DUMMY' );
SET @canarian_query_started_at = CURRENT TIMESTAMP;
SET @canarian_query_finished_at = @canarian_query_started_at;
EXECUTE IMMEDIATE @sql;
ROLLBACK; -- release locks, recover from possible failed proxy connection
SET @connected_ok = 'Y';
SET @canarian_query_finished_at = CURRENT TIMESTAMP;
MESSAGE STRING ( 'DIAG ', CURRENT TIMESTAMP, ' ', CONNECTION_PROPERTY ( 'Number' ),
' 204-4 Monitor ', @sampling_id, ': First heartbeat OK' ) TO CONSOLE DEBUG ONLY;
-- Note: Because the proxy_DUMMY table is the last proxy object created, a
-- successful query against proxy_DUMMY may taken as proof that a connection exists
-- and is usable. This is important because a previous connection attempt
-- may be dropped (timed out) after it created some but not all necessary proxy objects.
-- Such a partly-established connection must be treated as no connection at all, and
-- that is guaranteed by creating proxy_DUMMY last.

In the comment "This is done..." the phrase "as well as check the connection" provides extra information about the "What?" of this code, so that's OK as an explanation.

The phrase "calculate the heartbeat time" is a bit redundant, however... not wrong, but not really necessary given the preceding section title "Perform the basic heartbeat or canarian query."

The two one-line "Note:" comments are a bit other-worldly because they state true facts that have very little (nothing?) to do with nearby code. These score zero on the Comment Value Scale; not a positive score because they offer no real value, but not a negative score because at least they aren't wrong.

The long "Note: Because the proxy_DUMMY table is ..." paragraph scores very high, however, because it explains a very important "Why?" for this otherwise trivial-looking section of code.

Example 2

This example shows two separate explanations that were added to the code at two separate times.

The later paragraph "It is possible..." came first, chronologically speaking, to explain why the unusual step of calling GET_IDENTITY was performed instead of just letting the later INSERT assign the value... so far, so good, the original "Why?" is covered.

------------------------------------------------------------
-- Calculate candidate primary key for successful sample.
-- Note: The following discussion is moot, but @candidate_sample_set_number is still
-- assigned because later code refers to it.
-- It is possible for this event to take some time before inserting a row
-- in rroad_sample_set. During that time, it is also possible for sampling to be
-- stopped, and a "sampling stopped" row to be inserted. The row inserted by this
-- event should have a lower primary key than the "sampling stopped" event, so it
-- cannot wait until the actual INSERT rroad_sample_set statement to calculate the
-- value of the autoincrement column sample_set_number. The following statement
-- pre-assigns a value which will only be used for the INSERT of a "successful"
-- sample set; if that INSERT is not performed because of some problem, there will
-- be a gap in the primary key sequence.
SET @candidate_sample_set_number = GET_IDENTITY ( 'rroad_sample_set' );

The second explanation, the previous "Note: ...", was added later to explain that GET_IDENTITY was being called through laziness rather than real need. This explanation isn't quite as thorough, but it does serve to prevent the new maintenance programmer from searching the code to find the reason GET_IDENTITY was called.

In a perfect world, the call to GET_IDENTITY would have been removed as soon as it became unnecessary. In an imperfect world, it's easier to add the "Note:" when the call became moot... not just because it's less work to write the comment than change the code, but also because comments aren't compiled so the "Note:" doesn't have to be tested.

Example 3

The following example almost got left out because at first glance it looked like just another bad explanation: Who needs to be told that sa_locks needs DBA authority?

------------------------------------------------------------
-- Create proxy_sa_locks if necessary.
-- This is only done after a full connection process.
-- Note: DBA authority is needed to call sa_locks.

Well, with this application (Foxhound) whether the user id has DBA authority or not controls on how much detail the user's going to get, and this comment explains to the maintenance programmer that information about row locks is going to be missing from that detail if the user doesn't have DBA authority.

So, this "Note: DBA authority is needed to call sa_locks" looks like a "How?" but it's really a "What?", albeit on a grander scale: "What does the maintenance programmer need to know about this code?"

Example 4

Here is a breathtakingly-verbose "Note:" that explains a very simple-looking IF NOT EXISTS RETURN control structure:

------------------------------------------------------------------------------------------------------------------------
------------------------------------------------------------------------------------------------------------------------
-- Step 5 - Check to see if sampling has been turned off.
------------------------------------------------------------------------------------------------------------------------
------------------------------------------------------------------------------------------------------------------------
IF @ok = 'Y'
AND @connected_ok = 'Y' THEN
-- Note: This code is an attempt to prevent a successful sample from appearing between
-- successive "Sampling stopped at" and "Sampling stopped" lines in the Monitor display.
-- This process may already be executing the code above this point when sampling is stopped by
-- a different process, and the @sample_started_at used by this process is calculated in the
-- next section... which is after the the "Sampling stopped at" point calculated by the other
-- process but before the eventual "Sampling stopped" point which will be calculated by some
-- (probably much later) process.
--
-- In the following example, the sample at Apr 22 5:12:21 PM should not have been recorded:
-- Apr 22 5:13:35 PM 1m 14.2s 0s / .1s ...
-- Apr 22 5:13:21 PM 1m 18.8s -- Sampling stopped --
-- Apr 22 5:12:21 PM 0s 0s / 0s 1 ...
-- Apr 22 5:12:02 PM 1m 24.2s -- Sampling stopped at --
-- Apr 22 5:10:38 PM 1m 10.1s .1s / .1s ...
--
-- In the following example, the sample at 5:11:23 PM should not have been recorded:
-- 5:11:32 PM 8.5s .1s / 0s ...
-- 5:11:27 PM ALL CLEAR - Alert # 1: Foxhound has been unable to gather samples for 1m or longer.
-- 5:11:25 PM 4m 54.7s -- Sampling stopped --
-- 5:11:23 PM 0s 0s / .1s ...
-- 5:06:30 PM 0s -- Sampling stopped at --
-- 5:06:30 PM 39.6s -- Foxhound stopped --
-- 5:05:50 PM 0s -- Foxhound stopped at --
IF NOT EXISTS ( SELECT *
FROM rroad_sampling_options
WHERE rroad_sampling_options.sampling_id = @sampling_id
AND rroad_sampling_options.sampling_should_be_running = 'Y' ) THEN
RETURN; -- do not record this sample
END IF;

Here's the reason for the long explanation: The bug very hard to find, and the fix so simple that no future maintenance programmer, not even the origial author, would understand the "Why?" of the IF-NOT-EXISTS-RETURN code without some help.

In fact, without the explanation, it's not clear that the code is even correct let alone necessary. Comments don't just save time, they can prevent mistakes (like removing a piece of code because it looks wrong.)

In this case the comment itself wasn't that hard to write: the sample data was gathered as part of the diagnostic effort, and the explanation was worked out carefully and even written down as part of that same effort... it took very little extra effort to copy the explantion into the code as an aid to future developers.

The Bottom Line

Explanations belong inside the code, not locked in the fading memory of the original author or lost inside some other document no developer will ever read.

That is, if you think explanations are important.

Folks who don't think explanations are important generally don't think readability's important either. That's why most code without comments is also unreadable... not because there are no comments, but because the author cares more about abstract measurements of "elegance" or believes that somehow readability contradicts correctness.