+- Kodi Community Forum (https://forum.kodi.tv)
+-- Forum: Development (https://forum.kodi.tv/forumdisplay.php?fid=32)
+--- Forum: Feature Discussion (read-only) (https://forum.kodi.tv/forumdisplay.php?fid=183)
+--- Thread: Introducting Dependency Injection (/showthread.php?tid=246346)
Introducting Dependency Injection - jfcarroll - 2015-11-05
Hello all,
As we've been discussing I'm working on an example of how we can use Dependency Injection as a software design pattern to help with removing so much of the interdependency between various components of our code base.
If you want to see the progress so far you can get a snapshot of it here: https://github.com/jimfcarroll/xbmc/tree/di
After some deliberation and discussion I decided to start with the Weather functionality. The reason is that it's already somewhat well isolated.
The story so far:
Since Weather, as everything else, relies on logging, I decided to refactor logging slightly. To be able to separate Weather into it's own independently testable component I wanted to make the static methods on CLog a thin veneer over an underlying implementation.
To do this I reversed the relationship between CLog and ILogger. Instead of an ILogger implementation in terms of CLog, I use the ILogger interface (modified as needed) behind the implementation of all of the CLog static methods. This way we can, for example, set a CLog implementation for a test context that doesn't pull the user's home directory into the picture.
The current CLog implementation relies on a platform specific file management, platform specific date time, and g_advancedSettings. I'm currently in the process of trying to resolve this.
First, the 2 platform specific classes are now two (I didn't do the windows one yet ... does anyone really use windows? :-) ) implementations of ILogger. The appropriate one is compiled into the XbmcContext ( https://github.com/jimfcarroll/xbmc/blob/di/xbmc/XbmcContext.cpp ).
Of course, there's now a problem with CLog use outside of the context. I have a solution for this that can be removed once everything is managed within the context.
On the date/time functionality - I moved that to C++11 chrono functionality rather than port the platform specific code.
Now there's a major interdependency problem between the command line parser, Application, g_advancedSettings, CSpecialProtocol, main and CLog that I'm trying to resolve. I need to set and get the location of the log file and the log level settings. There is an order-of-initialization problem I'm trying to resolve.
I'm going to move the command line parsing to a passive object. Right now it calls out to the various affected components (and is therefore "active") as the command line is parsed. I'm going to create a second class that parses the command line but simply retains the values. I will move the parsing of the values one at a time to this class so the two will exist until everything affected is moved over.
Then, ultimately I'll get to the weather piece.
Any comments welcome.
Jim
RE: Introducting Dependency Injection - topfs2 - 2015-11-05
(2015-11-05, 01:38)jfcarroll Wrote: Now there's a major interdependency problem between the command line parser, Application, g_advancedSettings, CSpecialProtocol, main and CLog that I'm trying to resolve. I need to set and get the location of the log file and the log level settings. There is an order-of-initialization problem I'm trying to resolve.
I'm going to move the command line parsing to a passive object. Right now it calls out to the various affected components (and is therefore "active") as the command line is parsed. I'm going to create a second class that parses the command line but simply retains the values. I will move the parsing of the values one at a time to this class so the two will exist until everything affected is moved over.
I started cleaning up the app parser for kodi server but didn't get any help on windows, I think the windows devs were swamped with other stuff
Anyways, perhaps there is something for you here.
https://github.com/topfs2/xbmc/tree/appparser
RE: Introducting Dependency Injection - jfcarroll - 2015-11-05
Oh wow. It's absolutely the direction I was talking about. Thanks.
The CXBMCOptions in the first commit is exactly the passive command line class I was talking about. How hard would it be for you to finish this?
EDIT: I read through the entire change. It looks like exactly what I needed to do except farther along. Unless you can think of a reason not to, I'd like to take it. If you want it as a separate PR I can probably fix up the windows side.
RE: Introducting Dependency Injection - topfs2 - 2015-11-05
(2015-11-05, 14:30)jfcarroll Wrote: Oh wow. It's absolutely the direction I was talking about. Thanks.
The CXBMCOptions in the first commit is exactly the passive command line class I was talking about. How hard would it be for you to finish this?
EDIT: I read through the entire change. It looks like exactly what I needed to do except farther along. Unless you can think of a reason not to, I'd like to take it. If you want it as a separate PR I can probably fix up the windows side.
It was finished for all platforms except windows I think. I don't have a windows dev environment so can't take that up. Its missing build env additions to os x and android too but those should be minimal.
I'd be happy to fix anything if needed but windows is the main problem. For some reason the startup is completely different on windows than the rest of the platforms
RE: Introducting Dependency Injection - jfcarroll - 2015-11-05
Great. I have to set up my windows dev environment again but I'll do it and get it running. Maybe I can merge the startup techniques so there's less differences.
RE: Introducting Dependency Injection - topfs2 - 2015-11-05
(2015-11-05, 16:22)jfcarroll Wrote: Great. I have to set up my windows dev environment again but I'll do it and get it running. Maybe I can merge the startup techniques so there's less differences.
That would also be lovely, I'm guessing there is a reason for the differences but perhaps its just legacy
RE: Introducting Dependency Injection - jfcarroll - 2015-11-06
@topfs2, I took your branch and brought it up to date. I pushed it here: https://github.com/jimfcarroll/xbmc/tree/appparser
I'm not sure about a number of the configurations I can't test (android, with a breakpad (whatever that is)).
I'll need to install everything on my windows machine. Can we build under Windows 10?
RE: Introducting Dependency Injection - topfs2 - 2015-11-06
Awesome, nice!
The little I did for Android in the branch I did in the blind, I only tried Linux.
Can't help you with win 10, I sadly have no knowledge about it at all
RE: Introducting Dependency Injection - Razze - 2015-11-08
Windows 10 works without problems.
RE: Introducting Dependency Injection - jfcarroll - 2015-11-08
As soon as I get VS community edition installed. I'm going on to my 4th attempt. Constant perpetual hangs. Apparently I'm not the only one.
RE: Introducting Dependency Injection - jfcarroll - 2015-11-10
After many days of trying to install VS 2015 community edition I finally reinstalled Windows 10 from scratch and got VS installed last night.
Of course, Fallout 4 just came out so I might be a little distracted over this weekend but we'll see :-)