3
\$\begingroup\$

I'm working on a simple little one page website for giving a presentation and I have some working code now, but I'm thinking there has to be a more elegant way of doing this. I would describe myself, at best, as a "dabbler" in JavaScript at this point; I'm much more comfortable with statically-typed languages and am doing this as a learning exercise.

Here's my JSFiddle showing what I am doing.

In my "real" site, the PageContent object is coming from an AJAX POST response (via an instance of node.js). I've included all of that code to make it easier to understand what is going on (any suggestions on how to restructure that server-side code would also be appreciated). If I have the time, I'll probably read these "points" from a config file or something else, but for now, a switch statement fits my needs.

The segment of this that makes me the most uneasy is this:

$('#pageContent').append('<li id="' + i + '">' + mp.main);
 if (mp.subPoints.length > 0){
 $('li#'+i).append('<ul>');
 $.each(mp.subPoints, function(index, sp){
 $('li#'+i+' ul').append('<li>' + sp + '</li>');
 });
 $('li#'+i).append('</ul>');
 }
 $('#pageContent').append('</li>');

I don't like how I am having to add id values to my list items so I can then append my subpoints to the correct parent. Is there a less hacky-feeling way to go about this?

In case it isn't obvious from the fiddle, the page should show a list of main points, and, if they exist, subpoints under each main point. I'm basically trying to mimic PowerPoint.

Any criticisms of this code would be gladly accepted; I know this probably isn't idiomatic JavaScript.

Jamal
35.2k13 gold badges134 silver badges238 bronze badges
asked Nov 7, 2013 at 14:55
\$\endgroup\$

2 Answers 2

3
\$\begingroup\$

This will depend on your content and how you prepare it, but I'd suggest a very generic solution that'll work for any level of nested points. Now, arbitrary nesting doesn't appear to be required in your case, but, hey, nice to have.

Suppose your JSON content is structured like so:

var points = [
 {title: "Point", children: [
 {title: "Point"},
 {title: "Point"},
 {title: "Point"},
 {title: "Point", children: [
 {title: "Point"},
 {title: "Point"},
 {title: "Point", children: [
 // more...?
 ]}
 ]}
 ]}
]

You'll note that you can just keep nesting the points indefinitely.

To render this as HTML, you can use a recursive function. Like this:

function buildList(parentElement, items) {
 var i, l, list, li;
 if( !items || !items.length ) { return; } // return here if there are no items to render
 list = $("<ul></ul>").appendTo(parentElement); // create a list element within the parent element
 for(i = 0, l = items.length ; i < l ; i++) {
 li = $("<li></li>").text(items[i].title); // make a list item element
 buildList(li, items[i].children); // add its subpoints
 list.append(li);
 }
}

And call it like so:

buildList($("#pageContent").empty(), points);

The point is that since the structure is recursive, it can nest to any depth, but the code is simpler.

Here's a jsfiddle. This is quite a different approach, and Daniel Cook's answer is probably more immediately applicable to you current code, but I thought it worth to point out.

You could also extend it to add the "1", "1.1", "1.2" (and so on) numbers to the titles.

answered Nov 7, 2013 at 17:54
\$\endgroup\$
1
  • \$\begingroup\$ I don't know why I didn't think of recursive functions, yes, that would work very nicely. \$\endgroup\$ Commented Nov 7, 2013 at 19:51
2
\$\begingroup\$

You do not need to add the id to the created li if you store it to a variable.

The code below shows the basic concept. Does it make you more comfortable?

$.each(current.contents, function(_, mp){
 var $li = $('<li>' + mp.main + '</li>'); 
 if (mp.subPoints.length){
 var $ul =$('<ul>')
 $.each(mp.subPoints, function(_, sp){
 $ul.append('<li>' + sp + '</li>');
 });
 $li.append($ul);
 }
 $pageContent.append($li);
});

Here's a fiddle using the different coding I'm sure this could be improved more. I'm also still learning.

answered Nov 7, 2013 at 15:54
\$\endgroup\$
3
  • \$\begingroup\$ That is a helpful suggestion, thanks. Is it a best practice to have the index variable named the same when doing nested each loops like you have there (_ as the variable name) if you aren't going to use it? \$\endgroup\$ Commented Nov 7, 2013 at 15:59
  • \$\begingroup\$ I've been given the impression that using _ for a variable you do not intend to use is considered common practice at least. I got this impression from this question with answers on SO. \$\endgroup\$ Commented Nov 7, 2013 at 16:22
  • \$\begingroup\$ To answer the other part of your question, it hardly matters what the name of the index variable are since they are not used. If they were used, javascript would use the correct ones. Javascript handles scoping of variables on the function level, so even with a name conflict the correct variable would be used. I didn't even think about the same name issue since the indexes aren't used. \$\endgroup\$ Commented Nov 7, 2013 at 16:28

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

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

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.