Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I feel like this bit of code could be condensed or made better in some way. It may in fact not be, but figured I'd get some people to have a look. I have multiple pages on my site and in certain pages I'm setting a JavaScript variable:

var header_check = "user-profile";

Some pages I'm not setting that variable. In another JavaScript file that gets loaded on the page I check if that variable exists and perform various actions if it does.

Is this the best way to check if the variable exists? Also, is this the best way to see if user-profile is set?

var header_cookie = typeof header_check !== 'undefined' ? 'user-profile' : 'admin-profile';
var cookie_check = header_cookie == 'user-profile' ? true : false;

var city_profile = cookie_check ? 'userCity' : 'city';
var state_profile = cookie_check ? 'userState' : 'state';
share|improve this question

1 Answer 1

up vote 11 down vote accepted

First off, your code really breaks down to this logic and I find it helpful to write it out the longer version to full understand the logic flow before trying to shorten it:

var header_cookie, cookie_check, city_profile, state_profile;
if (typeof header_check !== "undefined") {
    header_cookie = 'user_profile';
    cookie_check = true;
    city_profile = 'userCity';
    state_profile = 'userState';
} else {
    header_cookie = 'admin_profile';
    cookie_check = false;
    city_profile = 'city';
    state_profile = 'state;
}

FYI, I also find this a LOT easier to follow what's actually happening than the code you have. It also saves several comparisons on the cookie_check value.


There are some ways to shorten this, but it's not entirely clear that any are "better" where the definition of better includes readability by someone who has never seen this code before, but you can decide what you think of that issue for the alternatives:

Since you really only have two states, you could predefine each state and then just pick which one to use and access the properties off a single state object:

var userState = {
    header_cookie: 'user_profile', city_profile: 'userCity', state_profile: 'userState';
};
var adminState = {
    header_cookie: 'admin_profile', city_profile: 'city', state_profile: 'state';
};
var state = typeof header_check !== "undefined" ? userState: adminState;

Done this way, you'd access state.header_cookie, state.city_profile and state.state_profile rather than your standalone variables.


Or, if you wanted to keep the individual variables, you could do this:

var states = {
    header_cookie: ['user_profile', 'admin_profile'], 
    city_profile: ['userCity', 'city'],
    state_profile: 'userState', 'state'];
};

var stateIndex = typeof header_check !== "undefined" ? 0 : 1;
var header_cookie = states.header_cookie[stateIndex];
var city_profile = states.city_profile[stateIndex];
var state_profile = state.state_profile[stateIndex];
share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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