How can I improve this php code

Question

Hi, I have a txt file that gets loaded and printed using a class.

I'm relatively new to php, so I wonder if the class can be improved and some of the duplicated code (when printing the change-log) can be removed.

Here is the class :

class changesHandle {

    public function load($theFile, $beginPos, $doubleCheck) {

        // Open file (read-only) and don't show errors
        $file = @fopen($_SERVER['DOCUMENT_ROOT'] . '/home/' . $theFile, 'r');

        $changes = Array();

        // Chnage the encoding of the file
        function ar($string) {
            $string = trim(iconv('UTF-8', 'windows-1256', $string));
            return $string;
        }

        //check if the file is opened
        if ($file) {

            // While not at the End Of File
            while (!feof($file)) {

                // Current line only
                $line = ar(fgets($file));

                $findBegining = strpos($line, $beginPos);

                // Double check for the begining
                $beginingCheck = strpos($line, $doubleCheck);

                // Find the begining
                if ($findBegining !== false && $beginingCheck !== false) {

                    while (!feof($file)) {

                        $line = ar(fgets($file));

                        // Remove space and the first 2 charecters ('-' + one space)
                        $line = trim(substr($line, 2));

                        if (!empty($line)) { // Don't add empty lines
                            array_push($changes, $line);
                        }
                    }
                }
            }
        } else {
            echo 'File ' . $theFile . ' does not excist';
        }

        // Close the file to save resourses
        @fclose($file);

        return $changes;
    }

    public function construct($array) {

        // Exit if the array has no data
        if(empty($array)) {
            return 'لا توجد تغييرات في هذه النسخة.';
        }

        // Count how many items does the array has
        $entries = count($array);

        $printedChange = '';

        // Make the first div with it's data
        if ($entries > 3) {

            $printedChange .= "<div>";

            for ($i = 0; $i < 3; $i++) {
                $printedChange .= "<li>$array[$i]</li>";
            }

            $printedChange .= "</div>";
        } elseif ($entries <= 3) {

            $printedChange .= "<div class='alone'>";

            for ($i = 0; $i < $entries; $i++) {
                $printedChange .= "<li>$array[$i]</li>";
            }

            $printedChange .= "</div>";
        }

        // If there is more than 3 items, make another div
        if ($entries >= 4) {

            $printedChange .= "<div>";

            if ($entries > 6)
                $entries = 6;

            for ($i = 3; $i < $entries; $i++) {
                $printedChange .= "<li>$array[$i]</li>";
            }

            $printedChange .= "</div>";
        }

        return $printedChange;

    }

}

This is how I call the class :

$changes = new changesHandle();

$loadedChanges = $changes->load('change-log.txt', $contact['version'], $contact['updateDate']);

$printedChanges = $changes->construct($loadedChanges);

echo '<ol>' . $printedChanges . '</ol>';

I coded this using the "Object Oriented Programming in PHP" approach (though I can't fully wrap my head around it yet :) ), So if anyone can tell me if it can be improved too in this side that would be highly appreciated.

Thanks in advance.

Answer 1

A few things at a quick glance:

1. Don't use @. Ever. You need to deal with the errors, not just suppress them.
2. move the ar() function definition outside of the load() method.
3. in the load() method, you do a check to see if the file was loaded, but then outside of this if block you close the file at the end of the method. Say that there was an error opening the file, then you're calling close() on a file that was never opened:

this:

if ($file) {
    // ...
}
fclose($file);

should be like this

if ($file) {
    // ...
    fclose($file);
}

And an aside, I think you misspelled "exist" near the end of your load() method