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.

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
add comment

1 Answer

up vote 2 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
add comment

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.