Single responsibility principle in API

Question

Please have a look at following piece of code:

public interface ICultureService
{
     List<Culture> GetCultures();
     bool IsCultureSupported(Culture culture);
     Culture GetFallbackCulture();
}

We found that most of the consumers first call IsCultureSupported to validate if their culture is supported or not. And if culture is not supported, they call GetFallbackCulture():

public CallingMethod()
{
     if(!cultureManager.IsCultureSupported(currentCulture))
     {
          currentCulture=cultureManager.GetFallbackCulture();
     }
     .
     .
     .
}

As per Single Responsibility Principle (and other OOPs rules), is it ok to introduce a function (in ICultureService and its implementation) like:

function GetFallbackCultureIfInvalid(Culture culture)
{
     if(this.IsCultureSupported(culture)
     {
          return this.FallbackCulture();
     }
}

You should fix it in your question (and add indentation). +++ In general, I'd suggest not taking any principle too far. It's a `ICultureService`, so the SRP tells me "it does all culture management (and nothing else)". It doesn't prohibit me from adding methods. It only forbids dealing with unrelated issues directly. — maaartinus, Feb 17 '17 at 16:36
@Pragmatic You seem to be an old enough member to know that you can upvote and accept answers that have helped you. Please leave a comment on my answer if you need further clarifications. To be honest, this question has a lot of missing information but I have tried to give you a reasonable answer with all the missing information. — Chetan Kinger, Feb 21 '17 at 07:21

Chetan Kinger · Answer 1 · 2017-02-17T16:24:45.487

0

As per Single Responsibility Principle (and other OOPs rules), is it ok to introduce a function (in CultureManager) like:

What you are referring to is called the Tell-Don't-Ask principle rather than the Single Responsibility Principle. Adding the GetFallbackCultureIfInvalid function actually makes the client code more readable. You should also reduce the visibility of the IsCultureSupported so that this method is no longer visible to the client code.

That said, it looks like CultureManager is an implementation of CultureService so it doesn't make sense to add a new method named GetFallbackCultureIfInvalid in CultureManager which is not a part of CultureService interface. What you should do is stick to a single method called GetFallbackCulture in CultureManager and let it return a fall back culture if the required condition is met :

Culture GetFallbackCulture(Culture culture) {
    Culture fallBackCulture = culture;
    if(!this.IsCultureSupported(culture) {
      fallBackCulture = this.FallbackCulture();
    } 

    return fallBackCulture;
}

edited Feb 17 '17 at 16:24

answered Feb 17 '17 at 14:51

Chetan Kinger

15,069
6
45
82

CKing, you are right, CultureManager will implement ICultureServerice and this method will be added to both of them. Method that you suggest does not fulfil the requirement, we need to pass culture and check if that is supported, if not return the supported one. – Pragmatic Feb 17 '17 at 16:23
thanks for the answer but I am not convinced with your method GetFallbackCulture, as the name suggest that this will return FallbackCulture which is not always the case. It can return the same culture (which is passed to it and not the fallback one). – Pragmatic Feb 25 '17 at 06:24
@Pragmatic Suit yourself. If I was the API designer, I would have a single method and document it's behavior as *Gets a `FallbackCulture` if the `culture` is not supported. Returns the `culture` argument otherwise.*. Rarely will you find API developers creating methods with names `getSomethingIfInvalid`. – Chetan Kinger Feb 25 '17 at 07:37

Single responsibility principle in API

1 Answers1