?
Solved

How to refactor edit/delete jQuery to include else statements

Posted on 2016-11-26
5
Medium Priority
?
85 Views
Last Modified: 2016-11-27
I had this question after viewing confirm popup before record delete takes place.

I am trying to make this code shorter by possibly changing to say if I click on edit then run the edit ajax otherwise if I click on delete then run the delete ajax.

	$(document).ready(function() {
$('.delete').on('click', function (e) {
					e.preventDefault();
					var $locationId = $(this).data('location-id');
					var $locationName = $(this).data('location-name');
					var $action = $(this).data('action');
					console.log($action);
					console.log($locationId);
					console.log($locationName);
					swal({
						title: 'Are you sure?',
						text: "You won't be able to undo this!",
						type: 'warning',
						showCancelButton: true,
						confirmButtonColor: '#FB404B',
						cancelButtonColor: '#d33',
						confirmButtonText: 'Yes, delete it!'
					}, function (isConfirm) {
						if (isConfirm) {
							$.ajax({
								type: 'post',
								url: 'formhandler.php',
								data: {locationName: $locationName, locationId: $locationId, action: $action},
								success: function (result) {
									window.location.href = 'locations.php'; 
								},

							});
						}else{
							console.log('cancelled');
						}
					});
				});

	
	$('.edit').on('click', function (e) {
					e.preventDefault();
					var $locationId = $(this).data('location-id');
					var $locationName = $(this).data('location-name');
					var $newLocationName = $(this).parents('tr').find("input[type='category']").val();
					var $action = $(this).data('action');
					console.log($action);
					console.log($locationId);
					console.log($locationName);
					console.log($newLocationName);
			
							$.ajax({
								type: 'post',
								url: 'formhandler.php',
								data: {locationName: $newLocationName, locationId: $locationId, action: $action},
								success: function (result) {
									window.location.href = 'locations.php'; 
//									alert(result);
								},

							});
						});
					});

Open in new window

0
Comment
Question by:Black Sulfur
[X]
Welcome to Experts Exchange

Add your voice to the tech community where 5M+ people just like you are talking about what matters.

  • Help others & share knowledge
  • Earn cash & points
  • Learn & ask questions
5 Comments
 
LVL 9

Accepted Solution

by:
Moussa Mokhtari earned 1200 total points
ID: 41902676
You can do this
$(document).ready(function() {
$('.delete, .edit').on('click', function (e) {
					e.preventDefault();
					var $locationId = $(this).data('location-id');
					var $locationName = $(this).data('location-name');
					var $action = $(this).data('action');

					if ($(this).hasClass('delete')) {
						swal({
						title: 'Are you sure?',
						text: "You won't be able to undo this!",
						type: 'warning',
						showCancelButton: true,
						confirmButtonColor: '#FB404B',
						cancelButtonColor: '#d33',
						confirmButtonText: 'Yes, delete it!'
					}, function (isConfirm) {
						if (isConfirm) {
							$.ajax({
								type: 'post',
								url: 'formhandler.php',
								data: {locationName: $locationName, locationId: $locationId, action: $action},
								success: function (result) {
									window.location.href = 'locations.php'; 
								},

							});
						}else{
							console.log('cancelled');
						}
					});
					}else{
					var $newLocationName = $(this).parents('tr').find("input[type='category']").val();
					$.ajax({
								type: 'post',
								url: 'formhandler.php',
								data: {locationName: $newLocationName, locationId: $locationId, action: $action},
								success: function (result) {
									window.location.href = 'locations.php'; 
//									alert(result);
								},

							});
					}

				});
});

Open in new window

0
 
LVL 58

Assisted Solution

by:Julian Hansen
Julian Hansen earned 200 total points
ID: 41902816
Edit and delete are two separate actions. There is often the temptation to combine code to make it more "efficient" by making it shorter but at the expense of readability.

Personally - I would keep the operations separate.
0
 
LVL 1

Author Comment

by:Black Sulfur
ID: 41902846
@ Julian,

So, are you saying it would be better to keep the code as per my original post?
0
 
LVL 31

Assisted Solution

by:Marco Gasi
Marco Gasi earned 600 total points
ID: 41902850
Hi.  I can agree with Julian but only depending on the compexity of involved code. in this case the if statement occupies only 6 lines and the rest is identical so readability is not compromised at all.
				$('.action').on('click', function (e) {
					e.preventDefault();
					var $locationId = $(this).data('location-id');
					var $locationName = $(this).data('location-name');
					var $action = $(this).data('action');
					if ($action == 'edit') {
						var $newLocationName = $(this).parents('tr').find("input[type = 'category']").val();
						var data = {locationName: $newLocationName, locationId: $locationId, action: $action};
					}else{
						var data = {locationName: $locationName, locationId: $locationId, action: $action};
					}
					console.log($action);
					console.log($locationId);
					console.log($locationName);
					console.log($newLocationName);
					swal({
						title: 'Are you sure?',
						text: "Do you want to change this record?",
						type: 'warning',
						showCancelButton: true,
						confirmButtonColor: '#3085d6',
						cancelButtonColor: '#d33',
						confirmButtonText: 'Yes, edit it!'
					}, function (isConfirm) {
						if (isConfirm) {
							$.ajax({
								type: 'post',
								url: 'formhandler.php',
								data: {locationName: $locationName, locationId: $locationId, action: $action},
								success: function (result) {
									window.location.href = 'formhandler.php';
								},
								error: function (jqXHR, textStatus, errorThrown) {
									console.log(textStatus, errorThrown);
								}
							});
						} else {
							console.log('cancelled');
						}
					});
				});

Open in new window


Remember to replace classes 'edit' and 'delete' with class 'action' in your buttons' markup :)
0
 
LVL 58

Expert Comment

by:Julian Hansen
ID: 41903648
I am saying - keep the functions separate and if there is overlap (and it makes sense to do so) move that functionality into a function with parameters.
0

Featured Post

Free Tool: ZipGrep

ZipGrep is a utility that can list and search zip (.war, .ear, .jar, etc) archives for text patterns, without the need to extract the archive's contents.

One of a set of tools we're offering as a way to say thank you for being a part of the community.

Question has a verified solution.

If you are experiencing a similar issue, please ask a related question

Introduction A frequently asked question goes something like this:  "I am running a long process in the background and I want to alert my client when the process finishes.  How can I send a message to the browser?"  Unfortunately, the short answer…
Introduction Knockoutjs (Knockout) is a JavaScript framework (Model View ViewModel or MVVM framework).   The main ideology behind Knockout is to control from JavaScript how a page looks whilst creating an engaging user experience in the least …
The viewer will learn how to dynamically set the form action using jQuery.
The viewer will learn the basics of jQuery, including how to invoke it on a web page. Reference your jQuery libraries: (CODE) Include your new external js/jQuery file: (CODE) Write your first lines of code to setup your site for jQuery.: (CODE)
Suggested Courses

770 members asked questions and received personalized solutions in the past 7 days.

Join the community of 500,000 technology professionals and ask your questions.

Join & Ask a Question