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

Drop vfsStream prefix from most classes? #213

Open
12 tasks
mikey179 opened this issue Feb 18, 2020 · 3 comments
Open
12 tasks

Drop vfsStream prefix from most classes? #213

mikey179 opened this issue Feb 18, 2020 · 3 comments
Assignees
Milestone

Comments

@mikey179
Copy link
Member

mikey179 commented Feb 18, 2020

PR #212 got me thinking about whether we want to keep the vfsStream prefix for class names? This stems from a time when there weren't namespaces in PHP. As we just changed the namespace anyway and are on the road for a new major release it might be an idea to drop this prefix from all classes except the vfsStream class itself. Some of the classes introduced after having a namespace were already named without that prefix. It would also allow to improve some of the class names. Here's a list of the class names that might change, I marked the ones which are currently declared as being part of the public API:

  • vfsStreamContent => Content
  • vfsStreamAbstractContent => AbstractContent
  • vfsStreamContainer => Container
  • vfsStreamContainerIterator => ContainerIterator
  • API vfsStreamBlock => vfsBlock
  • API vfsStreamDirectory => vfsDirectory
  • API vfsStreamFile => vfsFile
  • vfsStreamWrapper => StreamWrapper
  • vfsStreamAbstractVisitor => AbstractVisitor
  • API (not marked on the class but on constructor) vfsStreamPrintVisitor => Printer
  • API (not marked on the class but on constructor) vfsStreamStructureVisitor => StructureInspector
  • API (not marked on the interface but used in methods declared as part of the public API) vfsStreamVisitor => vfsStreamVisitor

I wouldn't change the vfsStream class itself. What do yo think?

@mikey179 mikey179 self-assigned this Feb 18, 2020
@bizurkur
Copy link
Contributor

That'd be a nice improvement 👍

@allejo
Copy link
Member

allejo commented Feb 19, 2020

I'm mostly in favor of this. For anything that's a public API, I'd lean towards leaving some type of prefix in the class name. Introducing a class with such a generic name like Directory or File would cause users to have to set aliases in their use statements if they're using real filesystem classes with names like File or Directory. So for public API classes, what about leaving a prefix like vfs? e.g.

  • vfsDirectory
  • vfsFile

@mikey179
Copy link
Member Author

Introducing a class with such a generic name like Directory or File would cause users to have to set aliases in their use statements if they're using real filesystem classes with names like File or Directory.

You are right, that's an important point. I updated the list to reflect this. For the concrete Visitor implementations however I don't think it's a big issue, so I'd keep my suggestion.

@mikey179 mikey179 added this to the 2.0.0 milestone Feb 21, 2020
mikey179 added a commit that referenced this issue Mar 5, 2020
This introduces the class name changes proposed with #213 into the v1.x series according to the migration strategy detailed in  #221.
mikey179 added a commit that referenced this issue Mar 23, 2020
* introduce class name changes in new namespace

This introduces the class name changes proposed with #213 into the v1.x series according to the migration strategy detailed in  #221.

* fix various type hints
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

3 participants