I try to adhere to a number of guidelines.

Priority#1 is source code readability. Efficiency is lower in priority. In practice, I've also found that by improving readability, efficiency tends to also improve.

No doubt you'll find a number of cases where the code does not follow these guidelines.

There are a few rare cases of 'rules are there to be broken', but more often it'll be a 'to do'. I.e. if you spot such issue, the code probably needs some cleanup and review.

Modules

Split up the source code into modules.

Each module uses a single global, with a short uppercase name. All functions and all global variables related to the module are attached to this single global.

Examples: UTILS, LOG, STYC

Logging

The source code uses extensive logging. The idea is to have sanity checks all over the place, and report any issues found by using LOG.logError(...) or LOG.logWarning(...).

No code should ever silently fail.

Sort The Functions By Name

This is a soft rule. Occasionally, I'll sort the functions in a module by name. This is mostly for the benefit of human readers who are staring at a paper listing: it's handy to roughly know where to find a function in a module.

As most people will read the code in a smart editor that will take them to a function definition, this is not a high priority.

Name the function parameters with 'in_' or 'io_' prefixes.

This makes it easier on the human reader to figure out how to use the function (when inspecting the function header), or, while reading the function, where a certain variable comes from.

• Any globals will be prefixed by a module prefix (e.g. LOG.LOG_ERROR: that's a global defined in the module LOG).
• Any local variables don't have prefixes
• Anything coming in as a parameter is either 'in_' or 'io_'.

When the parameter prefix is 'in_' it means it's a pure input to the function. It's not a guarantee, but it means that the function is unable or not intending to make any sensible changes to that parameter value. It's roughly like a 'call by value'.

When the parameter prefix is 'io_' it means that the function expects to receive a reference to some object, and it will augment that object with some more info. It's roughy like a 'call by reference'.

Functions Always Return At The Bottom.

When reading a function, check the end of the function. If this is a real function, there will be a 'return' there. If it is a subroutine, there is no return.

While debugging, you should be able to put a breakpoint on that return statement, and the function should not use return anywhere else. Instead, 'early bailouts' should use the break option offered by the use of the do-while(false) construct.

Also, always store the return value in a variable (e.g. retVal or success or so), then use return retVal;.

This helps debugging by making the return value inspectable just before the function returns.

Function Wrapper

Nearly all functions are wrapped in a wrapper of the form:

do{
  try{
    ...
    if (somethingBad) {
      LOG...
      break; // bail out
    }
  }
  catch(err) {
    LOG
    ...
  }
}
while(false);

construct.

The outer do-while(false) is a non-loop: its only purpose is to enable the use of the 'break' statement as a 'bail out' for the function.

This allows reducing the level of nesting of if statements, and a more readable way of expressing the preconditions for calling the function.

The basic function structure is:

...
if (! something needs to be true) {
  break;
}
if (! something else needs to be true) {
  // LOG.logError("report it if the missing precondition is an error")
  break;
}
... test all preconditions...
// Finally, if all is well
...doTheFunctionThings...

The function is like a ladder, and the preconditions are the rungs.

Only if the flow can clambe down to the bottom of the ladder without 'falling off' will the core functionality of the function be executed.

do-while(false) is very similar to try-catch, but it has a lower overhead and is a lot faster. So, if your function is about to report failure, you should preferably bail out by break rather than throw.

The wrapper I use also has a try-catch. This is mostly to catch unexpected issues. When an issue is caught, it should always be reported to the log, to help debugging.

I routinely sanity-check all parameters to the function.

I don't re-throw: the function will catch the exception, then return a failure status (e.g. return undefined or false) which the caller ought to treat as 'it failed'. The caller can then bail out from itself, or try something else if it knows how to handle the issue.

Generally speaking the code will try to keep running as sensibly as possible, skipping over major issues, so the log file will contain a complete run worth of logging. Beyond a certain point it might all be irrelevant, but often, subsequent error reports still make sense to the user.

I want to avoid making code that stops at the first error, which forces the user into a 'run/fix/run/fix/run/fix/run' pattern. Instead I aim for 'run/fix/fix/fix/run/fix/fix/fix...'

LogEntry/LogExit

Nearly all functions can add log entries when they are entered and exited.

This logging is turned off most of the time, but it is there and available for debugging issues on user's computers. It is often not possible to use a debugger on a user's computer, and turning this logging on will provide some idea of where things go wrong.

If desired, it is easy to enhance the release version command line script to strip these calls, but in my experience any overhead they cause is pretty negligible.

Avoid Long Parameter Lists And Globals.

Most functions accept an object parameter 'io_context'. This is a 'grab-bag' object which contains a collection of data that is relevant to the current job.

E.g. it will contain a reference to the document being processed, it will contain lookup tables and caches,...

When the job completes, this object vanishes, so no 'footprints' remain that might affect the next run.

It also shortens the parameter lists to the functions and avoids using globals to 'share' lookup tables and caches.

Avoid Non-descript Variable Names

For example, do not use i or idx. Instead call it paragraphIdx or storyIdx or whatever it is.

Try To Avoid Nested For Loops

Add additional nested function calls in the body of the outer for loop.

Move literal constants to the module globals

I will try to always move any literal strings, numerical constants or regular expressions into named module globals.

E.g. instead of writing

var retVal = s.replace(/\s/g, "");

I'll have

UTILS.REGEXP_SPACES = /\s/g;
...
var retVal = s.replace(UTILS.REGEXP_SPACES, "");

This improves the readability (who likes disassembling a RegExp to figure out what it does), speeds up the code (by pre-compiling any RegExp a single time only), and avoids typos (e.g. if you mis-type STYC.ReportHeder instead of STYC.ReportHeader you will get an undefined which should be flagged by the preconditions or try-catch. If you mistype "ReportHeder" instead of "ReportHeader" in a literal string, that won't cause any debuggable issues).

Global Constants Or Variables Names

Global names have a 'rhythm' to them.

Generally, globals are named by where they are, what they are, then what it does or what it is.

E.g. UTILS.REGEXP_SPACES: it's in the module UTILS. It's a RegExp. All other RegExp should also have a name starting with REGXP_.

If there is a specific unit, it's appended as a suffix.

E.g. UTILS.TIMEOUT_WAIT_FOR_EXTERNAL_PROCESS_MILLISECONDS. It's in UTILS. It's a TIMEOUT. We WAIT_FOR_EXTERNAL_PROCESS. It is expressed in MILLISECONDS.