current community

your communities

more stack exchange communities

Stack Exchange
tour help
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.
up vote 1 down vote favorite

What do you think of my code? I know it is a little rough around the edges but as of two days ago I had no idea how to use jQuery or JavaScript.

$(document).ready(function () {

var json = {

    "205100": {
        "success": true,
            "data": {
            "type": "game",
                "name": "MasterKin",
                "steam_appid": 9000,
                "required_age": 40,
                "is_free": false,
                "controller_support": "full",
                "dlc": [212894, 212893, 208575, 208570]
        }
    }

};

var game_name = [];
for (var key in json) {
    if (json.hasOwnProperty(key)) {
        var item = json[key];
        game_name.push({
            ItemName: item.data.name //Changing the .name to .dlc or .type will then display that result

        });
    }
}
console.log(game_name);
var tr;
for (var i = 0; i < game_name.length; i++) {
    tr = $('<tr/>');
    tr.append("<td>" + game_name[i].ItemName + "</td>");

    $('table').append(tr);
}
});
<script src="https://ajax.googleapis.com/ajax/libs/jquery/1.11.0/jquery.min.js"></script>
<table>
    <tr>
        <th>Name</th>

    </tr>
</table>

share|improve this question
    
Welcome (again) to Code Review! I'm glad that you were able to fix your code. I've modified your question to fit the task performed by your code. –  200_success♦ Nov 25 '14 at 9:39

1 Answer 1

up vote 1 down vote accepted

I don't know what the possible rest of your code looks like, so my opinion might be wrong. What I don't understand is why a lot of people use jQuery for the slightest bits nowadays. The display of the array could be done like this:

var table = document.getElementById("displayTable"); //give this ID to your table

for (var i = 0; i < game_name.length; i++) {
    var row = table.insertRow(i);
    var cell = row.insertCell(0);
    cell.innerHTML = game_name[i].ItemName;
}

Update:

If you insist on using jQuery, cache the selector in a variable and re-use the variable. Overusing selectors can result in poor performance and since you're using the selector inside a loop this can easily occur.

var table = $('#displayTable');

for (var i = 0; i < game_name.length; i++) {
    table.append("<tr><td>" + game_name[i].ItemName + "</td></tr>");
}

Furthermore, give useful names to variables. Names like tr or game_name are not meaningful, try tableRow and gameNames instead. I updated the JSFiddle working example.

share|improve this answer
    
Thanks for providing an example! I really didn't know I could parse JSON by just using javascript as most of the tutorials I have come by use jQuery to parse Json files.I guess I choose to use jQuery because I was also going to utilise jQuery Ajax capabilities –  user36278 Nov 25 '14 at 12:13
    
Yes, jQuery makes life easier but in some situations it can be overkill. You can also use AJAX without jQuery, altough I would recommend using it there. Here's an example of AJAX without jQuery. –  Abbas Nov 25 '14 at 13:34
    
I updated my answer with some more information. –  Abbas Nov 25 '14 at 13:55

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.