Efficiency Comparison Between the Codes

I would like to get feedback on the efficiency of solving a problem with a PHP script, as I wrote code different from my computer science teacher’s, which he did not look upon favorably.

CODE 1

if (isset($_POST["modificato"])) { // Checks if the form has been submitted with the "modificato" field.
   $descriptor = fopen("palloniOro.txt", "r"); // Opens the "palloniOro.txt" file in read mode.
   
   while (!feof($descriptor)) { // Continues reading until the end of the file.
       $line = fgets($descriptor); // Reads a line from the file.
       $content = str_replace("\n", "", $line); // Removes the newline character (\n) from the read line.
       
       if (strlen($content) > 2) { // Checks that the line is not empty or too short.
           $balls[] = explode(",", $content); // Splits the line into an array using a comma as the delimiter.
       }
   }
   
   $year = $_POST["anno"]; // Retrieves the year value sent via POST.
   $winner = $_POST["vincitore"]; // Retrieves the winner's name sent via POST.
   $team = $_POST["squadra"]; // Retrieves the team's name sent via POST.
   $score = $_POST["punteggio"]; // Retrieves the score sent via POST.
   
   fclose($descriptor); // Closes the file opened in read mode.
   $descriptor = fopen("palloniOro.txt", "w"); // Reopens the file in write mode to overwrite it.
   
   $notFound = -1; // Initializes a variable to check if the year was found (-1 means "not found").
   
   for ($i = 0; $i < count($balls); $i++) { // Iterates over each line stored in `$balls`.
       if ($balls[$i][0] == $year) { // If the first value (year) matches the specified year.
           fwrite($descriptor, $year . "," . $winner . "," . $team . "," . $score . "\n"); // Writes the updated data to the file.
           $notFound = 1; // Sets `$notFound` to 1 to indicate that the line was found and updated.
       } else { 
           fwrite($descriptor, $balls[$i][0] . "," . $balls[$i][1] . "," . $balls[$i][2] . "," . $balls[$i][3] . "\n"); // Writes the original line without modifications.
       }
   }
   
   fclose($descriptor); // Closes the file after writing all lines.
}

CODE 2 (MINE)

if (isset($_POST['modifica'])) { // Checks if the form has been submitted with the "modifica" field.
   $done = false; // Variable to indicate if a modification was made (initially false).
   $backupStream = fopen('backup.txt', 'a'); // Opens (or creates) the "backup.txt" file in append mode to write updated data.
   $mainStream = fopen('palloniOro.txt', 'r'); // Opens the original "palloniOro.txt" file in read mode.
   
   while (!feof($mainStream)) { // Continues reading until the end of the file.
       $line = fgets($mainStream); // Reads a line from the file.
       if ($line) { // Checks that the line is not empty.
           $data = explode(',', $line); // Splits the line into an array using a comma as the delimiter.
           if ($data[0] == $_POST['anno']) { // Checks if the year in the line matches the year sent via POST.
               $data[1] = !empty($_POST['nome']) ? $_POST['nome'] : $data[1]; // Updates the name if provided, otherwise keeps the original value.
               $data[2] = !empty($_POST['squadra']) ? $_POST['squadra'] : $data[2]; // Updates the team if provided, otherwise keeps the original value.
               $data[3] = !empty($_POST['punti']) ? $_POST['punti'] : str_replace("\n","",$data[3]); // Updates the score if provided, otherwise keeps the original value.
               $line = implode(',', $data); // Reassembles the updated line into a comma-separated string.
               $line = $line . "\n"; // Adds the newline character (\n).
               $done = true; // Indicates that a modification has occurred.
           }
           if (!$done) { // If no modification was made.
               $line = implode(',', $data); // Keeps the original line unchanged.
           }
           fwrite($backupStream, $line); // Writes the line (modified or not) to the backup file.
       }
   }
   fclose($backupStream); // Closes the "backup.txt" file.
   fclose($mainStream); // Closes the original "palloniOro.txt" file.
   unlink('palloniOro.txt'); // Deletes the original "palloniOro.txt" file.
   rename('backup.txt', 'palloniOro.txt'); // Renames "backup.txt" to "palloniOro.txt", replacing the original file.
}

As you can see, the script handles a POST request from a form to modify a row in a text file. My teacher don’t accept the append mode instead of the write mode, but i prefer that because I want to use two stream and file for a safe reading and writing. Who is right??

The text file has the following repeated structure for multiple lines:

YEAR1,PLAYER1,TEAM1,SCORE1
YEAR2,PLAYER2,TEAM2,SCORE2

Feel free to ask for clarifications about the problem, and thank you!

1 Like

Though we see a lot of people do it, this isn’t the right way to check for post submission. This is:-

if($_SERVER['REQUEST_METHOD'] === 'POST'){

When it comes to opening and processing the file, your file appears to be a CSV format. PHP has tools for working with CSV data which will make things much simpler. Eg:-

This whole block is a waste of coding, it serves no purpose at all to assign a variable value to another variable. It will only be useful if you trim or validate the input in some way. Passing unchanged values is pointless.

I think your’s may be better, but can be improved, Though I think your teacher probably won’t like you writing better code than them. :wink:

Hi,Surely I can write better code with your advice, but this is for educational purpose. However I’m happy about your feedback, thank you

The thing with opening backup.txt in append mode is that if somehow backup.txt already exists (possibly because another request is also being processed simultaneously) then the output would be incorrect, as the data already in backup.txt also ends up in the final output. Opening the file in write mode would also work and not have a chance at invalid output.

And, one of the problems about writing code that’s “better” than the teacher’s code is that, if the teacher is marking the course and responsible for deciding what grade you get, whether you pass or fail, then you should really do it as you’ve been taught, even if you can do better after you’ve finished the course. A lot of courses are about how well you can absorb the information you’ve been given and use it in assignments and tests, regardless of whether the specific techniques are still the best way to do things.

Probably the reason that in my A-level course, the teacher used a fictitious assembly language rather than a common one, and probably why my mate didn’t get good marks when he decided to use Z80 assembler instead.

2 Likes

Alright, I agree with you, but this is just a class test, not an actual application. I was looking for feedback on the efficiency of the codes logic. Thank you.

Both algorithms read the entire file and write the entire file, so performance should be similar. Kudos for the idea of writing a backup and then swapping it for the master file, but as @rpkamp mentions, you open yourself up for data sharing issues (that you haven’t addressed) and while there are techniques to deal with them, they are costly and likely not been taught yet.