What's "string math" and why is it bad?

I was recently berated by a fellow developer for using "string math" in an app I wrote. I'm pretty new to the whole development thing, with no formal training, and I haven't heard of this issue. What is it?

Code in question:

$('.submit-input').click( function() {
    var valid = true;
    $('input, select, r开发者_Go百科adio').removeClass('error');
    $('.error-message').hide();

    $('.validate').each( function() {
        if($(this).val() == $(this).attr('default')){
            valid = false;
            $(this).addClass('error');
        }
    });

    if(!$('select[name="contact"] option:selected').val() != ''){
        $('select[name="contact"]').addClass('error');
        valid = false;
    }

    if(!$('input[name="ampm"]:checked').length){
        $('input[name="ampm"]').addClass('error');          
        valid = false;
    }

    if(!valid){
        $('.error-message').css('display','block');
        return false;
    } else {

        var services_selected = 'Services Selected: ';
        services_selected += $('.l3').text() + ', ' + $('.l4').text() + ', ' + $('.l5').text() + '; ' + $('.l6').text();
        var prices = 'Prices: ';
        prices += $('.l7').text() + ', ' + $('.l8').text() + ', ' + $('.l9').text() + ', ' + $('.l10').text();
        var name = 'Name: ';
        name += $('input[name="name"]').val();  
        var phone = 'Phone: ' 
        phone += $('input[name="phone"]').val();
        var time = 'Preferred contact time: ';
        time += $('select[name="contact"] option:selected').val() + $('input[name="ampm"]:checked').val();

        $.ajax({
            url: 'php/mailer.php',
            data: 'services_selected=' + services_selected +'&prices=' + prices + '&name=' + name + '&phone=' + phone + '&time=' + time,
            type: "POST",
            success: function() {
                $('#email_form_box .container').children().fadeOut(500, function() {
                    $('#email_form_box .container').html('<div style="margin:20px auto;text-align:center;width:200px;">yada yada yada<br /><span class="close">Close</span></div>');
                });
            }
        });
    }

});

Edit: The gist I'm getting here is that this isn't a standard development colloquialism, and I should probably talk to the guy who gave me guff in the first place. So I'll do that. Thanks guys. I'll be back with an answer, or to check off whoever knew already.

---

In most Javascript browser implementations, concatenating strings is slow due to excessive copying. See JavaScript: String Concatenation slow performance? Array.join('')?

Preferred method is to use an array and join:

var pieces = ["You purchased "];
pieces.push(num, " widgets.");
el.innerHTML = pieces.join('');

Added more:

I think you may have a lurking bug in your code: you don't appear to be escaping your data values. If any of them include an ampersand, you'd be in trouble. Use escape() for all of your data values.

ps. And this is a real bug that the other developer missed. The string math issue is a performance / maintainability issue.

Added:

I rewrote your email composition section (quickly). I think it's cleaner (and will be slightly faster) when using a piece array.

....
} else {

var d = []; // the post_data pieces table

d.push ('services_selected='); // Start the services_selected value
d.push ('Services Selected: ');
d.push ($('.l3').text(), ', ', $('.l4').text(), ', ', $('.l5').text(),
        '; ', $('.l6').text());

d.push ('&prices='); // Start the prices value
d.push ('Prices: ');
d.push ($('.l7').text(), ', ', $('.l8').text(), ', ', $('.l9').text(),
        ', ', $('.l10').text());

d.push ('&name='); // Start the name value
d.push ('Name: ', $('input[name="name"]').val());

d.push ('&phone='); // Start the phone value
d.push ('Phone: ', $('input[name="phone"]').val());

d.push ('&time='); // Start the timevalue
d.push ('Preferred contact time: ',
        $('select[name="contact"] option:selected').val(),
        $('input[name="ampm"]:checked').val());

    $.ajax({
        url: 'php/mailer.php',
        data: d.join(''),
        type: "POST",
        success: function() {
            $('#email_form_box .container').children().fadeOut(500, function() {
                $('#email_form_box .container').html('<div style="margin:20px auto;text-align:center;width:200px;">yada yada yada<br /><span class="close">Close</span></div>');
            });
        }
    });
}

---

Okay, so here's the answer he told me:

I should have said inline string concatenation/parsing, which is a potential injection vulnerability and a sign of sloppy code or bypassing the framework.

Which doesn't exactly fit the other answers we have here. I'm going to give the check to the answer with the most upvotes, as it's probably the most useful, but just wanted to inform.

---

Are you perhaps storing/manipulating numerical data using strings? That's rarely a good idea.

---

Since you're new to development the best thing to do would be to discuss with this developer what "String math" is, how you can identify when you're doing it again, and how to avoid it. Then, come back here and answer your own question so we can see what this "String math" really is - from your fellow dev's perspective.

---

Edit: Ok, my bad, you don't use + for concatenation. Edited below:

Edit2: Ok, it is JavaScript, back to + :P

---

I think he's probably referring to something like:

$my_html = "<p>" + someVar + "<em>" + somethingImportant + "</em></p>";

i.e. using . for concatenation.

---

Since you retagged your question with javascript, then your colleague might mean bugs in your code that lead to questions like Strange javascript addition problem

Basically "1" + 1 evaluates to 11 in javascript, while 1 + 1 evaluates to 2. Now replace the first argument of + with a variable and you can get some unexpected behaviour.

---

it's probably lines like this that your co-worker has issue with. in theory this is perfectly correct code, but it's pretty much impossible to read.

services_selected += $('.l3').text() + ', ' + $('.l4').text() + ', ' + $('.l5').text() + '; ' + $('.l6').text();

have a look at the function and discussion here: http://frogsbrain.wordpress.com/2007/04/28/javascript-stringformat-method/

you can easily add this function to your JS and then you can change this horrible line of code to something like:

services_selected = '{0} , {1}, {2}, {3}; {4}'.format($('.l3').text(), $('.l4').text(), $('.l5').text(), $('.l6').text());

---

To extend Skilldrick's answer:

There's nothing wrong using "+" to concat strings (depending on your language) till one of your variables isn't a string:

echo 0 + ": hi!<br />";
echo 0 .. ": hi!<br />";

The first line might output "0" (as it tries to convert the string to a number). The second line works as expected writing "0: hi!
".

---

Here's what I think of when I hear "String math." I'd yell at him, too.

public String StringAdd (String str1, String str2){
   int int1, int2;
   switch (str1){
      case "Zero":
      int1 = 0;
      break;
      case "One":
      int1 = 1;
      break;
      //...etc...
      default:
      throw new BadNumberSpellingException("You spelled a number wrong.");
   }
   switch (str2){
      case "Zero":
      int2 = 0;
      break;
      //...etc...
   }

   int result = int1 + int2;
   switch (result){
      case 0:
         return "Zero";
      case 1:
         return "One";
      case 2:
         return "Two";
      //etc....
   }
}