View Issue Details

IDProjectCategoryView StatusLast Update
0000022KafkaEsqueImprovementpublic2019-05-24 16:56
ReporterOliver G.Assigned To 
PriorityhighSeveritytweakReproducibilityalways
Status newResolutionopen 
Product Version0.12.0 
Target VersionFixed in Version 
Summary0000022: Separate Responsibilities of the Controller
Description

The Controller class has too many responsibilities and it would be desirable to separate these responsibilities into separate classes:

1., Configuration Management (configHandler): inject the configHandler singleton whereever it is needed and try to remove/extract those parts of the code that have Feature Envy (e.g. when configHandler is used in connection with TopicMessageTypeConfig):
Map<String, TopicMessageTypeConfig> configs = configHandler.getTopicConfigForClusterIdentifier(selectedCluster().getIdentifier());
TopicMessageTypeConfig topicMessageTypeConfig = getTopicMessageTypeConfig(configs); //configHandler is also used inside this method
Map<String, String> consumerConfig = configHandler.readConsumerConfigs(selectedCluster().getIdentifier());

2., Kafka Service Layer: e.g. Number of Runnables executed by runInDaemonThread need only singletons like consumerHandlers injected, but probably do not need to be part of the JavaFx Controller

3., EventHandlers passed as parameter in setOnAction can be very likely extracted. Be aware that these EventHandlers will also benefit from 1., and 2.,

I will give prio high to this issue, since it can dramatically increase the speed of future develepmoent tasks.

TagsNo tags attached.

Activities

Oliver G.

Oliver G.

2019-03-18 21:03

reporter   ~0000027

Please check my "cleanercode" branch for possible way, how the refactoring could look like. I introduced the concept of the BackgroundRunnable and extracted TopicsForCluster as an example. At the end of the day the BackGroundTaskHolder will probably turn into some kind of BackgroundTaskManager taking this responsibility completely from the Controller. Please give me feedback to this approach. (unfortunately this commit contains a little refactoring from the develop branch, too, sorry)

https://github.com/grofoli/KafkaEsque/commit/b2bad5a671ab5f479ba5d67f9266a5b5f92351e8#diff-57f7831e57d5d46e1e68776b121d82d4

https://github.com/grofoli/KafkaEsque/commit/b2bad5a671ab5f479ba5d67f9266a5b5f92351e8#diff-e61f00380df8f263ae76df2baba743c9

Oliver G.

Oliver G.

2019-05-24 16:56

reporter   ~0000030

In order to have the ability to split the Controller class, it could be very helpful to have the KafkaesqueAdminClient injectable. Therefore I continued development on my cleanercode branch. Please check my latest changes as well: https://github.com/grofoli/KafkaEsque/commit/169b1d503b55768cff21afe62fe47d8796f61e4f

Issue History

Date Modified Username Field Change
2019-03-17 10:11 Oliver G. New Issue
2019-03-18 21:03 Oliver G. Note Added: 0000027
2019-05-24 16:56 Oliver G. Note Added: 0000030