Static Helper Class vs Dependency Injection

H F
H F
Vote:
 

Hi guys, 

very simple question, 

I had a static helper class that provides for me the Content References for some of the most used pages in the solution. 

Then other developer said, let' not use it but create an interface and class implementation with ServiceConfiguration  attrubute. And later on use it as a constructor injection to get the instance.

As far as I understand there is nothing wrong in the static helper class when i am not referencing none of the shared variables.. which in my case should not as I only get the references to the instances of the pages.. 

Where the approach with the dependency injection seems to me an overkill..  

In any way, just would like to hear the opinions of fellow EpiServer developers on this subject. Thank you

By the way, why Injected<T> is anti-pattern?

#200871
Jan 29, 2019 20:14
Vote:
 
  1. Just to check your not storing the GUID references in your static class? Are these just properties that locate these pages? With about a decade of CMS experience I would never ever hardcode a page reference in any CMS platform, you should put them in a configuration section and load them from there so they can always be updated.
  2. In my opinion there should be no static classes apart from extension methods. Separating out to a service arcitecture support better separation and also the ability to using mocking for unit tests. If any services you unit testing relied on this static class but you were swapped out the data layer you would not be able. So for testing it's always better to seprate
  3. Injected<T> is an anti pattern as it uses ServiceLocation under the hook which is bad. You should try to always use constructor injection, this will also identify issues with circular dependancies. 
#200890
Jan 30, 2019 10:31
Vote:
 

There are several problems with Injected<T> and ServiceLocator - mostly they hide the dependencies from outside. Also they make the class almost impossible to test (unless you mess with the service registration, and I can tell you that can be very messy)

I kinda disargee with Scott on 2. point. static helper classes can be useful if they only serve as true "static", i.e. no state. Of course that's when you have to decide between static helper class and singleton, but that's pretty much your taste. If your static helper class does not prevent its consumer classes from being unit tested (mocking etc.) then it's fine for me. 

#200899
Jan 30, 2019 12:19
Vote:
 

Let me clarify, if a static class just had a load of constants in it or extension methods that's acceptable. But I wouldn't personally want any methods in static classes, I would want them all in services and injected. Our solutions used to be littered with static helpers and they were a mess to test. 

#200900
Jan 30, 2019 12:24
Vote:
 

Doing dependency injection and making helper classes instead of writing C# code in razor views may seem like overkill to some developers.

However, when developing Episerver solitions, we want to make sure our code works by writing unit tests, in addition to manual testing.

For example, when showing pages in the main navigation, it's not enough to do

_contentLoader.GetChildren<PageData>(ContentReference.StartPage).ToList().

We need to make sure we're not returning:

  • pages without templates (container pages)
  • unpublished pages
  • restricted pages
  • pages that have VisibleInMenu set to false

This is something that developers should test by mocking IServiceLocator, ITemplateResolver, IPublishedStateAssessor etc.

However, unit-testing may be difficult if the business logic is located inside static classes, razor views, etc.

When it comes to mocking ServiceLocator and Injected<T>... it takes just two lines of code:

var serviceLocatorMock = new Mock<IServiceLocator>();
ServiceLocator.SetLocator(serviceLocatorMock .Object);

However, mocking internal episerver stuff can be quite tricky.

#200917
Jan 30, 2019 15:19
H F
Vote:
 

Great guys, 

thank you for the clarification.

#201074
Feb 04, 2019 18:16
Vote:
 

there is a trick to avoid ServiceLocator usage in extension methods to get access to the service required. You create extension method on the service. For example, not this:

pageData.GetSomeFancyExtenralUrl(..)

static GetSomeFancyExtenralUrl(this ContentData page, ...)
{
    var resolver = ServiceLocator.Current.GetInstance<UrlResolver>();
    ...
}

but you actually make an extension on `UrlResolver` instead:

resolver.GetSomeFancyExtenralUrl(pageData, ..)

static GetSomeFancyExtenralUrl(this UrlResolver resolver, ContentData page, ...)
{
    ...
}

and now you just have to solve the issue how you are going to get to the instance of the `UrlResolver` class. usually either you can go with `Injected<T>`, demand in constructor or do any other of the black magic to get access to.

#201156
Edited, Feb 07, 2019 11:37
* You are NOT allowed to include any hyperlinks in the post because your account hasn't associated to your company. User profile should be updated.