Introducting Dependency Injection
#1
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...ontext.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
Reply
#2
(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 Tongue

Anyways, perhaps there is something for you here.
https://github.com/topfs2/xbmc/tree/appparser
If you have problems please read this before posting

Always read the XBMC online-manual, FAQ and search the forum before posting.
Do not e-mail XBMC-Team members directly asking for support. Read/follow the forum rules.
For troubleshooting and bug reporting please make sure you read this first.

Image

"Well Im gonna download the code and look at it a bit but I'm certainly not a really good C/C++ programer but I'd help as much as I can, I mostly write in C#."
Reply
#3
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.
Reply
#4
(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 Smile
If you have problems please read this before posting

Always read the XBMC online-manual, FAQ and search the forum before posting.
Do not e-mail XBMC-Team members directly asking for support. Read/follow the forum rules.
For troubleshooting and bug reporting please make sure you read this first.

Image

"Well Im gonna download the code and look at it a bit but I'm certainly not a really good C/C++ programer but I'd help as much as I can, I mostly write in C#."
Reply
#5
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.
Reply
#6
(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 Smile
If you have problems please read this before posting

Always read the XBMC online-manual, FAQ and search the forum before posting.
Do not e-mail XBMC-Team members directly asking for support. Read/follow the forum rules.
For troubleshooting and bug reporting please make sure you read this first.

Image

"Well Im gonna download the code and look at it a bit but I'm certainly not a really good C/C++ programer but I'd help as much as I can, I mostly write in C#."
Reply
#7
@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?
Reply
#8
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 Sad
If you have problems please read this before posting

Always read the XBMC online-manual, FAQ and search the forum before posting.
Do not e-mail XBMC-Team members directly asking for support. Read/follow the forum rules.
For troubleshooting and bug reporting please make sure you read this first.

Image

"Well Im gonna download the code and look at it a bit but I'm certainly not a really good C/C++ programer but I'd help as much as I can, I mostly write in C#."
Reply
#9
Windows 10 works without problems.
Reply
#10
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.
Reply
#11
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 :-)
Reply

Logout Mark Read Team Forum Stats Members Help
Introducting Dependency Injection0