Replace radio/checkbox plugin

Question

I've just created my first plugin, but I think I've written too much bloated code. Could you point me to the right direction?

$.fn.replaceme = function() {
 function add_essentials(element) {
 var native_btn = element;
 native_type = native_btn.attr('type');
 native_btn.each(function() {
 var n_btn = $(this);
 n_btn.next('label').addClass('label-replacement');
 n_btn.wrap('<div class="' + native_type + ' input-container"></div>');
 n_btn.parent('.input-container').append('<span class="' + native_type + ' input-replacement"></div>');
 });
 }
 function replace_default_action(element) {
 var input = element;
 group = input.attr('name');
 $('input[name="' + group + '"]').each(function() {
 var input_select = $(this);
 if (input_select.is(':checked')) {
 input_select.siblings('.input-replacement').addClass('selected');
 } else {
 input_select.siblings('.input-replacement').removeClass('selected');
 }
 });
 }
 function label_action(element) {
 element.prevAll('.input-container').children('.styled-input').prop('checked', 'checked').trigger('change');
 }
 function live_check() {
 $('.styled-input:checked').each(function() {
 $(this).siblings('.input-replacement').addClass('selected');
 });
 };
 return this.each(function() {
 var $this = $(this);
 add_essentials($this);
 live_check();
 $('body').on('change', '.styled-input', function() {
 replace_default_action($this);
 });
 $('body').on('click', '.label-replacement', function() {
 label_action($this);
 });
 });
}
$('.styled-input').replaceme();

• megawac: group and native_type are globals to start. Edit, also if you'd construct a demo of what your code does on jsfiddle/bin I'd appreciate it
• John McGuinnes: jsfiddle.net/UCdKL/9 Here is the fiddle, thanks for your quick response

Answer 1

Scanning your code I have a few recommendations to make. If you update your question with a demo I'll make a few more.

As I mentioned in comments you have a few obvious issues with globals

var native_btn = element;
native_type = native_btn.attr('type');
//should be
var native_btn = element,
 native_type = native_btn.attr('type');

And similarly for group. I don't understand why you're doing this

function replace_default_action(element) {
 var input = element;
 group = input.attr('name');

Why not:

function replace_default_action(input) {
 var group = input.attr('name'); //on another note I'd prefer groupName over group

Throughout your code you make pretty good use of chaining in places it is clear and concise however there are a few spots I would adapt

Obvious one:

$('body').on('change', '.styled-input', function() {
 replace_default_action($this);
 })
 .on('click', '.label-replacement', function() {
 label_action($this);
 });

You can write this code with toggleClass to remove some lines

 var input_select = $(this);
 if (input_select.is(':checked')) {
 input_select.siblings('.input-replacement').addClass('selected');
 } else {
 input_select.siblings('.input-replacement').removeClass('selected');
 }
 //With toggle class
 var input_select = $(this);
 input_select.siblings('.input-replacement').toggleClass('selected', input_select.is(':checked'));

Also I'm pretty sure you have a logic error here:

.append('<span class="' + native_type + ' input-replacement"></div>')

Finally in javascript, especially with jQuery code (per their style guide), we prefer camelcasing over underscores for names. I'll point you to this discussion from SO.

Update Here's how I would probably write your code and here's your updated fiddle up and running. As you'll notice I removed all your helper functions that were only called once as there was no need for them and I found them confusing to be honest. I added guiding comments to replace them. I corrected the things mentioned in this review, including function names and condensed ways of writing the original. In the future I think you would benefit from trying to write your functions as straightforward and simple as possible (it took me a while to understand your original code cause of all the helper abstractions)!

$.fn.replaceme = function() {
 'use strict';
 return this.each(function() {
 var $this = $(this);
 var native_type = $this.attr('type');
 //setup button for plugin
 $this.each(function() {
 var $btn = $(this);
 $btn.next('label').addClass('label-replacement');
 $btn.wrap('<div class="' + native_type + ' input-container">');
 $btn.parent('.input-container').append('<span class="' + native_type + ' input-replacement">');
 });
 //check if is live
 $('.styled-input:checked').each(function() {
 $(this).siblings('.input-replacement').addClass('selected');
 });
 //handlers
 $('body').on('change', '.styled-input', function() {//set default action, override if already one exists
 var group = $this.attr('name');
 var $cur = $(this);
 $('input[name="' + group + '"]').each(function() {
 $cur.siblings('.input-replacement').toggleClass('selected', $cur.is(':checked'));
 });
 })
 .on('click', '.label-replacement', function() {
 $this.prevAll('.input-container').children('.styled-input').prop('checked', 'checked').trigger('change');
 });
 });
};

• John McGuinnes: Thanks for reviewing my code, i've learned a few new tips and tricks. Especially about organizing my code. I've got one question, what is the logic error you're pointing out?
• megawac: You're opening with a span and closing with a div. Should probably just remove the closing tag $('<span class="' + native_type + ' input-replacement">')
• John McGuinnes: Wow, it's time to take a nap! Haha, i feel so stupid!
• megawac: @JohnElswisk I editted my answer with a counter proposal
• megawac: Also looking at it again I'm certain this loop is unnecessary $('input[name="' + group + '"]').each(function() {