3

I'm really sorry if this come across as stupid but i've searched everywhere but i didn't find any tip on how to go about it. i have this variable 

 var id = $('#sometextfield').val();

This text field value is dynamically generated eg.

 <input type="text" name="id" id="id" value="<?php echo $_get[something];?>" />)

The problem here is that i have an if condition saying

if(id == 100 || id ==120) {
   // i have these variables
   var mulitiplier = 0.005
   var price = (some alog) * mulitiplier;

   // do some very long piece of code
}
else if (id == 200 || id == 220) {
   // Then i have these variables
   var mulitiplier = 0.090;
   var price = (some alog) * mulitiplier;
   // Do the same very long piece of code(its practically the same thing as 
   // the first if statement and the only change is the ariable multiplier)        
}

This works and all but is there any way to not repeat the same thing. I don't like the look of it atm. Many thanks in advance..

Improve this question
4
  • It would be great if you could separate text from code... or least make them comments in the code. Commented Oct 17, 2011 at 21:23
  • Really sorry, i thought i could explain it well that way, guess i was wrong :) Commented Oct 17, 2011 at 21:26
  • It's fine with comments, but this mix is just hard to read... at least for me ;) Commented Oct 17, 2011 at 21:27
  • Wow that was fast,Thank you all that answered, i'll try them and get back to you guys.. Commented Oct 17, 2011 at 21:37

4 Answers 4

Reset to default
3 
function long_piece(multiplier){
    ... long piece of code....
}
var MultMap = {100:0.005, 120:0.005, 200:0.009, 220:0.009}

long_piece(MultMap[id])
Improve this answer
Sign up to request clarification or add additional context in comments.

Comments

3

Just abstract out the very long piece of code to a function which takes the multiplier and price values as parameters. For example

var theCode = function (multiplier, price) {
  do some very long piece of code
};

if(id == 100 || id == 120) {
  var mulitiplier = 0.005
  var price = (some alog) * mulitiplier;
  theCode(multiplier, price);
} else if (id == 200 || id == 220) {
  var mulitiplier = 0.090;
  var price = (some alog) * mulitiplier;
  theCode(multpilier, price);
}

Note: You should consider using parseInt in the initialization of id and === for comparing values instead of == for this type of code.

Improve this answer

2 Comments

Note that if you switch to ===, you'll probably need to change 100 to "100" as well.
@ElliotNelson or better yet create id with parseInt so the comparison is between numbers
1

Easy, just put the "very long piece of code" in a custom function, accepting 1 parameter, that is, your multiplier.

Improve this answer

Comments

0

Wrapping the long piece of code in a function and rewriting your code like this would be more efficient (and more readable), I suppose:

var id100120 = id === 100 || id === 120,
    id200220 = id === 200 || id === 220,
    multiplier = id100120 ? 0.005 : id200220 ? 0.090 : null,
    price = (/*some alog*/) * mulitiplier,
    longPieceOfCode = function(){ /* your code here ... */ };

if (id100120 || id200220) {
  longPieceOfCode();
}

if id is a string value, you could also use something like:

var id100120 = '[100][120]'.indexOf('['+id+']')>-1,
    id200220 = '[200][220]'.indexOf('['+id+']')>-1

or (with exception of ie < 9 I think)

id = Number(id);
var id100120 = [100,120].indexOf(id) > -1,
    id200220 = [200,220].indexOf(id) > -1
Improve this answer

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.