3
\$\begingroup\$

I would like to know an alternative, more elegant way to write the following method. I am especially not enthusiastic of the nested if statement.

hasUserSavedCredentials: function () {
            var userName = Storage.get(CONFIG.app.storageUserName),
                password = Storage.get(CONFIG.app.storagePassword),
                result = false;
            if (userName !== null && password !== null) {
                if (userName.length > 0 && password.length > 0) {
                    result = true;
                    // save in state for faster retrivial
                    State.xx.isSaved = result;
                    State.xx.userName = userName;
                    State.xx.password = password;
                } else {
                    result = false;
                    State.xx.isSaved = false;
                }
            }
            return result;
        },
\$\endgroup\$
3
\$\begingroup\$

You can remove result = false;, as result is already false.

And to get rid of the nested statements, you could use early returns:

if (userName == null && password == null) {
    return false;
}

if (userName.length <= 0 && password.length <= 0) {
    State.xx.isSaved = false;
    return false;
}
State.xx.isSaved = true;
State.xx.userName = userName;
State.xx.password = password;
return true;

Is it correct that when username is null, isSaved = false should not be set? Because if not, then you could just combine the two if statements, either in my example, or in yours.

\$\endgroup\$
1
\$\begingroup\$

Seems kind of weird that you need to check null and '' from the storage value. You would think that if the storage kept a value that it would be valid. If you can't change how that works the following code could avoid an extra if check and be a little more terse:

hasUserSavedCredentials: function () {
            var userName = Storage.get(CONFIG.app.storageUserName) || '',
                password = Storage.get(CONFIG.app.storagePassword) || '',
                hasSavedCredentials = userName.length > 0 && password.length > 0

                if (hasSavedCredentials) {
                    State.xx.userName = userName;
                    State.xx.password = password;
                }
                State.xx.isSaved = hasSavedCredentials;

            return hasSavedCredentials;
        }

I am assuming storage will either return null or a string if it is found, since it null is a falsy the || will default it to ''. The extra if can be avoided because username or password will always be a string. Since there isn't an extra if check the result can be stored and used without the need for an else. Also naming result to a more descriptive value helps.

\$\endgroup\$

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.