0

I have a couple of async method that is basically a yes and no reply isUsernameAvalible() isOnline() isImage() yada yada... you probably get the point by now.

As a demonstration... try to think of this as a more generic async promise handling (not that it has anything to do with image manipulation) and it could be working kind of the same way for eg isOnline(id).then(showOn, showOff)

function isImage (file) {
    return new Promise(resolve => {
        let img = new Image
        img.src = URL.createObjectURL(file)
        img.onload = () => resolve(true)
        img.onerror = () => resolve(false)
    })
}


function showError () {
    // alert(not an image)
}

input.onchange = evt => {
    isImage(evt.target.files[0]).then(isImage => {
        isImage ? convert() : showError()
    }, err => {
        // code execution failed
        // couldn't tell if it's a image or not
        // 
        // alert(unknown error)
        // URL.createObjectURL is not a function
    })
}

vs

function isImage (file) {
    return new Promise((resolve, reject) => {
        let img = new Image
        img.src = URL.createObjectURL(file)
        img.onload = () => resolve(true)
        img.onerror = () => reject(new Error('Not a image file'))
    })
}

function showError (error) {
    // alert(instanceof error)
}

input.onchange = evt => {
    // This becomse a bit more nicer to read and write
    isImage(evt.target.files[0]).then(convert, showError)
}

I feel like the second method is a nicer way to deal with it. But I misuse use the way of how promises should deal with errors

I feel like isSomething() should resolve to a boolean true/false and reject if there is any code execution error. Like unable to make GET request

What do you think?

3

Yes, it is surprising and/or misleading for isImage to reject rather than resolve to false when the filepath successfully resolves to a non-image. But the the "client code" is indeed shorter and simpler in the second example.

What this tells me is that your example "client code" wants a different API from the one implied by the name isImage. I would suggest a different promise-returning method called getImage which blindly assumes the filepath leads to an image (and thus its promise should reject on non-images).

0

Since there are only two states and you typically want to follow completely different code paths upon success and error, it seems more useful and clean to reject() if the image does not load. You will notice your .then() handler seems simpler too because the success and error cases are already split out for you.

Much of deciding what should be a return value on fulfillment vs. a reject error is entirely up to you.

My main razor for deciding is "what will the calling code usually code?". If the calling code will usually want to continue processing other sequential async operations, the resolve(xxx) makes sense because that allows a .then() chain to continue going. If the outcome is unlikely to continue .then() processing because of the error, then reject(yyyy) seems more useful and appropriate.

If it's the isImage() names that is bothering you and making you think both cases should resolve to boolean, then you can change it to something like validateImage() or checkImage() or verifyImage().

Getting a load error is typically a significant error in the grand scheme of things. For example, a 404 from an ajax call would pretty much always be considered a failure. Why not something similar for an image that won't load.

0

it is surprising and/or misleading for isImage to reject rather than resolve to false when the filepath successfully resolves to a non-image

I agree, My common sense says it should only reject if there was some error
Those sticking to resolving true or false (method 1)
I also had in mind how it should work if it used yield/await. (wouldn't want to wrap it in a try/catch)

So I'm doing Higher-order function instead to keep it slightly more readable

// function that returns a function and calls a function
function yepNope(yep, no){
   return trutly => trutly ? yep() : no()
}

isImage(file).then( yepNope(convert, wrongFormat), execError )
isOnline(userId).then( yepNope(showOn, showOff), networkError )

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.