1
\$\begingroup\$

I used a glyphicon as a link to delete a comment:

<a class="delCommentLink" href="{% url 'article:comment_delete' comment.id %}">
    <span id="{{ comment.id }}" class="glyphicon glyphicon-trash" aria-hidden="true">delete</span>
</a>

I send the request using Ajax.

  1. retrieve the comment_url
  2. get the grand-grand-parent element to hide
  3. send request to views.py and delete the target comment
$(document).ready(function () {
    $("body").on("click",".delCommentLink",function (e) {
        e.preventDefault();
        var comment_url = $(e.target).parent().attr("href");
        var $commentEle = $(e.target).closest(".comment");
        if (window.confirm("Delete this comment?")) {
            $.ajax({
                type: "get",
                url: comment_url,
                success: function (data) {
                    var ret = JSON.parse(data);
                    if (ret['status'] == 1) {
                        $commentEle.hide();
                    } else {
                        alert(ret['msg']);
                };
            },//success
        });//ajax  
        } else {
            e.preventDefault()
        }
    });//click event
})

Click a link to delete it. My code seems like it has too many lines for such a routine task.

How can I complete it elegantly?

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

I only see two things to "reduce", regarding the line amount.

  1. The if statement in the success callback could be replaced by a ternary operator.
  2. The last e.preventDefault() is redundant, you can squarely remove the whole else part.

    $(document).ready(function () {
      $("body").on("click",".delCommentLink",function (e) {
        e.preventDefault();
        var comment_url = $(e.target).parent().attr("href");
        var $commentEle = $(e.target).closest(".comment");
        if (window.confirm("Delete this comment?")) {
          $.ajax({
            type: "get",
            url: comment_url,
            success: function (data) {
              var ret = JSON.parse(data);
              (ret['status'] == 1) ? $commentEle.hide() : alert(ret['msg']);
            },//success
          });//ajax  
        }
      });//click event
    });
    

That is 6 lines less...

\$\endgroup\$
1
\$\begingroup\$

Louys has already mentioned a couple great techniques. There are a few others:

  • Use the shortcut method $.get() passing the URL as the first parameter and the success callback as the second parameter
  • Do elements with the class "delCommentLink" get added to the page after the page is loaded or do they all exist when the page is loaded? If the latter is the case then the alias $.click() could be used instead of the $.on() method.

Additionally:

  • You didn't mention which version of jQuery is used/supported, but presuming it is 3.0 or higher (correct me if that is incorrect) the form $(document).ready(handler) is deprecated1 and the only supported form is $(handler) so the code could be updated like below.

  • Instead of using bracket notation for accessing a property of the return value:

     ret['status']
    

    It can be done using dot notation:

    ret.status
    

Simpler code:

$(function () { // DOM ready callback
    $("body").on("click", ".delCommentLink", function (e) {
        e.preventDefault();
        var comment_url = $(e.target).parent().attr("href");
        var $commentEle = $(e.target).closest(".comment");
        if (window.confirm("Delete this comment?")) {
            $.get(comment_url, function (data) {
                var ret = JSON.parse(data);
                ret.status == 1 ? $commentEle.hide() : alert(ret['msg']);
            });//ajax  
        }
    });//click event
}); // DOM ready callback
\$\endgroup\$

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.