current community

your communities

more stack exchange communities

company blog
Stack Exchange Inbox Reputation and Badges
tour help
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

Sign up
Here's how it works:
  1. Anybody can ask a question
  2. Anybody can answer
  3. The best answers are voted up and rise to the top
up vote 3 down vote favorite

The GetDropdownList() function is being called on a couple of controllers based on this example and it may be called in many other controllers, so this code is being repeated. I was thinking of encapsulating this portion of code to the base class and call that base class in these controllers, but I got stuck and don't know how to implement it. Can you please show me a way using SOLID principles?

public interface IDropdownListEntity
{
    string Value { get; set; }
    string Text { get; set; }
}

public sealed class DropdownListEntity: IDropdownListEntity
{
    public string Value { get; set; }
    public string Text { get; set; }
}

public class CountryController : ApiController
{
    //Returns the list of countries
    public IEnumerable<IDropdownListEntity> GetDropdownList()
    {
        CountryStore store = new CountryStore();

        return store.SimpleSortedListByName<DropdownListEntity>();
    }
}

public class CityController : ApiController
{
    //Returns the list of cities
    public IEnumerable<IDropdownListEntity> SortedListForDropdown()
    {
        CityStore store = new CityStore();

        return store.SimpleSortedListByName<DropdownListEntity>();
    }
}
share|improve this question
    
Do your stores share an interface or base class? – Andy Wiesendanger Oct 22 '15 at 15:47
    
@Andy: yes, the stores are inherited from a base class and that base class is inherited from couple of interfaces. I think, I know what you are thinking based on your question but I will not spill water onto your thoughts... Lol – Coder Absolute Oct 23 '15 at 1:11
    
In fact, I did try to use one of the interface which is plugged into store's base class but that didn't work for me... Maybe I did it the wrong way.. Am a beginner! – Coder Absolute Oct 23 '15 at 1:14
    
I have also thought of using Reflection, but not sure again... – Coder Absolute Oct 23 '15 at 1:16
up vote 1 down vote accepted

  1. Web API should not be bound to a particular UI

    API calls should not bound to a user interface requirement, so that it could be used in multiple places without any addition requirement. In the above code, it is returning a list of dropdownlist values rather than a list of entities.

    Your controller code may look like this:

    public class ConstantsController : ApiController
{
    private readonly IConstantRepository _constantRepository;

    public ConstantsController(IConstantRepository constantRepository)
    {
        _constantRepository = constantRepository;
    }


    [Route("Constants/City")]
    [HttpGet]
    public IEnumerable<City> GetCity()
    {
        return _constantRepository.GetCity();
    }


    [Route("Constants/Country")]
    [HttpGet]
    public IEnumerable<Country> Country()
    {
        return _constantRepository.GetCountry();
    }
}

    Note: I am using Web API 2 for this code.

    This code can be used by any kind of application like this:

    http://yoururl/Constants/Country

  2. Use a DI container to inject dependencies into your controller

    Use Unity or a simple injector to inject dependencies. It will help you to isolate components.

    public interface IConstantRepository
{
    IEnumerable<City> GetCity();

    IEnumerable<Country> GetCountry();
}

Also, include unit tests in your applications.

share|improve this answer
    
How the the implementation of _constantRepository.GetCity() and _constantRepository.GetCountry() would look like? – Coder Absolute Oct 23 '15 at 5:51
    
It should call your db and get the data. If you are using Entity Framework , it would look something like _dbContext.City.Get() . – paritosh Oct 23 '15 at 5:54
    
So, you are suggesting me to group all the constants together, maybe into a single repository called 'Constants'? – Coder Absolute Oct 23 '15 at 6:08
    
Yes , If it is a master list , you can group those in one class , It is up to your design requirement. – paritosh Oct 23 '15 at 6:10
    
Ok. The reason I have chosen to implement IDropdownListEntity is because I thought it's light weight and transfer less bytes to the client, meaning the entity has only (Text and Value properties). But, if I return the original entity it may contain a lot of other fields and that's going to be irrelevant in this particular case, meaning values for other fields would be transferred to the client as null (using Json). Maybe, I should rephrase the (Text and Value) to (Id, Name) or there is something wrong with my thought process? – Coder Absolute Oct 23 '15 at 6:17

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.