I have read the proposed changes and I agree in principle, although the suggested re-factoring feels a bit too extensive.
>> remove a few interfaces which are not actually used, i.e. they've been introduced because various classes have methods with similar signatures, but there's no real concept behind I disagree with the characterization that they do not have a "real concept behind them" they represent contracts of entities that have similar attributes. IMO, all the various *XxxHolder*(s) represent an entity that provides whatever these "attribute" interfaces hold. This allows for a level of abstraction that I find very useful - not to mention enforcing certain behavior (as interfaces do). I can go along with perhaps reducing some of them, but certainly not most of them. Furthermore, since they are very "narrow" interfaces, they allow users to provide an implementation that does not "drag" in a bulky object. E.g., if one defines a method as accepting a *ClientSessionHolder* it simply tells the user that it will need a *ClientSession* - regardless of where it originates - and there are many such holders in our code. By removing these "markers" we would be forced to un-necessarily ask for "bulkier" objects than necessary. These interfaces makes mocking, proxying, overriding behavior etc.. - as well as useful containers for default and static helper methods that are common too all such holders. >> remove the usage of java.nio.channels.Channel which is overused Agree in principal, but IMO we should keep it for any "pipe" that carries information or provides API(s) for it since this is what a *Channel* is (IMO). Certainly sessions, channels, streams, SCP/SFTP clients, SFTP opaque file handles, etc... satisfy this condition. It is also a very convenient standard way to provide a *Closeable* interface + *isOpen()* indicator. I do agree though that perhaps it has been used in contexts other than "data pipes" and in these cases a better definition should be provided - however, let's not remove its usage altogether without discriminating between the valid cases and the invalid ones. >>> introduce an ExecutorService, extending the usual java.util.concurrent.ExecutorService and the org.apache.sshd.common.Closeable interfaces and refactor all the usage to use this interface instead of conveying both the ExecutorService along with 100% in favor Cheers, Lyor aaa
