Discussion on: Nested Conditional Operators

[Marcel Cremer]

In my opinion the main problem in this particular case is not the tenary operator - it's the ugly code that results from the violation of the DRY principle at the "error throwing".

Let's break this down a bit:

• You have several "I validate a field"-things going on.
• each of them will reject the "overall"-Promise, when the validation is not valid.
• if every field validation is successfull, the Promise is resolved.

Why don't you just tell your program exactly this idea?

"I validate a field" becomes a function, that rejects a Promise on error, or resolves it if everything is fine:

// async can be Replaced with new Promise(bla)
validateField = async (condition, message) => {
    if(!condition)
        throw new Error(message);
}

Now we can set a condition and get a resolved / rejected Promise back - we just need to execute all of them (with Promise.all) and use the overall success as indicator, that all validation was successful:

const validateForm = ({ field1, field2, field3 }) => Promise.all([
    validateField(!!field1, 'Missing field1'),
    validateField(Array.isArray(field2), 'field 2 is not an Array'),
    validateField(!isValidType(field3), 'field3 is invalid')
]);

I think that this increases readability by several levels and tells us on the first glance, what it's doing. You might even add other fancy stuff in the "validateField"-Function, which applies to all validations, or you can use the "validateField"-part without calling the validateForm-Function.

I think if I found your piece of code, I would've refactored it like that and I can imagine, other places where "multiple" tenarys would be needed can be refactored like that as well.

What do you think about this attempt?

[Avalander]

Using Promise.all is a very interesting approach, I like it.

[Nested Software]

validateField seems reasonable, but I am not sure why it is asynchronous. All of the field information seems to be available as-is, so it looks as though validateField and validateInput could be simplified:

const main = () => {
    const obj = {
        field2: [1,2,3],
        field3: 'joe'
    }

    result = validateInput(obj)

    result
        .then(success=>console.log('validation was successful'))
        .catch(error=>console.log(`${error}`))
}

const validateInput = ({field1, field2, field3}) => {
    try {
        validateField(exists(field1), 'Missing field1')
        validateField(Array.isArray(field2), 'field 2 is not an Array')
        validateField(isValidType(field3), 'field3 is invalid')

        return Promise.resolve()
    } catch (error) {
        return Promise.reject(error)
    }
}

const validateField = (condition, message) => {
    if (!condition)
        throw new Error(message)
}

//implemented as a stub
const isValidType = field => true

//not sure if there needs to be a distinction between boolean
//fields that are set to false, fields that are set to null or empty string, 
//and fields that simply don't exist in the object
const exists = field => field != undefined

main() //Error: Missing field1

Also, I'm not sure why there are negations in !!field1 and !isValidType(field3).

[Marcel Cremer]

I just made it async, because I was too lazy to write new Promise bla (async always returns a Promise). Also I used Promise.all because I guessed that Avalander had some fancy async validation stuff going on.

!!field1 casts a value to a boolean (a bit hacky, I know 😉 Just wanted to point out, that someone should provide a boolean value).

!isValidType is taken from the original post - of course non-negated conditions should be preferred.

[Avalander]

Yeah, let's not forget that the code I posted is a simplification of the real problem, but we can ignore the asynchronous part, I just wanted to discuss the chained conditional operators.