Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

General notes on usage #283

Open
uuf6429 opened this issue Jan 14, 2023 · 0 comments
Open

General notes on usage #283

uuf6429 opened this issue Jan 14, 2023 · 0 comments

Comments

@uuf6429
Copy link

uuf6429 commented Jan 14, 2023

First of all, I went for this package because the gillion other vfs composer packages seem to be unmaintained or not very mature.

I got the feeling that this library mixes up global/static scope with instances a bit.
For example, looking at the provided sample, I thought vfsStream::setup() would return a vfsStream object, not a vfsDirectory.
After looking at the code, it became obvious: in reality there is only one stream wrapper being registered.

Also, storing the created vfsDirectory instance in a (private) property causes an IDE warning that it seems to not be used (only written once). While it's not a problem, the warning does kinda make sense - there are many ways in PHP to avoid an early deconstruction/dereferencing of objects.

While static classes might be handier to work with, they do encourage the habit of writing bad code - so I'd recommend a singleton approach is a bit better than static. This approach might also open up the possibility of multiple isolated vfs wrappers.

Here's what I imagine it to look like:

class MyClassTest extends TestCase
{
    public function testThatSomethingWorks(): void
    {
        $vfs = VfsStream::setUp();
        $sut = new MyClass($vfs());     // $vfs() is just a shortcut for url() - since I find that url is an implementation
                                        // detail of vfs. There are probably more idiomatic alternatives, like ->get() or so,
                                        // whereas the __invoke approach is just a handy shortcut.

        $sut->doSomething();

        $this->assertFileExists($vfs('file-created-by-myclass.txt'));
    }

    public function testThatMultipleVfsWorks(): void
    {
        $vfs1 = VfsStream::setUp();
        $vfs2 = VfsStream::setUp();
        file_put_contents($vfs1('test.txt'), 'hello world');

        rename($vfs1('test.txt'), $vfs2('test2.txt'));

        $this->assertFileDoesNotExist($vfs1('test.txt'));
        $this->assertFileExists($vfs2('test2.txt'));
    }

This is just my two cents, library is already totally usable as it is.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant