current community

your communities

more stack exchange communities

Stack Exchange Inbox Reputation and Badges
tour help
Sign up ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 1 down vote favorite

Below is a chunk of some switch code used to create an array of objects which should be visible. Previously I combined case -1 and case 0 to reduce redundancy, but I am wondering if this code can be reduced in size and/or improved in readability.

var visibles = [];
switch (hrdid) {
    case -1:
    case 0:
        if (row("tcd")) {
            visibles = ['lc', 'ts2', 'rtc', 'ctb'];
            if (row("cbr")) visibles.push('ccb2');
        } else {
            visibles = ['lc1', 'tc', 'rc', 'ico', 'ccb'];
        }
        if (hrdid == 0) visibles.push('bcf');
        if (hrdid == -1) {
            visibles.push('bcfa');
            visibles.push('cpma');
        }
        break;
    //case 1, etc...
}
share|improve this question
    
do the cases not shown also have code in common with these? My first thought was that the first if/else chunk should just be moved out of the switch altogether, but then I saw your //case 1 comment. –  nickles80 Feb 15 '13 at 1:46
1  
Is there a rhyme of reason why 'xyz' should be visible for hrdid=t? If it is arbitrary, you should put this in a data structure. Like so: {'-1':['lc', 'ts2', 'rtc', 'ctb', 'bcfa', 'cpma'], ...} Then you can get the result you want visibles=visiblesByHrdid[String(hrdid)] where visiblesByHrdid holds the data. –  abuzittin gillifirca Feb 15 '13 at 8:12
    
@nickles80: The cases not shown do not have code in common with these two cases. –  Brian Feb 15 '13 at 13:59

1 Answer 1

up vote 1 down vote accepted 
var visibles = [];
function mainWork () {
        if (row ("tcd")) {
            visibles = ['lc', 'ts2', 'rtc', 'ctb'];
            if (row("cbr")) visibles.push ('ccb2');
        } else {
            visibles = ['lc1', 'tc', 'rc', 'ico', 'ccb'];
        }
}
switch (hrdid) {
    case -1:
       mainWork ();
       visibles.push ('bcfa');
       visibles.push ('cpma');
       break;
    case 0:
        mainWork ();
        visibles.push ('bcf'); 
        break;
    //case 1, etc...
}
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.