Less global and mutable state #285
Conversation
|
I'm still hesitant to deviate the |
I agree that keeping |
ppalaga
force-pushed
the
201222-refactor
branch
3 times, most recently
from
e17fd4f to
407b615
Compare
|
I have removed the second commit for now and I have simplified the first one as much as possible. Could you please re-review? |
| private static void configure( | ||
| CliRequest cliRequest, | ||
| EventSpyDispatcher eventSpyDispatcher, | ||
| Map<String, ConfigurationProcessor> configurationProcessors) |
There was a problem hiding this comment.
Same as above...
Those methods are private and need to be given fields, so it's better to make them non static imho.
There was a problem hiding this comment.
configure is called twice with different params, so it is IMO safer not to allow reading the instance fields.
| Slf4jLoggerManager plexusLoggerManager, | ||
| Logger slf4jLogger, | ||
| ILoggerFactory slf4jLoggerFactory, | ||
| Function<CliRequest, TransferListener> createTransferListener) { |
There was a problem hiding this comment.
What's the benefit of using a static method and passing all parameters ?
There was a problem hiding this comment.
Actually now, that those fields are final, you are right, the method could be non-static without any danger that we inadvertently change the state. Let me change it.
ppalaga
force-pushed
the
201222-refactor
branch
from
407b615 to
2fc3cec
Compare
|
2fc3cec: improved what I was able to. Is this good to merge now? |
Yes |
@gnodet this should be a pure refactoring with no changes of behavior. The main intention is to make the code easier to read and easier to reason about.