0

I wrote this counter which keeps track of yes/no for a website and it works decently well, the problem is somehow the files get messed up while writing. For example: it'll go from 126 to 27. The script gets called from an iOS app I wrote so most likely there are multiple connections modifying the file at the same time and I think this is what is causing the problem. I am not really a PHP guy so I was hoping for some insight on what could make the code a little better and handle multiple simultaneous connects.

<?php

        $yes_file = 'yes.txt';
        $no_file  = 'no.txt';

        $yes_count = file_get_contents($yes_file);
        $no_count = file_get_contents($no_file);

        if ($_GET['result'])
        {
                if( strcmp($_GET['result'], "YES") ) {
                        $no_count+=1;
                        file_put_contents($no_file, $no_count);
                }
                else {
                        $yes_count+=1;
                        file_put_contents($yes_file, $yes_count);
                }
        }

        $total = $yes_count + $no_count;
        echo "{\"yescount\":" . $yes_count.",";
        echo "\"nocount\":" . $no_count.",";
        echo "\"total\":" . $total."}";

?>

Thanks!

Corey
  • 771
  • 3
  • 14
  • 32
  • Might want to check this out http://stackoverflow.com/questions/15435619/php-lock-text-file-for-editing – j08691 May 27 '13 at 21:53
  • 6
    use a data base not a txt file, problem solved –  May 27 '13 at 21:53
  • Yeah, I was considering that. I was trying to make it as simple as possible because it's a free app with no ads. I'll probably switch to a db at some point soon – Corey May 27 '13 at 21:57
  • A db is simpler than a flat file. i have never found using a db harder than adding\editing\manipulating a flat file –  May 27 '13 at 22:01
  • just remember to use transaction with db or you'll have the same problem. – imel96 May 27 '13 at 22:43

2 Answers2

1

First of all, I'd propose to use a database system to track counters.

Regarding your problem, it would be helpful to flock() the file during the read-update-write cycle. This could prevent race conditions.

Marty
  • 39,033
  • 19
  • 93
  • 162
SteAp
  • 11,853
  • 10
  • 53
  • 88
1

This should be more efficient.

FYI, a database puts a write lock on the row/table whilst incrementing, which is the same as what I've done below, so a database is not the solution - the solution is a write lock (via a database or via PHP). You could use flock, but I find that messy so I've done it just with a temporary file.

The only issue with my code here is that if the server crashes in the middle of this script then the write lock will stay in place (MySQL has this issue too sometimes). I usually get around this by writing the time() in the file and checking that it is not older than an an hour or something. But in your case it's probably unnecessary.

<?php

// Your variables
$yes_file = 'yes.txt';
$no_file  = 'no.txt';

if (isset($_GET['result']))
{
// Write lock
while(file_exists('temporaryfile')) usleep(100000);
file_put_contents('temporaryfile','1');

$yes_count = (int)file_get_contents($yes_file);
$no_count = (int)file_get_contents($no_file);

// Increment
if ($_GET['result']=='YES')
    {
    $yes_count++;
    file_put_contents($yes_file, $yes_count);
    }
else
    {
    $no_count++;
    file_put_contents($no_file, $no_count);
    }

// Unlock
unlink('temporaryfile');
}
else // No need for any lock so just get the vars
{
$yes_count = (int)file_get_contents($yes_file);
$no_count = (int)file_get_contents($no_file);
}

$total = $yes_count + $no_count;
echo "{\"yescount\":$yes_count,\n\"nocount\":$no_count,\n\"total\":$total}";
Alasdair
  • 13,348
  • 18
  • 82
  • 138