I put the global line in purely for consistency with your original code. If the original code is all there is, that line is unnecessary. You only use $directoryName in this scope and the calling method. The return handles the calling method. If you have additional code that relies on $directoryName being set, then I think that @pacmaninbw's answer @pacmaninbw's answer is right: you should use a class rather than a series of functions with global variables to connect them.
I put the global line in purely for consistency with your original code. If the original code is all there is, that line is unnecessary. You only use $directoryName in this scope and the calling method. The return handles the calling method. If you have additional code that relies on $directoryName being set, then I think that @pacmaninbw's answer is right: you should use a class rather than a series of functions with global variables to connect them.
I put the global line in purely for consistency with your original code. If the original code is all there is, that line is unnecessary. You only use $directoryName in this scope and the calling method. The return handles the calling method. If you have additional code that relies on $directoryName being set, then I think that @pacmaninbw's answer is right: you should use a class rather than a series of functions with global variables to connect them.
Consider changing the name of readFile to something like segmentFile. Because it doesn't just read the file. I wouldn't expect something named readFile to write new files.
function closeWriteFile($file)
{
fclose($file);
}
What's the purpose of this? You're just rewriting a function call as...a function call. If this were in a class and $file was an object field, then it would make more sense. But as it is, there doesn't seem any reason why closeWriteFile($file) is any better than fclose($file).
The same issue with segmentContent. You are abstracting something that doesn't need it. Just say fwrite($writeHandle, $line);. No need for additional complexity. That just hides what you are actually doing, making the code harder to read.
The other functions make a little more sense, but not that much. Generally, you want to make functions from a sequence of operations, not just as aliases for a single operation. For example, you could take
$directoryName = explode('.',$FILE)[0];
and
//Creates a directory on the file name to store segmeted chapters from $FILE
global $directoryName;
mkdir($directoryName);
$file_handle = fopen($FILE, "r");
chdir($directoryName);
and rewrite this as
$file_handle = fopen($FILE, "r");
global $directoryName;
$directoryName = explode('.', $FILE)[0];
changeToDirectory($directoryName);
with
function changeToDirectory($directoryName)
{
if (! file_exists($directoryName))
{
mkdir($directoryName);
}
chdir($directoryName);
return $directoryName;
}
I put the global line in purely for consistency with your original code. If the original code is all there is, that line is unnecessary. You only use $directoryName in this scope and the calling method. The return handles the calling method. If you have additional code that relies on $directoryName being set, then I think that @pacmaninbw's answer is right: you should use a class rather than a series of functions with global variables to connect them.
Even this function isn't really necessary. The code is actually shorter without it. But it does abstract out a feature that you could reuse, even if you don't now.
$matchTag = False;
while (!feof($file_handle))
{
$line = fgets($file_handle);
if(checkingMatchTag($line))
{
if($matchTag){
closeWriteFile($writeHandle);
incrementCounter();
}
else
{
$matchTag = True;
}
$writeHandle = openWriteFile();
segmentContent($writeHandle, $line);
}
elseif($matchTag)
{
segmentContent($writeHandle, $line);
}
}
You don't need $matchTag. Because PHP is weakly typed, you can instead say
global $TAG;
$count = 0;
$extension = '.' . explode('.', $FILE)[1];
$writeHandle = false;
while (!feof($file_handle))
{
$line = fgets($file_handle);
if (preg_match($TAG, $line))
{
if ($writeHandle)
{
fclose($writeHandle);
$count++;
}
$writeHandle = fopen($directoryName . $count . $extension, 'w');
}
if ($writeHandle)
{
fwrite($writeHandle, $line);
}
}
Again, if you don't need to use $count other than the code you posted, you don't need it to be global. If you do use it elsewhere, then a class would handle it in a more reusable way.
The $TAG global might better be a function parameter, so you'd say something like segmentFile($FILE, '/x-berschrift-1--nur-f-r-Header-/');.
In PHP, double quotes allow variable interpolation and single quotes don't. So I tend to use single quotes for any string where I don't have to use double quotes.
This also gets rid of the helper functions. Other than a little setup, it doesn't increase the length of this section of code. Calculating $extension beforehand saves recalculating it every time you see a match.
You can actually get rid of the if statements as well if you like.
while (!feof($file_handle))
{
$line = fgets($file_handle);
if (preg_match($TAG, $line))
{
$writeHandle = fopen($directoryName . $count . $extension, 'w');
fwrite($writeHandle, $line);
break;
}
}
while (!feof($file_handle))
{
$line = fgets($file_handle);
if (preg_match($TAG, $line))
{
fclose($writeHandle);
$count++;
$writeHandle = fopen($directoryName . $count . '.' . $extension, 'w');
}
fwrite($writeHandle, $line);
}
There is some duplication of code in that but it saves checking that it's not the first match on each iteration.