I just coded the following loop which works just fine:

let position = 0;
// tslint:disable-next-line:no-conditional-assignment
while ((position = source.indexOf('something interesting', position)) > 0) {
    // do something interesting with something interesting
  position += 1;
}

But note I was obliged to add the tslint exception. Apparently somebody has decided that it is undesirable to allow assignment statements inside the while conditional. Yet this is the most direct code pattern I know for this particular situation.

If this is not "allowed" (or maybe I should say "discouraged") then what do the people driving this think the correct code pattern is?

I just coded the following loop which works just fine:

let position = 0;
// tslint:disable-next-line:no-conditional-assignment
while ((position = source.indexOf('something interesting', position)) > 0) {
    // do something interesting with something interesting
  position += 1;
}

But note I was obliged to add the tslint exception. Apparently somebody has decided that it is undesirable to allow assignment statements inside the while conditional. Yet this is the most direct code pattern I know for this particular situation.

If this is not "allowed" (or maybe I should say "discouraged") then what do the people driving this think the correct code pattern is?

• javascript
• typescript
• tslint

• 1 They also can be an indicator of overly clever code which decreases maintainability. - Gotta say, what you have written there makes my eyes bleed and I've been at this for a while. palantir.github.io/tslint/rules/no-conditional-assignment – Adam Jenkins Commented Nov 5, 2019 at 3:26
• I don't know about correct code pattern, but the following will make your co-workers hate you (a little) less: let position = 0; while(true) { if(source.indexOf('something interesting', position) <= 0) break; // do something interesting with something interesting position+=1; } – Adam Jenkins Commented Nov 5, 2019 at 3:32
• 1 @Adam I have been using this code pattern since the early '70s with the first C pilers that were available on a PDP-11. The syntax was written into K&R for a reason: it is concise and useful. – AlanObject Commented Nov 5, 2019 at 3:37

3 Answers 3

It makes the intent of the code harder to determine. Using assignment as an expression is unusual - if you see it inside a condition, how sure are you that the writer of the code intended to use the assignment, or might it be a typo? What if the writer of the code intended to pare the value? EG

while ((position = source.indexOf('something interesting', position)) > 0) {

vs

while ((position == source.indexOf('something interesting', position)) > 0) {

Those two are very different. (In this particular case, Typescript will warn you that the parison against the boolean and number is probably a mistake, but that won't happen with Javascript, nor in some other situations.)

Rather than assigning inside the conditional, assign outside of it, or even better, for the code above, you could probably use a regular expression (though it's impossible to say for sure without knowing the contents of // do something interesting with something interesting).

It looks repetitive, unfortunately, but an alternative is

let position = 0;
const getPos = () => {
  position = source.indexOf('something interesting', position);
};
getPos();
while (position > 0) {
  // do something interesting with something interesting
  getPos();
}

As in Adam's example in the ments, an alternative with unconditional assignment could look like this:

let position = 0;
while (true) {
  position = source.indexOf('something interesting', position);
  if (position < 0) {
    break;
  }
  // do something interesting with something interesting
  position += 1;
}

The syntax could bee even more pact if you disable curly, which enforces braces on if/for/do/while statements even though K&R Ch 3 does not mandate those braces at all*. This invites a good parison regarding the motivation for these rules in general. TSLint defines its purpose on its homepage:

TSLint is an extensible static analysis tool that checks TypeScript code for readability, maintainability, and functionality errors.

Is it entirely possible to use a brace-free if statement correctly, particularly if you are diligent about indentation and formatting? Of course. However, it is also entirely possible to misuse that feature in a subtle way, like in the docs for curly:

if (foo === bar)
    foo++;
    bar++;

Your question supposes "apparently somebody has decided that it is undesirable", but the more-accurate statement is that this pattern is a significant source of bugs across codebases, enough that the authors of tslint have offered built-in detection and feedback for that particular pattern. Rather than "undesirable", "not 'allowed'", or "discouraged", the pattern is simply rare-enough and misused-enough to have its detection on by default.

As with all rules in tslint, you can configure the default rule set using tslint.json or tslint.yaml, and you and your team may choose to disable rules like curly and no-conditional-assignment. This may be especially useful if this cases is often a false positive in your coding style (as it seems to be), or if you are applying tslint to an existing codebase where that pattern is mon and correctly-used. However, I think it is entirely reasonable for a team to claim (as others have done on this question) that the pattern is prone to =/== typos and other errors and that the correct uses are insufficient to justify allowing the pattern in their codebase.

*Note: Though K&R doesn't mandate braces, it does note on p56 that "The indentation shows unequivocally what you want, but the piler doesn't get the message, and associates the else with the inner if. This kind of bug can be hard to find; it's a good idea to use braces when there are nested ifs." This suggests that even the original K&R text is supportive of reasonable style rules or limits beyond what their syntax supports.

Yet this is the most direct code pattern I know for this particular situation.

Actually, it is not. A more direct and easier to read pattern is the following:

let position = 0;
while (position >= 0) {
    // do something interesting with something interesting
  position = source.indexOf('something interesting', position) + 1;
}

The code above works because indexOf returns -1 if the string is not found.