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

Simplified dokan-java #50

Closed
hrstoyanov opened this issue Jul 22, 2020 · 2 comments
Closed

Simplified dokan-java #50

hrstoyanov opened this issue Jul 22, 2020 · 2 comments

Comments

@hrstoyanov
Copy link

@infeo Some while back I forked dokan-java here and wanted to simplify the code base for my needs. You can see that my fork has much less classes/interfaces, yet i find it still just as useful - "less is more" . I wonder if you would like such simplifications contributed back. Here are a few details:

1. DokanFileSystem/DokanFileSystemStub/AbstractDokanFileSystem removed
The problem with DokanFileSystem@friends is that is a very large and brittle interface. It essentially flattens out the many small interfaces in _DokanOperations, where you already had the correct design . Further, the way you coded this requires annotation processing (see NotImplemented), which is not good - what if a developer inherits a class from AbstractDokanFileSystem and forgets to annotate methods?

I do see however, how DokanFileSystem can separate components in the cryptomator project, and maybe they belong there. I just do not see any need for them in dokan-java, removed them in my fork and did not miss them at all!

2. I salvaged the code from AbstractDokanFileSystem
into a new class DokanyMountPoint

3. New meaning of DokanyFileSystem,
I renamed FileSystemInformation -->DokanyFileSystem

**4. Consolidated a bunch of static methods into DokanyUtils

5. Consolidated all exception into one - DokanyException

6. Added default static methods in DokanyOperations
This way the a user can provide implementations only for what they have to.

I might be forgetting some other changes I made, but overall the number of classes and API surface shrunk considerably, making dokan-java codebase easier to use/understand.

Let me know what you think?

@infeo
Copy link
Member

infeo commented Jul 25, 2020

Thanks for the interest in this library and putting thought into contributing something back!

In general I favor "less is more", but one needs to keep the focus of this project in mind: Providing an easy to use wrapper for the Dokan interface and not a 1-to-1 java API to it.

Yes, the DokanyFileSystem class is a big interface and may not be easy to implement, but one does not have to. As a normal developer you only need to extend the DokanFileSystemStub class and override the methods you need. All else will not be set in the dokan operations struct. With this architecture you also do not need to worry about implementing mount or unmount methods. I cannot see any reason to extend the AbstractDokanFileSystem directly, so also the annotation is a valid tool here. Your argument only gains weight by the fact, that the project lacks a lot of documentation and therefore the usage is not obvious.
On the other hand, by having the DokanyFileSystem, you have all methods in one place. And they are methods, not some "strange" JNA-Callback interfaces which need to be implemented.

Of course, one would like to use the "raw" dokan interface provided by Dokany, but then it may be better to directly use JNI which is much faster than this project (due to JNA). I actually struggled a lot with this question the last three months, but I think I come to a conclusion now: If a developer needs it raw, they also need to go the hard way.

You might have noticed it, currently there are even approaches to give the lazy developer a more "Java"-like interface (see PR #35 ).

  1. I salvaged the code from AbstractDokanFileSystem
    into a new class DokanyMountPoint

For this the Mount interface exists (;

**4. Consolidated a bunch of static methods into DokanyUtils

This is really something needs to be done.^^

  1. Consolidated all exception into one - DokanyException

As already said in #31, I agree with you that all exceptions that can be thrown should be at least encapsulated by one generic DokanException.


Lastly, I like to stress that this project is decoupled from Cryptomator. (really, currently Cryptomator does not use dokan-java and thinks about switching to FUSE API). It is true that I came over the Cryptomator project here and of course used my experience I collected there. But still, dokan-java is an independet project.

@hrstoyanov
Copy link
Author

@infeo thanks, I still prefer many small functional interfaces and no reflection/annotations over a big interface and reflection/annotation tricks to make it usable.
I will try to catch up to the latest codebade and see if I can ignore some of the code while use the rest. As I mentioned, removing classes and Interfaces, reducing the "API surface area" made things simpler for me.

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

2 participants