gowebdav
gowebdav copied to clipboard
Allows to disable making parent collection while writing stream
Sometimes it is useful and also not necessary to call the createParentCollection(). Hence, a bool param is introduced disabling the function call.
Thanks for your feature.
We could move this to a global option/setter to make this work for all operations. Or a refactoring into a decorator that handles the recursive collection creation.
Discussions are welcome.
What do you mean by 'all operations'? If I see it right, there are only 3 occurrence of the createParentCollection().
Hence, by global option is meant an additional Client param e.g. skipcreateParents, which would be than checked before each createParentCollection() call... ?
yes. this PR captures just one call to createPatentCollection().
as a Client setter createPatentCollection(true|false) would make the feature available for all present and future write operations.
the check would be done inside createPatentCollection(path).
Hence, I've adjusted the changes based on your thoughts and suggestion. Please have a look at it.
Just my opinions:
- I don't like the variadic bool argument. I think it's not intuitive why the argument is variadic, as a user I would ask myself "why can I supply an arbitrary number of bool parameters?" I understand that you chose that way to keep compatibility, but I would rather have a new method
WriteWithCreateParentCollection(string, []byte, os.FileMode, bool)with a single bool. The currentWrite(string, []byte, os.FileMode)can then just callWriteWithCreateParentCollection(string, []byte, os.FileMode, true)to keep compatibility. (The name of the new method is debatable ;)) - I support the decision to keep the choice of creating parent collections on a per-operation basis rather than a "global" bool setting in
Client. It's more flexible this way, I think.
@ueffel thanks for the insight!
First, yes you're right about the placement of variadic arg. It is used to ensure compatibility.
Second, personally to parametrize the createParentCollection(bool) is not the prefered way for me either, I would keep it like it is now. Instead, I would parametrize the Write or any other method calling the createParentCollection() fcn to be or not to be called via a condition statement, as you pointed out.
Third and to sum up - I agree with your suggestion, I see it quite the same in respect to the point 2. Therefore, I'll modify it based on the suggestion from point 1.
I would not be in favor of more method arguments for feature toggling. For the shake of consistency, I'd prefer a global flag, it's KISS. Or some kind of Client decorator witch takes care of parent collections. The basic intent of the project is to be consistent with a file API.