4
\$\begingroup\$

The code below is part of a toy test assertion library I've written.

I'm looking for feedback specifically on lines 4 - 9. I'd like to extract them from the throws function. I can think of two options.

  1. Create a function with a name like maybeReportExceptionError(). This seems not great because a) it feels like the function has two responsibilities and b) the maybe in the name seems hard to understand for someone not familiar with the inner workings of the function.

  2. Replace the if/else if with a polymorphism. I'm not sure how to go about this.

Any thoughts greatly appreciated!

function throws(testFunction, expectedMessage) {
  var thrownMessage = getFunctionExceptionMessage(testFunction);

  if (thrownMessage === undefined) {
    throw new Error("Expected an exception");
  } else if (thrownMessage !== expectedMessage) {
    throw new Error("Threw - " + thrownMessage + " - not - " + expectedMessage);
  }
};

function getFunctionExceptionMessage(fn) {
  try {
    fn();
  } catch (exception) {
    return exception.message;
  }
};

// USAGE:

// Throws "Error: Expected an exception"
throws(function() {

}, "Expected message");

// Throws "Error: Threw - Thrown message - not - Expected message"
throws(function() {
  throw new Error("Thrown message");
}, "Expected message");
\$\endgroup\$
4
\$\begingroup\$

It kind of boils down to coding style, but this is how I'd refactor your code.

  1. I like my assertions to clearly state they are; rename throws() to assertThrows().

  2. I like guard clauses rather than "unneccessary" else statements.

  3. If a function ever throws an exception with an undefined message, your assertion would fail.

  4. I like a specific format of assert failures: expected xxx but got yyy.

Code:

function assertThrows(testFunction, expectedMessage) {

    var actual = catchExceptionFrom(testFunction); 

    if (actual === undefined)
        throw new Error("Function did not throw an exception.");

    if (actual.message !== expectedMessage)
        throw new Error("Expected exception '" + expectedMessage + "' but got '" + actual.message + "'");
}; 

function catchExceptionFrom(fn) { 
    try { 
        fn(); 
    } 
    catch (exception) { 
        return exception; 
    } 
};
\$\endgroup\$
  • 1
    \$\begingroup\$ This is really helpful. All your refactors make sense. Thank you! \$\endgroup\$ – Mary Rose Cook Jul 22 '16 at 18:50

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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