Introduce FileLockService.Locking to leverage try-with-resource-blocks #2853
Conversation
| @@ -20,11 +20,34 @@ | |||
| */ | |||
| public interface FileLockService { | |||
|
|
|||
| interface Locking extends AutoCloseable { | |||
There was a problem hiding this comment.
Why do we need a new interface and can not simply use the existing FileLocker one?
There was a problem hiding this comment.
As said above, actually the FileLocker interface is not necessary anymore with this auto-closable style.
And from the name FileLocker describes something that can lock something (in this case a file), the implementation of Locking represent a locked state (it could also be named LockedFile or similar). Therefore I would just remove FileLocker and add something that reflects the state representation better and just extends AutoClosable.
That being said, I mainly introduced Locking just because AutoClosable throws an Exception. But using Closable instead only throws an IOException which is already handled in almost all callers anyway.
For now I removed the FileLocker interface entirly and only kept FileLockerImpl as an implementation detail of FileLockServiceImpl that is queried in the FileLockServiceTest. But if there would be a real isLocked checked in the test, even FileLockServiceImpl could become a hidden internal class of FileLockServiceImpl, probably stripped down to the minimum.
There was a problem hiding this comment.
This interface originally used the Equinox resource API and was somehow driven by what this API offers, so from my side if Tycho does not use this functionality anymore it could be removed.
HannesWell
force-pushed
the
simplifyLocking
branch
3 times, most recently
from
96d7bbc to
4713547
Compare
This allows to simplify the usage of the FileLocker/FileLockingService
to:
try (var locked = fileLockService.lock(fileToProtect)) {
// do something
}
Remove FileFlocker and simplify FileLockServiceImpl and FileLockerImpl.
HannesWell
force-pushed
the
simplifyLocking
branch
from
4713547 to
4654a3c
Compare
This allows to simplify the usage of the FileLocker/FileLockingService to:
try (var locking = fileLockService.lock(fileToProtect)) {
// do something
}
With this changes the FileLocker interface is only used in tests. Therefore my suggestion is to remove it completely and just use the
FileLockService.Lockinginstead.