Can you please tell me if this code has any problems with it?

JavaScript
1

Can you please tell me if this code has any problems with it?

$(document).ready(function(){
	 $("select.ui-select").selectWidget({
    change: function(changes) {

      alert($("select[name='channel']").val());

	  $.ajax({
	  type: "POST",
	  url: "/ajax.php",
	  data: "channel_id="+channel_id,
	  dataType: 'json',
	  statusCode: {
	  200: function(data) {
	  for(i = 0; i < data.length; i ++) {
	  $("select[name='sub_category']").append("<option value='"+data[i]["sub_channel_id"]+"'>"+data[i]["sub_channel_name"]+"</option>");
	  }
	  }
	  }
});

      return changes;
    },
    effect: "slide",
    keyControl: true,
    speed: 200,
    scrollHeight: 250
  });
});
2

It doesn’t appear to have any HTML to interact with.

1 Like
3

Thank you

1 Like
4

What’s wrong with that code?

  • $ is used for the ready part. Normally jQuery is used instead for compatibility, then $ within
  • Spaces are missing from around function parameter parenthesis
  • It’s missing “use strict” at the head of the function.
  • You’re mixing spaces and tabs
  • The indentation is all messed up, making the code much harder to understand
  • It’s better to use window.alert than just alert
  • Spaces should be on either side of operators, such as the + operator
  • The channel_id variable seems to be undefined
  • You’re mixing single and double quotes - pick one quoting style and be consistent with it
  • Numbers are being invalidly used for an object key
  • Do not use a for loop to iterate over arrays, use the built-in forEach methods instead
  • Do not use array indices when dot notation can be used instead
  • Do not write HTML code within JavaScript - it’s better to use new Option(text, value) instead
  • Callback hell! See http://callbackhell.com/ Or pyramid of doom

Fix up those things and there will be a lot less wrong with the code.

5 Likes
5

It is rather difficult to interpret the effectiveness of a piece of code when we can’t see what it’s interacting with, the context within which it is set, we’re told nothing about what you expect it to do, nor whether it works at all. It could be perfect, but right now, that would be the result of guesswork on our part.

6

Thanks for the replies

7

Here’s what the code looks like with most of the problems fixed up.

/*jslint browser */
/*global window, jQuery */
jQuery(document).ready(function ($) {
    "use strict";

    function addChannels(data) {
        data.forEach(function (item) {
            var option = new Option(item.sub_channel_name, item.sub_channel_id);
            $("select[name='sub_category']").append(option);
        });
    }

    function changeChannelHandler(changes) {
        window.alert($("select[name='channel']").val());
        $.ajax({
            type: "POST",
            url: "/ajax.php",
            data: "channel_id=" + channel_id,
            dataType: "json",
            statusCode: {
                "200": addChannels
            }
        });
        return changes;
    }

    $("select.ui-select").selectWidget({
        change: changeChannelHandler,
        effect: "slide",
        keyControl: true,
        speed: 200,
        scrollHeight: 250
    });
});

It still doesn’t do anything about the mysterious channel_id variable though.

1 Like
8

This topic was automatically closed 91 days after the last reply. New replies are no longer allowed.