I have a set of services that are used to read/write to the database. These get injected into my controllers:
ISystemSettingService _systemSettingService = null;
IStatusTypeService _statusTypeservice = null;
public MyController(
ISystemSettingService systemSettingService,
IStatusTypeService statusTypeservice)
{
_systemSettingService = systemSettingService;
_statusTypeservice = statusTypeservice;
}
So when I need something from _systemSettingService
I can easily get it. Now I also have some static helper classes/functions which I call from MyController
. In these functions I often need access to the services so I can access the DB. e.g. I have a Validate(string userData, ISystemSettingService systemSettingService)
function that accepts some data passed in from the user.
So far, into the Validate
function I have been passing in the _systemSettingService
so I can use it. I'm not sure this is correct.
My question - is my approach correct or, in the Validate
function should I be creating a unity container to Resolve
an instance of my ISystemSettingService
or, as I have been reading about, should my helper class NOT be static, and I should inject ISystemSettingService
into the constructor which will apparently make unit testing easier.
I'm a bit confused! Thanks.
2 Answers 2
Your questions sounds a little bit confused about Validate
being a function or a class, but for sake of simplicity, let us assume it is a function.
If I got you right, you want to make sure your Validate
function uses internally the same ISystemSettingService
object as MyController
. Passing it as a parameter like
Validate(userData, _systemSettingService)
at all places where is Validate
is called is surely a working approach. However, by making Validate
non-static, you can actually reach the same goal in a more readable and less error-prone fashion. So if all calls to the static version of Validate
would use _systemSettingService
as a second parameter, with no exceptions, then making it non-static and saving the second parameter is IMHO the better alternative.
Usage of a DI container for the helper function will probably make the code longer, less readable, more complex and error prone, since you have to make sure the Resolve
method will return the same systemSettingService
object provided to MyController
. IMHO it is better to use a DI container exclusively where it clearly makes things simpler.
In case Validate
is a method of a helper class Validator
, the same measures apply: passing the ISystemSettingService
object directly to the method is not inherently wrong, but passing it to the constructor of Validator
and making Validate
non-static will probably result in more maintainable code.
-
My babbling meant to say
Validate
is a static function in a static class. Suppose I made the Helper class non-static, without DI then I would still have to have an instance of the_systemSettingService
where ever I used the helper class - something that I may not necessarily have without passing the instance all over the place - would this scenario make you use DI? If yes, how would I create the helper class as and when needed in the code?Percy– Percy2018年02月03日 15:55:19 +00:00Commented Feb 3, 2018 at 15:55 -
@Rick: a non-static helper class which was formerly static can most probably constructed in the constructor of
MyController
and assigned to a new member variable_validator
. So instead of writingValidator.Validate(userData, _systemSettingService)
you will have to write_validator.Validate(userData)
.Doc Brown– Doc Brown2018年02月03日 22:18:11 +00:00Commented Feb 3, 2018 at 22:18
Now I also have some static helper classes/functions
This is where you are going wrong.
Normally as in @docbrowns answer I would recommend having a non static class with a construction parameter, much the same as your controller.
However! I notice the name of you class implies (to me at least) that the values provided by ISystemSettingService
are unchanging setup parameters for you application?
In this case I would go further and try to eliminate the service altogether. Loading this type of settings from the app.config file and injecting them directly as the appropriate value type into services that require them.
My feeling on this issue is that although providing config settings dynamically from a service or database does give you a 'control panel' for your deployed application. You are much better in the long run having these settings managed by deployment. This gives an audit trail and more surety around how exactly a deployed service will be running, without having to take into account network connectivity, caching or other errors
-
Hi, they are likely never to change but the user wants to be able to change them if desired, so they need to be in the database. I think you're correct about the non-static class. I guess anything that depends on a service, for example, should not be static.Percy– Percy2018年02月03日 15:49:54 +00:00Commented Feb 3, 2018 at 15:49