A Simple Refactoring

Yesterday, I was required to create some Javascript functionalities using Prototype. It’s nothing out of ordinary, I was to create a page with 3 radio buttons and 1 select, depending on which button selected – the select will be enabled/disabled.

Being new with Prototype as well as having a task with a fixed allocated time, I didn’t want to invest too much time in front to get this task done - so I quickly looked up at some existing codes and adapt them to meet the requirement.

I came up with this code below, it gets the work done and the task is completed.

<script type="text/javascript">
	Event.observe(
		window
		, "load"
		, function(){

			Event.observe(
				$( 'button-none' )
				, 'change'
				, function(){ toggleSelection( 'None' ); } );

			Event.observe(
				$( 'button-all' )
				, 'change'
				, function(){ toggleSelection( 'All' ); } );

			Event.observe(
				$( 'button-selected' )
				, 'change'
				, function(){ toggleSelection( 'Selected' ); } );

		}
		, false
	);

Now to stop here, would “probably” be OK – the functionality works as required, however I feel that the code can be done in a better way PLUS I have spare time to actually improve the code.

The problem with the current version is the repetition of the code that attaches event to the buttons, it would be nice if I have a generic block of code that does the attaching of event observer to any buttons required.

So I decided to Google around and see some more code examples out there. And this the final version that I come up with.

<script type="text/javascript">
	Event.observe(
		window
		, "load"
		, function(){
			var buttonIds = $( 'button-none', 'button-all', 'button-selected');

			buttonIds.each(
				function( item ){
					Event.observe(
						item
						, 'click'
						, function(){
							// the regex bit is to get the last word from the ids ie: none, all, selected
							toggleSelection( item.id.replace( new RegExp( '[a-zA-Z]+[\-]', 'g' ), '' ) );
						}
					);
				}
			);
	);
</script>

I feel at peace with this current version – perhaps it still can be improved more, suggestions are welcomed.

Also note that I am now attaching the event observer to the click event instead of the change event because the on change event in IE behaves differently than in Firefox.

Moral of the story - always seek to improve your codes.