1
\$\begingroup\$

I have created a Like/Unlike system. The user clicks one to indicate a "like", and clicks again to "unlike" it. Currently I do that like this:

try {                                             -- like
    $stm1 = $db_conn->prepare("INSERT into votes (post_id, user_id) VALUES (?, ?)");
    $stm1->execute(array($post_id, $_SESSION['id']));

} catch(PDOException $e){
    if ((int) $e->getCode() === 23000) {          -- unlike
        $stm2 = $db_conn->prepare("DELETE FROM votes WHERE post_id = ? AND user_id = ?");
        $stm2->execute(array($post_id, $_SESSION['id']));

    } else {
        $error = true;
    }
}

Note: $e->getCode() === 23000 means row is duplicate. because there is a unique index on post_id and user_id columns.

Well my code works as well. Just when a professional programmer see it, he says: your algorithm is wrong and you shouldn't delete something when insert fails (on duplicate).

Well, should I change my current code? What's wrong with it? Trust me it works perfectly :) ..!

\$\endgroup\$
  • 1
    \$\begingroup\$ Why don't you ask the guy who told you it's bad what he meant exactly? \$\endgroup\$ – Mast Jun 5 '16 at 14:27
  • 1
    \$\begingroup\$ My only thought is that unliking something isn't exceptional - it's an intended feature of the application. Treating it as an exception means treating it like an error, which it isn't. \$\endgroup\$ – HPierce Jun 5 '16 at 14:28
  • \$\begingroup\$ @Mast I did .. but he just says "deleting on error isn't a good idea" ..! that's what I cannot understand. \$\endgroup\$ – stack Jun 5 '16 at 14:30
  • 1
    \$\begingroup\$ I didn't downvote, but are you absolutely sure this works? I'm no PHP expert, but this looks like an odd way of database management. \$\endgroup\$ – Mast Jun 5 '16 at 14:30
  • 1
    \$\begingroup\$ So essentially, if it is present in your database, that means it's a "Like", and if it's not present, there is no like? I suppose that's why you have the feature that removes it if it already is there? So if it was liked already, clicking will unlike it? \$\endgroup\$ – Simon Forsberg Jun 5 '16 at 15:22
4
\$\begingroup\$

This design is a consequence of a poorly designed API. It looks like the client will hit one URL that will cause the like/unlike state to be toggled. But what if the user has multiple tabs open on the same page, then clicks "like" on each one? The user might believe that they are reiterating the "like", when in fact they are undoing it.

My advice is not to toggle the state on the server side. Offer "like" and "unlike" as two separate calls. The client should be responsible for deciding which API to call, based on the client-side state.

\$\endgroup\$
  • \$\begingroup\$ Yes exactly cliend hit one URL for both like and unlike state. However I don't understand your point of multiple tabs. Anyway +1 \$\endgroup\$ – stack Jun 5 '16 at 16:45
3
\$\begingroup\$

There are (at least) two different approaches that are better than your current approach.

One option is to run a SELECT query first to check whether it exists or not. If it exists, run DELETE. If it doesn't exist, run INSERT. Personally I wouldn't prefer this solution as it is slower than necessary and, if implemented correctly, would require more table locking. Consider the case of two scripts running simultaneously, here's what could happen:

  • Script 1 queries database and finds that nothing exists yet
  • Script 2 queries database and finds that nothing exists yet
  • Script 1 inserts into database
  • Script 2 inserts into database

Now both scripts will insert into database, which will cause a duplicate on the second one.


Another option is to use an additional column in your database that specifies whether or not it is a like. So instead of running DELETE on your existing row, you can just run UPDATE and set the flag to false (to say "don't count this"). Using this option allows you to run an INSERT ... ON DUPLICATE KEY UPDATE statement that can handle both cases.

I agree that catching an exception and deleting the row in that case is not a good way to handle. You shouldn't use try ... catch as normal program flow. Exceptions are meant to be exceptions, not normal behavior.

\$\endgroup\$
  • \$\begingroup\$ Your first approach is acceptable .. thx +1 \$\endgroup\$ – stack Jun 5 '16 at 15:30
  • 1
    \$\begingroup\$ Your INSERT … ON DUPLICATE KEY UPDATE is a fine solution. However, the SELECT with a possible DELETE is bad advice: it creates all kinds of race conditions. \$\endgroup\$ – 200_success Jun 5 '16 at 15:36
  • 1
    \$\begingroup\$ @200_success Agreed, I thought about that as well. I edited. \$\endgroup\$ – Simon Forsberg Jun 5 '16 at 16:58
1
\$\begingroup\$

It does look a little dirty depending on errors for normal operations. This will bloat error logs. I think it would be more proper to write a stored procedure that first SELECTs whether the record exists, then dependent on result inserts or deletes the record.

\$\endgroup\$
1
\$\begingroup\$

I would not recommend to implement any application logic relying on Exceptions created through external sources.

Imagine someone drops the unique index in your database (through fault or purpose). Your application would still be working, but it will be possible to "like" a post an unlimited amount of times. Because the Exception will never be thrown.

You surely will notice it, but until then your table votes will be in an inconsistent state.

An option would be to include a score column inside votes table. This way you could follow these step.

  1. Insert entry of score 1/0 into table for post_id,user_id on first like/unlike. or
  2. Set score to 1/0 for post_id,user_id on successive like/unlike's.
\$\endgroup\$
  • \$\begingroup\$ Exactly. INSERT into votes (post_id, user_id, score) VALUES (?, ?, 1) ON DUPLICATE KEY UPDATE score=1-score. \$\endgroup\$ – Vedran Šego Jun 13 '16 at 21:12
0
\$\begingroup\$

You have to do this in a Not style so you do to the following:

1.Get the answer of the current user whether like or unlike.

2.Make an inverting to this answer.

3.then make the modify operation with the new answer

-Note that you have to add a field in your table which is the value that would be 0 or 1 according to like or unlike.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.