Error handling suggestion

[asfktz]

Hi @eshaham!

I wanted to suggest a small change in the error handling logic of scaper.js: Using negative conditions instead of positive ones.

The way, we can first eliminate the "bad" scenarios and end up with the optimistic path at the end:

if (!encryptedCredentials) {
  // could not find credentials file
  return;
}

if (!result.success) {
  // error: result.errorMessage
  return;
}

// At that point, we're safe to assume that we got everything we need

for (let i = 0; i < result.accounts.length; i += 1) {
  if (account.txns.length) {
    // exporting transactions for account
  } else {
    // no transactions for account
  }
}

// csv files saved

Another advantage is that when additional conditions will be added in the future, the code is less likely to grow to the right:

if (additionalCondition) {
    if (encryptedCredentials) {
      // success: result.success
      if (result.success) {
        for (let i = 0; i < result.accounts.length; i += 1) {
          if (account.txns.length) {
            // exporting transactions for account
          } else {
            // no transactions for account
          }
        }
        // csv files saved
      } else {
        // error type: result.errorType
        // error: result.errorMessage
      }
    } else {
      // could not find credentials file
    }
} else {
    // another error
}

It's just a cosmetic change in the end.

What do you think? (:

[eshaham]

@asfktz personally I despise empty return statements. I realize sometimes it makes indent level too much, but the flow is clearer that way, at least to me. So I prefer not to merge this one 🙏

[asfktz]

No Problem 😊

I'll keep it in mind for the next PRs