NeedleProject/FileIo – Step 4: Add features, small refactor, fix issue #4

The library is starting to have a usable form but is still a lot to do.

In the previous posts we have fixed 2 issues and created an example file.

Well, let’s establish our work for today:

1. add extension identification
2. fix issue #4
3. small refactors

1. Add extension identification, name and base-name get methods

We will do that by extending the current File API with 4 new methods:

• hasExtension(): bool – should tell us whether the file has an extension or not
• getExtension(): string – Should return the file’s extension
• getName(): string – Should give us the file’s name (without an extension)
• getBasename(): string – Should give us the file’s name including the extension

Because we aim to follow the TDD principle as much as possible, we first write these methods in a “dummy” state:

<?php
// [...]

class File
{
    // [...]
    /**
     * State existence of a file's extension
     * @return bool
     */
    public function hasExtension(): bool
    {
        return false;
    }

    /**
     * Get file's extension
     * @return string
     */
    public function getExtension(): string
    {
        return '';
    }

    /**
     * Get file's name without extension
     * @return string
     */
    public function getName(): string
    {
        return '';
    }

    /**
     * Get file's name with extension
     * @return string
     */
    public function getBasename(): string
    {
        return '';
    }
    // [...]
}

Now we should extend the tests so they can cover these methods:

<?php
// [...]

class FileTest extends \PHPUnit_Framework_TestCase
{
    // [...]

    /**
     * Test extension
     *
     * @param $filename
     * @param $extension
     *
     * @dataProvider provideFileAndExtension
     */
    public function testGetExtension($filename, $extension)
    {
        $file = new File($filename);
        $this->assertEquals(
            $extension,
            $file->getExtension(),
            sprintf("Expected extension %s, received %s", $extension, $file->getExtension())
        );
    }

    /**
     * Test has extension
     *
     * @param $filename
     *
     * @dataProvider provideFilesWithExtension
     */
    public function testHasExtensionTrue($filename)
    {
        $file = new File($filename);
        $this->assertTrue(
            $file->hasExtension(),
            sprintf("File %s should have extension", $filename)
        );
    }

    /**
     * Test that it does not have an extension
     *
     * @param $filename
     *
     * @dataProvider provideFilesWithoutExtension
     */
    public function testHasExtensionFalse($filename)
    {
        $file = new File($filename);
        $this->assertFalse(
            $file->hasExtension(),
            sprintf("File %s should not have an extension", $filename)
        );
    }

    /**
     * Test name retrieval
     *
     * @dataProvider provideFileAnExpectedNames
     * @param $filename
     * @param $expectedName
     */
    public function testGetName($filename, $expectedName)
    {
        $file = new File($filename);
        $this->assertEquals(
            $expectedName,
            $file->getName(),
            sprintf("Expected extension %s, received %s", $expectedName, $file->getName())
        );
    }

    /**
     * Test name retrieval
     *
     * @dataProvider provideFileAnExpectedBasenames
     * @param $filename
     * @param $expectedBasename
     */
    public function testGetBasename($filename, $expectedBasename)
    {
        $file = new File($filename);
        $this->assertEquals(
            $expectedBasename,
            $file->getBasename(),
            sprintf("Expected extension %s, received %s", $expectedBasename, $file->getBasename())
        );
    }
    // [...]

    /**
     * Provide filenames and their expected extensions
     * @return array
     */
    public function provideFileAndExtension(): array
    {
        return [
            [__FILE__, 'php'],
            ['.htaccess', 'htaccess'],
            ['name', ''],
            ['a.combined.filename.with.multiple.ext.separator', 'separator']
        ];
    }

    /**
     * @return array
     */
    public function provideFilesWithExtension(): array
    {
        return [
            [__FILE__],
            ['.htaccess'],
            ['a.combined.filename.with.multiple.ext.separator']
        ];
    }

    /**
     * @return array
     */
    public function provideFilesWithoutExtension(): array
    {
        return [
            [pathinfo(__FILE__, PATHINFO_FILENAME)],
            ['name']
        ];
    }

    /**
     * Provide filenames and their expected extensions
     * @return array
     */
    public function provideFileAnExpectedNames(): array
    {
        return [
            [__FILE__, pathinfo(__FILE__, PATHINFO_FILENAME)],
            ['.htaccess', ''],
            ['name', 'name'],
            ['a.combined.filename.with.multiple.ext.separator', 'a.combined.filename.with.multiple.ext']
        ];
    }

    /**
     * Provide filenames and their expected extensions
     * @return array
     */
    public function provideFileAnExpectedBasenames(): array
    {
        return [
            [__FILE__, pathinfo(__FILE__, PATHINFO_BASENAME)],
            [__DIR__ . DIRECTORY_SEPARATOR . '.htaccess', '.htaccess'],
            ['.htaccess', '.htaccess'],
            ['name', 'name'],
            ['a.combined.filename.with.multiple.ext.separator', 'a.combined.filename.with.multiple.ext.separator']
        ];
    }
}

As expected, running the test will fail, so, I will run the tests using –filter argument:

phpunit --filter=testHasExtensionTrue
phpunit --filter=testHasExtensionFalse

And now let’s move on to implement the logic:

1. add a private member in the class definition private $hasExtension with false as default value
2. replace hard-coded return inside ::hasExtension()
3. implement the logic that set’s the value for the $hasExtension

Now let’s show what we end up with:

<?php
// [...]

class FileTest extends \PHPUnit_Framework_TestCase
{
    /**
     * File extension separator
     * @const string
     */
    const EXTENSION_SEPARATOR = '.';

    // [...]
    /**
     * Whether the file has an extension or if it is set by us
     * @var bool
     */
    private $hasExtension = false;

    // [...]
    /**
     * File constructor.
     *
     * @param string $filename
     */
    public function __construct(string $filename)
    {
        $this->filename = preg_replace('#(\\\|\/)#', DIRECTORY_SEPARATOR, $filename);
        $fileParts = explode(DIRECTORY_SEPARATOR, $this->filename);
        $filename = array_pop($fileParts);
        if (false !== strpos($filename, static::EXTENSION_SEPARATOR)) {
            $this->hasExtension = true;
        }
    }
    // [...]

    /**
     * State existence of a file's extension
     * @return bool
     */
    public function hasExtension(): bool
    {
        return $this->hasExtension;
    }
    // [...]
}

The same “step-by-step” is procedure for the rest of the scenarios, ending up with the following implementation:

<?php
// [...]

class FileTest extends \PHPUnit_Framework_TestCase
{
    /**
     * File extension separator
     * @const string
     */
    const EXTENSION_SEPARATOR = '.';

    // [...]
    /**
     * File's extension - For no extension a blank string will be used
     * @var null|string
     */
    private $extension = null;

    /**
     * File's name without extension
     * @var null|string
     */
    private $name = null;

    /**
     * Whether the file has an extension or if it is set by us
     * @var bool
     */
    private $hasExtension = false;

    // [...]

    /**
     * File constructor.
     *
     * @param string $filename
     */
    public function __construct(string $filename)
    {
        $this->filename = preg_replace('#(\\\|\/)#', DIRECTORY_SEPARATOR, $filename);
        $fileParts = explode(DIRECTORY_SEPARATOR, $this->filename);
        $filename = array_pop($fileParts);
        if (false !== strpos($filename, static::EXTENSION_SEPARATOR)) {
            $this->hasExtension = true;
            $filenameParts = explode(static::EXTENSION_SEPARATOR, $filename);
            $this->extension = array_pop($filenameParts);
            $this->name = implode(static::EXTENSION_SEPARATOR, $filenameParts);
        } else {
            $this->name = $filename;
            $this->extension = '';
        }
    }
    // [...]


    /**
     * State existence of a file's extension
     * @return bool
     */
    public function hasExtension(): bool
    {
        return $this->hasExtension;
    }

    /**
     * Get file's extension
     * @return string
     */
    public function getExtension(): string
    {
        return $this->extension;
    }

    /**
     * Get file's name without extension
     * @return string
     */
    public function getName(): string
    {
        return $this->name;
    }

    /**
     * Get file's name with extension
     * @return string
     */
    public function getBasename(): string
    {
        if (false === $this->hasExtension()) {
            return $this->name;
        }
        return $this->name . static::EXTENSION_SEPARATOR . $this->extension;
    }
}

The downside is that File class constructor has become very ugly (the constructor should be light and straight-forward, no if-else like I’ve done, but, as I stated in the first post, it will get ugly before it will get better).

We will resolve this issue later, for now we will leave it as it is.

2. Fix issue #4

We will handle this issues using a small refactor procedure.

For the issue at hand, we should apply “move creation to Factory“.

I’ve created the tests and they can be viewed in the repository so I will skip them from the post (I guess you got the idea about how TDD works).

The factory end’s up like this:

<?php
namespace NeedleProject\FileIo\Factory;

use NeedleProject\FileIo\Content\Content;
use NeedleProject\FileIo\Content\ContentInterface;

class ContentFactory
{
    /**
     * @const string
     */
    const EXT_TXT = 'txt';

    /**
     * @const string
     */
    const EXT_JSON = 'json';

    /**
     * Create a ContentInterface based on the extension and string content
     * @param string $extension
     * @param string $content
     * @return \NeedleProject\FileIo\Content\ContentInterface
     */
    public function create(string $extension, string $content): ContentInterface
    {
        switch ($extension) {
            /**
             * Create JSON Content
             * @todo Actually create a JsonContent
             */
            case static::EXT_JSON:
                return new Content($content);
                break;
            /**
             * Default TXT Content
             */
            case static::EXT_TXT:
            default:
                return new Content($content);
        }
    }
}

And now, let’s use it in our File class. The result is shown below:

<?php
// [...]
class File
{
    /**
     * @var null|ContentFactory
     */
    private $contentFactory = null;

    // [...]

    /**
     * @return \NeedleProject\FileIo\Content\ContentInterface
     * @throws \NeedleProject\FileIo\Exception\FileNotFoundException
     * @throws \NeedleProject\FileIo\Exception\IOException
     * @throws \NeedleProject\FileIo\Exception\PermissionDeniedException
     */
    public function getContent(): ContentInterface
    {
        // [...]

        // removed this:
        // return new Content($stringContent);
        return $this->getFactory()
            ->create($this->extension, $stringContent);
    }

    // [...]

    /**
     * Returns a factory responsible for creating appropriate content
     * @return \NeedleProject\FileIo\Factory\ContentFactory
     */
    protected function getFactory(): ContentFactory
    {
        if (is_null($this->contentFactory)) {
            $this->contentFactory = new ContentFactory();
        }
        return $this->contentFactory;
    }
}

Now we can close issue #4 and continue to our next step in our list.

3. Small refactors

I highlighted that the constructor of File‘s class constructor has become a little to ugly, so, I decided to refactor it to a cleaner form.

The form I have in mind should only define the class members ($this->[variableAtHand]) and should delegate the value extractor.

For that I have created a PathHelper that should handle all this small processing data:

<?php
namespace NeedleProject\FileIo\Helper;

use NeedleProject\FileIo\File;

class PathHelper
{
    /**
     * Convert path separator to the system's default separator
     *
     * For example:
     * root\\foo/bar\ becomes root:
     * - \\foo\\bar on Windows and
     * - /root/foo/bar on Linux
     *
     * @param string $path
     * @return string
     */
    public function normalizePathSeparator(string $path): string
    {
        $path = preg_replace('#(\\\|\/)#', DIRECTORY_SEPARATOR, $path);
        return preg_replace('#' . DIRECTORY_SEPARATOR . '{2,}#', DIRECTORY_SEPARATOR, $path);
    }

    /**
     * Extract the the filename from a path
     *
     * Example: from /foo/bar/file.ext will result file.ext
     *
     * @param string $file_path
     * @return string
     */
    public function extractFilenameFromPath(string $file_path): string
    {
        $fileParts = explode(DIRECTORY_SEPARATOR, $file_path);
        return array_pop($fileParts);
    }

    /**
     * Split a filename into an array containing the name and extension
     * Example:
     *  file.ext -> [file, ext]
     *  .htaccess -> ['', htaccess]
     *  name -> [name, '']
     *
     * @param string $filename
     * @return array
     */
    public function splitFilename(string $filename): array
    {
        if (false === strpos($filename, File::EXTENSION_SEPARATOR)) {
            $extension = '';
            $name = $filename;
        } else {
            $filenameParts = explode(File::EXTENSION_SEPARATOR, $filename);
            $extension = array_pop($filenameParts);
            $name = implode(File::EXTENSION_SEPARATOR, $filenameParts);
        }

        return [
            $name,
            $extension
        ];
    }
}

By using this Helper, we end up with constructor that doesn’t do logic, it delegates it:

class File
{
    /**
     * File constructor.
     *
     * @param string $filename_with_path
     */
    public function __construct(string $filename_with_path)
    {
        $pathHelper = new PathHelper();
        $this->filenameWithPath = $pathHelper->normalizePathSeparator($filename_with_path);
        $filename = $pathHelper->extractFilenameFromPath($this->filenameWithPath);
        if (empty($filename)) {
            throw new \RuntimeException(
                sprintf('Given path %s does not represents a file!', $filename_with_path)
            );
        }
        list($this->name, $this->extension) = $pathHelper->splitFilename($filename);
        $this->hasExtension = (bool)$this->extension;
    }    
}

This approach is not the best, the __constructor should always be light and the dependencies should be injected, not constructed inside the File class, but given the fact that I do not have any other construction layer (for example, a Dependency Injection framework) this method should be sufficient.

Now, we can actually expose publicly the code by tagging an initial release version: v0.1.0.

To conclude:

• We have a small and usable library that can be installed using the instructions here
• We discussed refactoring using an example

This should be the end of the “FileIo” posts but the library will be further extended and maintained.

What I’ve tried in the past few posts is to show that nothing can be correctly planned ahead (at leas from my point of view),  tried to share how things works for me in hope of helping others organize their process, showed that any piece of code will be altered many times before it will reach a “mature” state (that will eventually will further be modified upon extension) and also that tests are not just “a thing”, their are meant to help and catch issues before they reach a point when it causes problems.

If you wish to participate, contact me or submit requests on the library’s Github page