refactor this javascript code

Question

I have two click-events, that are nearly similar, but not quite. I am wondering how to refactor them best:

  $('.remove_fields.dynamic').live('click', function(e) {
    var $this = $(this);
    var after_removal_trigger_node = $this.closest(".nested-fields").parent();
    trigger_removal_callback($this);
    e.preventDefault();
    $this.closest(".nested-fields").remove();
    trigger_after_removal_callback(after_removal_trigger_node);
  });

  $('.remove_fields.existing').live('click', function(e) {
    var $this = $(this);
    var after_removal_trigger_node = $this.closest(".nested-fields").parent();
    trigger_removal_callback($this);
    e.preventDefault();
    $this.prev("input[type=hidden]").val("1");
    $this.closest(".nested-fields").hide();
    trigger_after_removal_callback(after_removal_trigger_node);
  });

As you can tell there is a fair bit of overlap. I am wondering what the best/nicest way would be to refactor this code.

Step 1: Don't use .live(). It's been deprecated.

user1106925
– user1106925

2012-04-06 23:10:27 +00:00
Commented Apr 6, 2012 at 23:10 — user1106925
– user1106925, Commented Apr 6, 2012 at 23:10

Elliot Bonneville · Accepted Answer · 2012-04-06 23:18:22Z

4

Do a class check in the .remove_fields click function.

$('.remove_fields').click(function(e) {
    var $this = $(this);
    var after_removal_trigger_node = $this.closest(".nested-fields").parent();
    trigger_removal_callback($this);
    e.preventDefault();
    if($this.hasClass("dynamic") {
        $this.closest(".nested-fields").remove();
    } else if($this.hasClass("existing")) {
        $this.prev("input[type=hidden]").val("1");
        $this.closest(".nested-fields").hide();
    }
    trigger_after_removal_callback(after_removal_trigger_node);
});

edited Apr 6, 2012 at 23:18

answered Apr 6, 2012 at 22:58

Elliot Bonneville

53.6k23 gold badges101 silver badges125 bronze badges

Sign up to request clarification or add additional context in comments.

1 Comment

Joseph Over a year ago

trigger_after_removal_callback is common in both cases, you can factor it to the bottom.

Niet the Dark Absol · Accepted Answer · 2012-04-06 22:57:19Z

3

Combine the selectors into $('.remove_fields.dynamic, .remove_fields.existing').

Then, test if $this has class existing. If so, run $this.prev("input[type=hidden]").val("1");.

Done.

answered Apr 6, 2012 at 22:57

Niet the Dark Absol

326k86 gold badges480 silver badges604 bronze badges

Comments

Joseph · Accepted Answer · 2012-04-07 07:25:16Z

1

taking cleaning a bit more:

$('.remove_fields').click(function(e) {
    e.preventDefault();

    var $this = $(this);
    var $nestedFields = $this.closest(".nested-fields");

    trigger_removal_callback($this);

    if($this.hasClass("dynamic") {
        $nestedFields.remove();
    } else if($this.hasClass("existing")) {
        $this.prev("input[type=hidden]").val("1");
        $nestedFields.hide();
    }

    trigger_after_removal_callback($nestedFields.parent());

});

edited Apr 7, 2012 at 7:25

answered Apr 6, 2012 at 23:25

Joseph

120k31 gold badges186 silver badges238 bronze badges

Collectives™ on Stack Overflow

refactor this javascript code

3 Answers 3

1 Comment

Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

1 Comment

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related