Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Code quality improvements #18

Closed
levino opened this issue Jul 5, 2016 · 1 comment
Closed

Code quality improvements #18

levino opened this issue Jul 5, 2016 · 1 comment

Comments

@levino
Copy link
Contributor

levino commented Jul 5, 2016

See https://github.com/figo-connect/node-figo/blob/master/lib/figo.js#L115 and following:

Currently (wrong):

if (this.statusCode === 401) {
  callback(new FigoError("unauthorized", "Missing, invalid or expired access token."));
} else if (this.statusCode === 403) {
  callback(new FigoError("forbidden", "Insufficient permission."));
} else if (this.statusCode === 404) {
  callback(null, null);
} else if (this.statusCode === 405) {
  callback(new FigoError("method_not_allowed", "Unexpected request method."));
} else if (this.statusCode === 503) {
  callback(new FigoError("service_unavailable", "Exceeded rate limit."));
} else {
  callback(new FigoError("internal_server_error", "We are very sorry, but something went wrong."));
}

Better:

if (this.statusCode === 401) {
  return callback(new FigoError("unauthorized", "Missing, invalid or expired access token."));
}
if (this.statusCode === 403) {
  return callback(new FigoError("forbidden", "Insufficient permission."));
}
if (this.statusCode === 404) {
  return callback(null, null);
}
if (this.statusCode === 405) {
  return callback(new FigoError("method_not_allowed", "Unexpected request method."));
}
if (this.statusCode === 503) {
  return callback(new FigoError("service_unavailable", "Exceeded rate limit."));
}
return callback(new FigoError("internal_server_error", "We are very sorry, but something went wrong."));

See also:

http://nodeguide.com/style.html#return-statements

levino added a commit to levino/node-figo that referenced this issue Jul 5, 2016
levino added a commit to levino/node-figo that referenced this issue Jul 5, 2016
levino added a commit to levino/node-figo that referenced this issue Jul 5, 2016
JeremyCraigMartinez added a commit that referenced this issue Jul 13, 2016
@JeremyCraigMartinez
Copy link
Contributor

Issue resolved: 5cffdbd

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants