1
\$\begingroup\$

I have a promise where I'm taking the eventual result from the callback and storing it in a value that's scoped outside of the current block in a before hook, all in one line, like this.

describe('mongo stuff', () => {
  let project;

  before(() => {
    return mongo.findFirst('projects').then(data => project = data);
  });

  it('should have the correct company name', () => {
      expect(project.projectName).to.equal('Company1');
  });

});

Note: Mocha, the test runner I'm using, understands promises. That means, placing a return statement before a promise in a before hook, or in an it test, will halt the execution of the next task in the mocha test runner until the promise has been resolved. In other words, data => project = data will always completely execute before any tests that evaluate the contents of project are run.

I use data as a temp variable just so I can assign it to the higher-scoped project variable. This lets me use it in some later test like in the example above.

I was told in a code review that using an assignment as an expression isn't as clear as writing it out.

return mongo.findFirst('projects').then(data => {
    project = data;
});

Which do you think is better? Do you have any solid justification for avoiding the one-liner?

\$\endgroup\$
2
\$\begingroup\$

The only thing I think is "bad" when using the one-liner arrow is the implicit return. Whatever the expression evaluates to becomes the return value of the arrow function. In the case of promises, you should be careful when returning from within then callbacks since the return value becomes the resolved value of the promise.

Now in your case, the return value of then isn't going anywhere since the promise is not being used. So it shouldn't matter which form you should use. Therefore it should be fine.

A bigger problem is the entire snippet. You should make sure that project has a value by the time the code using it uses it or ensure that the code using it only runs after you fetch the value of project. If possible, since you have to wait for project anyways, you can make the call to get project synchronous instead.

|improve this answer|||||
\$\endgroup\$
  • 3
    \$\begingroup\$ I count 2 bad things there ;) The other thing you dont mention is that the use of project up top smells like a global in the worst way. \$\endgroup\$ – konijn May 1 '17 at 18:39
  • \$\begingroup\$ I'll update my question to include a mocha tag, as well as supplemental docs explaining how they understand promises. \$\endgroup\$ – Droogans May 1 '17 at 20:05

Not the answer you're looking for? Browse other questions tagged or ask your own question.