5
\$\begingroup\$

I need to create an HTML unordered list (to display with jstree) to display the hierarchy of users, along with a list underneath the user's "subusers" of their comments. Each user could have many users underneath him, and each of those users could have the same, so I used a recursive filter to accomplish this. However, this has lead to code that feels very repetitive, besides small differences, and it seems to me that surely something could be reduced. I have already identified some repeated code (extracted to _output_comments below), but I would hope I could do more somehow.

Note before the code: User has a foreign key to itself, with subusers as the related_name, and Comment has a foreign key to User.

from django import template

register = template.Library()


@register.filter
def unordered_user_list(user):
    """
    Takes a user and returns an HTML unordered list (without surrounding <ul> tags) of 
    users and comments under it.
    """
    html = ""
    if len(user.subusers.all()) > 0:
        html += "<li id='user-" + str(user.id) + "'>" + user.name
        if len(user.comment_set.all()) > 0:
            html += "<ul>"
            for subuser in user.subusers.all():
                html += unordered_user_list(subuser)
            html += "<li>Comments<ul>"
            html += _output_comments(user.comment_set.all())
        else:
            for subuser in user.subusers.all():
                html += "<ul>" + unordered_user_list(subuser) + "</ul>"
        html += "</li>"
    elif len(user.comment_set.all()) > 0:
        html += "<li id='user-" + str(user.id) + "'>" + user.name + "<ul><li>Comments<ul>"
        html += _output_comments(user.comment_set.all())
    else:
        html += "<li id='user-" + str(user.id) + "'>" + user.name + "</li>"
    return html

def _output_comments(comment_list):
    output = ""
    for comment in comment_list:
        output += "<li id='comment-" + str(comment.id) + "'>" + comment.name + "</li>"
    output += "</ul></li></ul>"
    return output

Finally, I would appreciate any other pointers, since this is my first filter. It seems straightforward enough, but for all I know this could be a completely inappropriate use of a filter.

\$\endgroup\$
2
\$\begingroup\$

First of all, you can move the first <li> out of the first if condition. That allows you to totally get rid of the last else:

html = "<li id='user-" + str(user.id) + "'>" + user.name
if len(user.subusers.all()) > 0:
    # ...
elif len(user.comment_set.all()) > 0:
    html += "<ul><li>Comments<ul>"
    # ...
    # don't add the last </li> here
html += "</li>" # add it there instead
return html

Then, _output_comments should handle the whole comment stuff, including the heading:

def _output_comments(comment_list):
    output = "<li>Comments<ul>"
    for comment in comment_list:
        output += "<li id='comment-" + str(comment.id) + "'>" + comment.name + "</li>"
    output += "</ul></li></ul>"
    return output

I think that the way you add <li></li> and <ul></ul> tags does not reflect the structure of the page. You should improve this:

def _output_comments(comment_list):
    output = ("<li>" +
              "Comments" +
              "<ul>")

    for comment in comment_list:
        output += "<li id='comment-" + str(comment.id) + "'>" + comment.name + "</li>"

    output += ("</ul>" +
               "</li>" +
               "</ul>")
    return output

Overall, you should try to write Python code whose structures more or less reflects the structure of the HTML code you are writing. That will avoid tricky mistakes and tag mismatches. When I read your code as is, I have trouble knowing whether there are tags mismatches or not.

\$\endgroup\$
  • \$\begingroup\$ Thank you! About the amount of <ul> and <li> tags, there are definitely a ton, but I'm pretty sure they're all needed. :/ jsTree's HTML page shows how it wants the list to be organized, and it is coming up with the structure I'm looking for. I'll definitely be pleased if it works with less, though. :) \$\endgroup\$ – ZAD-Man Aug 1 '14 at 21:33
  • \$\begingroup\$ @ZAD-Man Ok, I will try to read your code again with that in mind :) \$\endgroup\$ – Morwenn Aug 1 '14 at 21:35
  • \$\begingroup\$ @ZAD-Man I have probably made some mistakes since I didn't check the code properly, but wouldn't something like that be enough? \$\endgroup\$ – Morwenn Aug 1 '14 at 21:45
  • \$\begingroup\$ Oh, nice, it certainly looks better, whatever the case. :) I'll try it out and let you know how it goes (though that unfortunately might not be until Monday). \$\endgroup\$ – ZAD-Man Aug 1 '14 at 21:50
  • \$\begingroup\$ @ZAD-Man Let me know if it works. In which case I'll add it to the answer :) You will have to add some parenthesis to have it work. I somehow forgot that multiline expressions needed parenthesis to work. \$\endgroup\$ – Morwenn Aug 1 '14 at 21:59

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.