search

SAtacker / bb-config

Configure your beagle device
MIT License
41 stars 7 forks source link

Refactor & several improvement. #13

Closed ArthurSonzogni closed 3 years ago

SAtacker commented 3 years ago

What do you think of beagle_config_utils namespace instead of connman_handler , command_handler , which will help reduce redundant code? I would use the modularized UI implementations to implement basic stuff and the rest would go to beagle_config_utils

ArthurSonzogni commented 3 years ago

In my opinion, I wouldn't create the two directory: connman_handler and command_handler.

It seems to me you were about to separate your project by UI vs logic. I believe it is better to split by components and keep the UI and the logic of each component together. Those are usually tied, but components are independent.

I would keep the logic of wireless and internet connexion sharing within their own component. Currently inside the directories:

--

Adding many namespace isn't really useful. You just want to avoid your symbols not to clash with other libraries you depends on or are depending on you. As long as your project isn't big enough, splitting your project with many different namespace doesn't look useful to me. Namespace isn't really a way to stay "organized".

Of course, those are just my opinions, feel free to proceed the way you like and iterate/change opinions over time your project evolve.

SAtacker commented 3 years ago

I totally agree with the recommendation you have. Yes, I was initially going with UI vs logic but it seems like I am building some redundant code like shell_helpers. I would implement the logic in the component itself as it's manageable in the same place.

SAtacker commented 3 years ago

@ArthurSonzogni , I am stuck solving https://pastebin.com/6jSur9zE