1
\$\begingroup\$

Given I have to write a simple function that essentially just does a http-fetch to the given URL but is part of a public API that will be used by thousands.

I have two main questions about the problem:

  1. Since this is a public API Im not sure how to handle inproper input. Should I return undefined or throw an error? (I rather throw errors to make it most obvious what types are expected). I've seen other API's however just returning undefined, which to me seems pretty stupid, as surely you dont want to be handling undefined in high level code?

    function at(_url, _options) {
    
        if (!_options)
            throw new ReferenceError("No options provided.")
    
        if (typeof _options !== "[Object object]")
            throw new TypeError("No options provided.")
    
        if (typeof _url !== "string")
            throw new TypeError("URL must be a string.")
    
        ...
    
  2. Is it too much to actually test if each possible inproper input is at hand? (I find myself writing the same test code for each function I write and after a while it seemed kind "wrong" to be doing typechecking in JS. Is there a standard for typechecking inside API's in JS?)

    const expect = require("chai").expect
    const fetchWithOptions = require("./../index")
    
    describe("me()", () => {
        it("should throw error if no options are provided", () => {            
            expect(() => fetchWithOptions.at("some-url")).to.throw(ReferenceError)
        })
    
        it("should throw an error if options are not an object", () => {
            expect(() => fetchWithOptions.at("some-url", "string options")).to.throw(TypeError)            
        })
    
        it("should throw an error if the provided url is not a string", () => {
            expect(() => fetchWithOptions.at(123123, {someOption: "someOption"})).to.throw(TypeError)                        
        })
    
    })  
    
\$\endgroup\$
  • \$\begingroup\$ Perhaps this test code is working but the code looks very much like stub code, thus it is off-topic. If you have complete working test code, then please edit your post to contain the entire code. See What types of questions should I avoid asking? \$\endgroup\$ – Sᴀᴍ Onᴇᴌᴀ Nov 14 '17 at 18:33
  • \$\begingroup\$ This is not stub code, its working of course and taken from my actual code base. I only renamed some things to not have the actual name of the api in here. Sorry I hadnt read the guidelines on posting here but "Given I have..." was merely false wording. \$\endgroup\$ – ThatBrianDude Nov 14 '17 at 19:01
  • 3
    \$\begingroup\$ Don't throw, return undefined. Every throw you add means the consumer of the API needs to write extra code to handle it. Don't use prefixed underscores!! Don't be so type strict, (what if _url is an object that has a toString method that will return the url when coerced to one). Use default parameters function at(url, options = {...defaultOptions}) that way you always have options as an object and it contains the correct data. \$\endgroup\$ – Blindman67 Nov 14 '17 at 19:57
  • \$\begingroup\$ Thank you so much for your feedback thats exactly what I need. However I have a few questions, dont you want to post as an anwser? \$\endgroup\$ – ThatBrianDude Nov 14 '17 at 22:23
  • 2
    \$\begingroup\$ 1. You say that the consumer must handle each throw, but thats not true if he himself makes sure that he only passes the right types. Isnt the returned undefined a much bigger problem then a thrown error as the consumer might miss the undefined while developing since he only passes correct values. \$\endgroup\$ – ThatBrianDude Nov 14 '17 at 22:25
2
\$\begingroup\$
  1. Assuming this function is async, neither. Provide a callback parameter or return a Promise (I'd favour Promises personally). This is a more common / understood pattern in JavaScript.

  2. Unfortunately no there isn't, it's very much part of the downsides (and beauty) of using a loosely-typed language. If you're concerned about type-safety then you should really take a look at TypeScript or equivalents.

With regards to the tests, personally I think covering each error scenario is fine - I do it myself, they're minimal effort and will help catch any regressions.

\$\endgroup\$
  • \$\begingroup\$ Thank you for the feedback, much appreciated. Regarding your first point however, I was returning a promise anyway. In this case however I would still be able to either resolve with undefined or throw an error/reject \$\endgroup\$ – ThatBrianDude Nov 15 '17 at 0:11
  • \$\begingroup\$ @ThatBrianDude rejecting with the relevant error makes the most sense to me, resolving with undefined is unintuitive. When a Promise resolves it's generally presumed to be successful, if something went wrong I'd expect it to be rejected. \$\endgroup\$ – James Nov 15 '17 at 0:19
  • \$\begingroup\$ Okay but what about when invalid types are provided? I guess thats a reason to throw an error isnt it? \$\endgroup\$ – ThatBrianDude Nov 15 '17 at 1:02
  • \$\begingroup\$ @ThatBrianDude yes, that's a perfectly valid scenario to throw an error. \$\endgroup\$ – James Nov 15 '17 at 16:02

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