ansaurus

Question

I have alot of "IF/ELSE" statements, should I write this differently?

Answer 1

+8 A:

I am assuming this is JavaScript generated by PHP. To keep a maintainable PHP file, I would do the following:

<script type="text/javascript">
 function element_hide(id) { document.getElementById(id).style.display = "none"}
 function element_show(id) { document.getElementById(id).style.display = "block"}
</script>

<?php

$categories = array(

   // Insert all categories here. If a new property comes along
   // (e.g. "active/inactive") we can easily add it.
   array("label" => "Smycken & Klockor", "id" => "nav_sub_juveler"),
   array("label" => "Vuxen-plagg", "id" => "nav_sub_vuxen_plagg")
   ...

);

foreach ($categories as $category)
 {
   echo "if (category == '".$category["label"]."') ".
        "element_show('".$category["id"]."');".
        " else element_hide('".$category["id"]."');\n";
 }
?>

This does not care so much about the output, but that can easily be prettified with a few \ns and whitespace when echoing the condition.

Pekka 2010-02-17 15:24:20

Why should you write Javascript by PHP when you can do the same in Javascript?

Harmen 2010-02-17 15:29:25

@Harmen I am assuming he is getting his data from somewhere (e.g. a PHP database). He didn't say, so I am guessing at this point because he added the `php` tag. If it's JavaScript only, your solution is no doubt the most elegant.

Pekka 2010-02-17 15:35:59

Answer 2

A:

I would say your use of ternary statements will probably make that the smallest block of code (and to me the cleanest) that you are going to get. The only suggestion I could make without knowing something about your purposes here would be to put the relevant items into an array and then foreach through the array and have a single ternary statement using vars from the looping array. This will make your code 'look' cleaner with the same net effect.

Plus it will speed up changes you make so you make it once instead of 100 times.

angryCodeMonkey 2010-02-17 15:25:35

Answer 3

+2 A:

I would use a dictionary (associative array) and have a simple loop to iterate over.

jldupont 2010-02-17 15:25:37

Answer 4

+2 A:

It could be done using an object which can be seen as an associative array:

    var categories = {
        'Ljud & Bild': 'nav_sub_ljud_bild', 
        'Datorer': 'nav_sub_datorer'
    };
    var active = 'Datorer';

    for(name in categories){
        var display = (name == active) ? 'block' : 'none';
        document.getElementById(categories[name]).style.display = display;
    }

Harmen 2010-02-17 15:27:20

Technically it's an object, not an associative array.

Gordon 2010-02-17 15:30:25

@Gordon: You're right. Now I wonder if it's possible to define an associative array on one line like you can define an object...

Harmen 2010-02-17 15:33:01

Not that I know of, but I wouldn't bother about it as long as the solution is working.

Gordon 2010-02-17 15:41:47

+1 snap :-) You don't get an associative array type in JS, though an Object can do in a pinch as long as you have String keys and no need to store names that clash with `Object` methods like `toString`. You can build a real, any-type associative array out of two Arrays, but performance isn't nice.

bobince 2010-02-17 15:44:31

Answer 5

+3 A:

var category_ids= {
    'Ljud & Bild': 'nav_sub_ljud_bild',
    'Datorer': 'nav_sub_datorer',
    // ...and many more...
};

for (var c in category_ids)
    byId(category_ids[c]).style.display= category===c? 'block' : 'none';

It would be simpler still if you didn't have to have the static category_ids lookup. Is the category id automatically generated from the text, eg. by lowercasing and replacing spans of punctuation with _? If so you could save yourself a bunch of typing there.

Or if we are talking about picking a category from a <select>, you'd traditionally have an id in the value of the option, which could be mapped directly to the matching nav.

bobince 2010-02-17 15:29:07

Out of curiosity, may object members contain spaces and ampersands?

Gordon 2010-02-17 15:42:29

Yes, they can contain any character, though obviously only the ones that are valid JavaScript identifiers can be accessed using the object-dot-name syntax. The only problem member names are the empty string and ones that clash with `Object.prototype` member names, which in IE aren't enumerated by `for...in` due to JScript being buggy.

bobince 2010-02-17 15:49:52

Thanks, I ran it through JSLint as well and it gave no warnings either.

Gordon 2010-02-17 16:28:43

Answer 6

A:

Here is one of many ways to go-

You could collect all of the elements with display switches in a group- for example, give them all the same class name, or make them the only tags of some type of some parent 'categories'.

var G= byId('categories').getElementsByTagName('li');

And find a quicker way to resolve the element you want displayed-

var cat= category.toLowerCase().split(/[/*&\-]+/);
catname= (function(){
 switch(cat[0]){
  case 'Böcker': case 'Hobby': return cat[1];
  case 'Smycken ': return 'juveler';
  case 'Leksaker': return 'leksaker_barn';
  case 'Ljud': case 'Musik': return cat.join('_');
  default: return cat[0];
 }
})();

catnamed= byId('nav_sub_'+catname);
var G= byId('categories').getElementsByTagName('li')
for(var i=0, L=G.length;i<L;i++){
 who=G[i];
 // if(who.className=='catToggle'{
 if(who== catnamed) who.style.display= 'block';
 else if(who.style.display!= 'none') who.style.display= 'none';
}

kennebec 2010-02-17 16:17:08

ansaurus

tags:

views:

answers:

I have alot of "IF/ELSE" statements, should I write this differently?

related questions