Parsing glider data from IGC files

Question

I have to do a program in PHP that reads IGC files like this IGC file and gets records about the glider.

For now I came up with something like this:

<?php
 class Pilot {
 public $name;
 public $gliderType;
 public $competitionId;
 public $gpsDatuml;
 public $competitionClass;
 public $startPoint;
 public $endPoint;
 }
 $pilot = new Pilot();
 $myFile = new SplFileObject("https://xcportal.pl/sites/default/files/tracks/2020-06-09/069daro396091568.igc", 'r', 10);
 for($i=0; $i < 10; $i++) {
 $information = $myFile->fgets();
 if(($pos = strpos($information, "PILOT")) !== FALSE){
 $pilot->name = substr($information, $pos+7);
 }
 if(($pos = strpos($information, "GLIDERTYPE")) !== FALSE){
 $pilot->gliderType = substr($information, $pos+12);
 }
 if(($pos = strpos($information, "COMPETITIONID")) !== FALSE){
 $pilot->competitionId = substr($information, $pos+15);
 }
 if(($pos = strpos($information, "GPSDATUM")) !== FALSE){
 $pilot->gpsDatuml = substr($information, $pos+10);
 }
 if(($pos = strpos($information, "CLASS")) !== FALSE){
 $pilot->competitionClass = substr($information, $pos+7);
 }
 // echo strpbrk($information, "PILOT");
 // echo substr($information, 10);
 }
 echo $pilot->name;
 echo $pilot->gliderType;
 echo $pilot->competitionId;
 echo $pilot->gpsDatuml;
 echo $pilot->competitionClass;
 
 $myFile = null;
 ?>

but I wonder if there is anyway to do it better that I haven't figured out already.

Also is it good practice to keep properties of the class as a private field or can I leave them as public if any other class doesn't use that property?

Answer 1

I don't think I understand the point of declaring a class if you aren't going to write any methods in it. I mean why go the expense of declaring a class just to store some variables outside of the global scope. Assuming this class can never populate its properties without parsing a document, so it make sense to have its constructor initiate the file reading and generate values for the properties.

• I haven't personally used SPLFileObject before, but the manual says that:

  • the 2nd parameter $open_mode defaults to r and
  • the 3rd parameter $use_include_path expects a boolean value. I expect that that 10 is not doing what you intend.

• Reading a limited number of lines from the file with fgets() is a professional decision that shows real care and deliberate scripting. Removing all debate regarding micro-optimization or if reading the whole file is an "affordable" cost, I love that you are endeavoring to ONLY "read what you need".

• By my count, there are 3 factors which should halt the loop of reading of lines:

  1. there are no more lines in the file
  2. a line is encountered that does not contain a colon
  3. you have extracted all relevant details from the file up to your self-imposed "magic counter" 10

  The first two logical break points should be baked into the loop's breaking syntax. The third is probably something that you don't want to get into the habit of. Another developer may look at the code (without seeing a sample input file) and think: "Why is this operation limited to 10? Why not 20? Why not 5?" It is best to avoid magic numbers, or if you need to have them, write a comment explaining why it is implemented.

• That battery of if conditions is considerably less wise. Think about it. If the first if is satisfied -- why would you want to bother executing the other four checks? They should never be satisfied if the first one is. While I am going to suggest a different technique in my snippet to follow, your snippet at the very least would be improved by using subsequent elseif conditions.

Because I'm rather familiar with regex, I find using regex with a lookup array to be a very convenient technique to extract your targeted values from the remote file. Not only does regex afford a more compact script, it maintains the flexibility of your original script by allowing the targeted lines to exist in any order.

A code suggestion:

class Pilot
{
 public $name;
 public $gliderType;
 public $competitionId;
 public $gpsDatuml;
 public $competitionClass;
 public $startPoint; // I don't know where/how this gets populated
 public $endPoint; // I don't know where/how this gets populated
 private $keywords = [
 'PILOT' => 'name',
 'GLIDERTYPE' => 'gliderType',
 'COMPETITIONID' => 'competitionId',
 'GPSDATUM' => 'gpsDatuml',
 'CLASS' => 'competitionClass',
 ];
 
 public function __construct(string $url)
 {
 $this->populatePropertiesFromUrl($url); 
 }
 
 public function populatePropertiesFromUrl(string $url): void
 {
 $pattern = '~(' . implode('|', array_keys($this->keywords)) . '): (.*)~';
 $file = new SplFileObject($url);
 while (!$file->eof()
 && ($line = $file->fgets())
 && strpos($line, ':') !== false
 ) {
 if (preg_match($pattern, $line, $match)) {
 $this->{$this->keywords[$match[1]]} = $match[2];
 }
 }
 }
}
$pilot = new Pilot('https://xcportal.pl/sites/default/files/tracks/2020-06-09/069daro396091568.igc');
var_export(get_object_vars($pilot));

Output (should be):

array (
 'name' => 'DARIUSZ KISZKO',
 'gliderType' => 'ADVANCE IOTA 2',
 'competitionId' => '0000',
 'gpsDatuml' => 'WGS-84',
 'competitionClass' => 'Paraglider (Standard)',
 'startPoint' => NULL,
 'endPoint' => NULL,
)

*Disclaimer: I am making some assumptions about the text in the remote file. I don't actually know how the text may vary. I don't know if my suggested code will hold up against other files. You may wish to adjust my loop breaking algorithm.

• Sᴀᴍ Onᴇᴌᴀ: Nice - I like it ... I believe the while conditions would need to be updated like while (!$file->eof() && ($line = $file->fgets()) && strpos($line, ':') !== false) { since 1. the assignment of $line seems to need to be outside the call to strpos (unless there is some config option I am missing?) and 2. the loop would stop as soon as a line contains a colon if === is used between strpos() and false; And I agree about it seeming pointless to have a class with no methods - I guess my thought was incomplete.
• mickmackusa: Yes, thanks, I had the boolean check reversed. I clearly did not test my code and I still haven't because I don't have the time to setting a test. I am only eyeballing the code. I am assuming that fgets() is going to return a scalar value -- when it is false, it will not find a colon. Admittedly, I don't actually like declarations inside of conditions, but I was endeavoring to avoid any separate break conditions inside the while()'s body.