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

Add a factory class for Directory objects. #6561

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jbroadus
Copy link
Contributor

Create a DirectoryFactory class that is owned by the Library or Device objects
and provided to methods that need to create Directory objects. This will allow
information to be provided through the Directory object. For example:

  • If the directory is on a removable device.
  • A mount point that may change on future uses.
  • Whether paths should be relative or absolute.

@jbroadus
Copy link
Contributor Author

Consider this an RFC. This is a pattern that isn't really used anywhere else in the code base, as far as I can tell. An alternative would be to pass around a non-factory object (DirectoryManager?) that can be queried for this information when a Directory is created.

@jbroadus
Copy link
Contributor Author

Also, if you're trying to stabilize a 1.4 release, this is probably 1.4.1.

@jbroadus jbroadus force-pushed the library-factory branch 3 times, most recently from 0538a7d to 4e4036e Compare February 11, 2020 20:37
@jbroadus
Copy link
Contributor Author

Renamed DirectoryFactory -> DirectoryManager to be more general.

Create a DirectorManager class that is owned by the Library or Device objects
and provided to methods that need to create Directory objects. This will allow
information to be provided through the Directory object. For example:
- If the directory is on a removable device.
- A mount point that may change on future uses.
- Whether paths should be relative or absolute.
@@ -61,12 +62,12 @@ ConnectedDevice::ConnectedDevice(const QUrl& url, DeviceLister* lister,
model_ = new LibraryModel(backend_, app_, this);
}

ConnectedDevice::~ConnectedDevice() {}
ConnectedDevice::~ConnectedDevice() { delete directory_manager_; }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not to use the unique_ptr?

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

Successfully merging this pull request may close these issues.

2 participants