It is working. But is it better?

Question

When I start learning a new language, it always seems to me that I am not doing it in a practical, standard way. So, here the jQuery question and what I am doing is acceptable.

I have 3 images.

<img src="del.png" class="delete user" /> <img src="del.png" class="delete event" /> <img src="del.png" class="delete recipe" /> 

Then I have jQuery to detect the click event on $ ('. Delete').

 $('.delete').click(function() { if( $(this).hasClass('user') ) { action = 'user' } if( $(this).hasClass('event') ) { action = 'event' } if( $(this).hasClass('recipe') ) { action = 'recipe' } // I then use ajax, send the action as $_POST to the PHP  // and it tells PHP what to delete. // Doing it this way, I don't have to use 3 onclick functions and 3 ajax functions. }); 

Answer 1

I agree with @Tommi, you keep yourself from writing more code than you need, and also to consolidate your ajax call in one place instead of three about the same ajax calls (or writing an ajax function and calling it from 3 in separate places )

If this works for you and does not confuse, I do not think that something is wrong with him. There are actually many ways to do what you are trying to do in an ugly and inelegant way, but this is not one of them.

Answer 2

I would say that there is nothing wrong with doing what you do. It is actually very clean and readable for using two classes in such a dispatch function.

Answer 3

What if the object has a user and an event as classes, then the action will be an event, not a user?

Answer 4

This solution makes your system depend on javascript. A more robust solution is to create something that is at least functional without javascript and then use javascript when loading to change the behavior of the buttons to do something ajax-y more.

It requires more code to do it this way and does not look so elegant. But if you are looking for the “best” solution, you might want to weigh yourself and determine if handling “no javascript” angular register priority or not.

Whatever path you choose, you must at least consciously make a decision one way or another, rather than letting it surprise you later.

Answer 5

You might like to create a separate function for your AJAX and pass action to it.

As your code stands, it will allow you to dynamically change actions and have multiple actions for a single img . I would register if this is intentional.

Answer 6

This code is perfectly visible in isolation. However, I would suggest using the onClick event on <a> directly, rather than relying on the functionality of adding jquery to your elements.

While jquery does an acceptable job, you get code that is hard to read because the end result does not display the fact that it is actually an event handler associated with the onClick event. I personally prefer to see what code is actually running, using tools like Firebug to check the page I'm serving.

This is not a problem if you are the only one who has ever read your code.

Answer 7

If I were tempted to do something like this, I would generalize things a bit more, like this

 function getType(elem) { var m = /\btype:([a-zA-Z_]+)\b/.exec(elem.className); return m ? m[1] : null; } function deleteType(type) { // delete code instead of alert alert("Deleting type " + type); } $(function() { $(".delete").click(function(ev) { var type = getType(this); if (type) { deleteType(type); } return false; }); }); 

used for items such as

 <a href="#" class="delete type:foo">delete foo</a> <a href="#" class="delete type:bar xxx">delete bar</a> <a href="#" class="delete">do nothing</a> 

Extra code to add new types.

Answer 8

Assuming you have a LI structure (or something similar):

 <ul> <li>Some text <a href="delete.php?do=user">delete</a></li> <li>Some text <a href="delete.php?do=event">delete</a></li> <li>Some text <a href="delete.php?do=recipe">delete</a></li> </ul> 

You can do it

 $('.delete').click(function(){ var t=$(this); jQuery.ajax({ type: "GET", url: t.attr('href'), cache: false, success: function(data){ t.parent().remove(); // you remove the whole li tag } }); }); 

Of course, if you need to run a different function for ajax success, then your paths, which I consider to be the best choice.

Answer 9

When reading the code, I had the same desire to reorganize the code into something shorter, especially in that there is a repetition in the structure of the code.

The following solution gets the list of classes of the selected div. Then he retrieves the last class.

 $('.delete').click(function() { // Your 'action' class type must always be the last in the list action = $(this).attr('class').split(' ').slice(-1); // Remaining logic goes here... });