Efficiency Comparison Between the Codes

[kappa1945]

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!

[Peter_S]

Hi @kappa1945,

First of all, as long as you are a student, don't try to challenge your teachers. Eventually, you might be smarter than them, so was I, but that is not the purpose. Teachers are giving you methods and are teaching you a way to organize yourself, in order to achieve a goal. You need to assimilate these basis, then, later, in your professional career, or own projects, you can explore more complex things, or find more advanced solutions. But at first you need to stick with the simple basis.

That being said, I had a quick look at the two codes.

The first thing that comes to my mind is that your code is unnecessarily complex, and can't be understood easily. Keep in mind that, when you write a piece of code, it's not only for yourself, the code needs to be easily understandable by co workers, or other persons working with you. Also, if you want to rework your code in 5 or 10 years, you need to still be able to understand what it does easily and quickly.

K.I.S.S.

Secondly, why are you opening the "backup.txt" file using the "a" attribute? If the file already exists (for any reason, including a problem with the previous call to your script), you are going to append the data at the end, and this is not what you want to do.

If you really want to do something more secure, and faster, you can always look at the "file_get_contents" to load the whole file into a variable that your can "explode" to fetch the lines. And as for writing you can do the opposite, "implode" your lines, and then use "file_put_contents" using the "LOCK_EX" flag.

Also, when you read one line from the file, it looks like you are not removing the end of line character, so when you write back the line, you'll get two end of lines, isn't it? As a side note, removing the "\n" is not safe. I would recommend to use the "rtrim" function, this would handle the end of line and extra white spaces.

But again, I think that you should stick with simple things first, during your classrooms. Just follow the teacher, do simple things. Then once you have mastered the basis, you can then explore advanced methods. But do one thing at a time, first thing first.

Good luck with your studies.