HiveBrain v1.2.0
Get Started
← Back to all entries
patternphpMinor

Function that converts file eol's

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
fileconvertsfunctioneolthat

Problem

I've been unable to find a PHP function that will convert eol in files. This is what I have so far. It works, no errors.

Your educated opinions, thoughts, improvements, potential bugs and suggestions are welcome.

```
/*
$file_name - full/path/to/file.name

$eol - wanted end of line: "\n", "\r", "\r\n"
from "\n" to "\r" || "\r\n"
from "\r\n" to "\r" || "\n"
from "\r" to "\n"

$len - buffer size

return true on success or false on fail
*/
function file_eol($file, $eol, $len=8192) {

$ok = FALSE;
$extension = '.my_temp';

$hread = FALSE;
if (is_string($file)) $hread = fopen($file, 'rb');
if ($hread === FALSE) e("Bad file in file_eol(): $file");
if (!is_resource($hread) || get_resource_type($hread) !== 'stream') e("Bad hread in file_eol(): $hread");

$hsave = fopen($file.$extension, 'wb');
if ($hsave === FALSE) e("Bad hsave file in file_eol(): $file.my_temp");

switch ($eol) {
case "\n": $order = array("\r\n", "\r"); // from "\r\n" || "\r" -> "\n"
break;
case "\r": $order = array("\r\n", "\n"); // from "\r\n" || "\n" -> "\r"
break;
case "\r\n": $order = array("\n", "\r\r\n"); // from "\n" -> "\r\n"
break;

default:
e("Bad eol in file_eol(): $eol");
}

while (($str = fread($hread, $len)) && $str !== FALSE) { // error or eof

$new = str_replace($order, $eol, $str);
// if (is_string($new)) $ok = fwrite($hsave, $new); // this ???
if ($new !== '') $ok = fwrite($hsave, $new);
if ($ok === FALSE) break; // or e("Error writing file: $file$extension");
}
$ok = $ok === FALSE ? FALSE : TRUE; // ok could be valid 0bytes
$ok = $ok && ((bool)$str || feof($hread)); // check if fread and feof failed
$ok = $ok && fclose($hread) && fclose($hsave); // ALWAYS check for fclose!

if ($ok) $ok = rename($file.$extension, $file); // overwrite $file, and delet

Solution

First of all, your function name sounds weird. The name of the function should brefly describe what it does, so, a name like convertEol would be more appropriate.

Second, I see much redundancy on your code, like this part:

if ($hread === FALSE)   e("Bad file in file_eol(): $file");
if (!is_resource($hread) || get_resource_type($hread) !== 'stream') e("Bad hread in file_eol(): $hread");


As you can see on the PHP manual of fopen function, it will return false if it is not able to open de file, otherwhise, it will ALWAYS return a resource, no matter what (except that you are doing something really crazy, like opening more files than a process is allowed to do).

So, your second verification is redundant.

Also, returning booleans in functions that do not check for some condition it too old-fashioned, too 80's. The same way, triggering uncatchable errors is almost a crime.

You should really learn how to work with Exceptions.

Here is what I would do, not necessarily the only one correct implementation:

function convertEol($fileName, $eolChar) {
    /* For windows, unix and mac.
     * IMPORTANT: for this implementation, \r\n MUST be the first element on the array
     */
    static $replaceableChars = array("\r\n", "\n", "\r"); 

    $originalFile, $tmpFile;

    try {
        $originalFile = new SplFileObject($fileName, 'r'); // open a file for reading
    } catch (RuntimeException $e) {
        throw new Exception("Unable to open file {$fileName} for reading", null, $e);
    }

    try {
        $tmpFile = new SplFileObject(uniqid() . '.tmp', 'w+'); // open or create a file for writing
    } catch (RuntimeException $e) {
        throw new Exception("Unable to create temp file", null, $e);
    }

    // iterates over the original file, line by line
    foreach ($originalFile as $line) {
        // writes the replaced string
        $tmpFile->fwrite(str_replace($replaceableChars, $eol, $line));
    }

    $tmpFile->fflush(); // makes sure file was written

    rename($tmpFile->getPathname(), $originalFile->getPathname());
}


Usage:

// Normal
convertEol('path/to/file', "\n");

// Unexistent file
convertEol('path/to/unexistent/file', "\n"); // will throw an exception

// Catching error
try {
    convertEol('path/to/unexistent/file', "\n");
} catch (Exception $e) {
    echo 'Oops, something went wrong:' . $e->getTraceAsString();
}
// ...

Code Snippets

if ($hread === FALSE)   e("Bad file in file_eol(): $file");
if (!is_resource($hread) || get_resource_type($hread) !== 'stream') e("Bad hread in file_eol(): $hread");
function convertEol($fileName, $eolChar) {
    /* For windows, unix and mac.
     * IMPORTANT: for this implementation, \r\n MUST be the first element on the array
     */
    static $replaceableChars = array("\r\n", "\n", "\r"); 

    $originalFile, $tmpFile;

    try {
        $originalFile = new SplFileObject($fileName, 'r'); // open a file for reading
    } catch (RuntimeException $e) {
        throw new Exception("Unable to open file {$fileName} for reading", null, $e);
    }

    try {
        $tmpFile = new SplFileObject(uniqid() . '.tmp', 'w+'); // open or create a file for writing
    } catch (RuntimeException $e) {
        throw new Exception("Unable to create temp file", null, $e);
    }

    // iterates over the original file, line by line
    foreach ($originalFile as $line) {
        // writes the replaced string
        $tmpFile->fwrite(str_replace($replaceableChars, $eol, $line));
    }

    $tmpFile->fflush(); // makes sure file was written

    rename($tmpFile->getPathname(), $originalFile->getPathname());
}
// Normal
convertEol('path/to/file', "\n");

// Unexistent file
convertEol('path/to/unexistent/file', "\n"); // will throw an exception

// Catching error
try {
    convertEol('path/to/unexistent/file', "\n");
} catch (Exception $e) {
    echo 'Oops, something went wrong:' . $e->getTraceAsString();
}
// ...

Context

StackExchange Code Review Q#42135, answer score: 7

Revisions (0)

No revisions yet.