3
\$\begingroup\$

I am building an e-commerce website. I have an application layer service called SearchService. SearchService receives a search request from the view through HomeSearchRequestViewModel (home page search request view model). Then it maps the request to Elasticsearch request entity and sends the search request to the search engine.

Elasticsearch performs the search and returns the result. The result from ES is a flattened (denormalized object). I use a factory to instantiate my Ad Summary View Model from ES result.

public class SearchService : ISearchService
{
    private ISearchClient _searchClient;

    public SearchService(ISearchClient searchClient)
    {
        _searchClient = searchClient;
    }

    public HomeListViewModel Search(HomeSearchRequestViewModel searchRequest)
    {
        // map search request View Model to Elasticsearch request entity
        HomeESRequestMapper esRequestBuilder = new HomeESRequestMapper(searchRequest);
        var elasticsearchRequest = esRequestBuilder.GetESRequest();

        // do the search 
        ESResultContainer esResultContainer = _searchClient.SearchDocuments(elasticsearchRequest);

        // map ES result to view model            
        List<IAdSummary> adSummaries = AdSummaryFactory.CreateAdSummaryInstance(esResultContainer.AdResults);

        // put the summary View Models in HomeListViewModel container
        HomeListViewModel homeList = new HomeListViewModel(searchRequest.MainQuery)
        {
            SearchResult =
            {
                AdSummaries = adSummaries,
                ServerError = esResultContainer.ServerError
            },
            HomeSearchRequest = searchRequest
        };

        return homeList;
    }
}

This is ESResultContainer, which is a container for ES Result:

public class ESResultContainer
{
    public ESResultContainer(string serverError = "")
    {
        AdResults = new List<AdResult>();
        ServerError = serverError;
    }

    public List<AdResult> AdResults { get; set; }

    public string ServerError { get; set; }
}

AdResult is the denormalized model which is returned from ES:

[ElasticsearchType(Name = "ad")]
public class AdResult
{
    public AdResult()
    {
        Distance = -1;
    }

    [Text(Index = false)]
    public int Distance { get; set; }

    public long Id { get; set; }

    public long AdChangeTrackerId { get; set; }

    public bool IsActive { get; set; }

    [Text(Analyzer = "english")]
    public string Title { get; set; }

    public short AdDurationInDays { get; set; }

    public DateTime AdStartTime { get; set; }

    [Text(Analyzer = "english")]
    public string Description { get; set; }

    public string MainPhotoUrl { get; set; }

    public int ChildCategoryId { get; set; }

    [Text(Analyzer = "english")]
    public string ChildCategoryName { get; set; }

    public string Controller { get; set; }

    public int ParentCategoryId { get; set; }

    [Text(Analyzer = "english")]
    public string ParentCategoryName { get; set; }

    public GeoLocation GeoLocation { get; set; }

    public string Locality { get; set; }

    public string Area { get; set; }

    public decimal Price { get; set; }

    public decimal Rent { get; set; }

    public DateTime AvailableFrom { get; set; }

    public short NoOfBedrooms { get; set; }

    public short NoOfBathrooms { get; set; }

    public bool PetFriendly { get; set; }

    public bool Furnished { get; set; }

    public string Make { get; set; }
}

This is the static factory, for instantiating my Ad Summery View Models. Each Search Result object has a Controller property. The controller determines the type of View Model that this object belongs to. For Example if Controller = "Car" then I would have to instantiate a CarAdSummaryViewModel (which implements IAdSummary).

 // I am using static because I want to maintain the list of AdSummary types in memory, so I won't have to reload it from the disk everytime
public static class AdSummaryFactory
{
    private static readonly Dictionary<string, Type> AdSummaryTypes;

    // when factory initializes, I load all the types (ad summary view models) which implement IAdSummary and keep it in memory
    static AdSummaryFactory()
    {
        // load ad summary types
        AdSummaryTypes = new Dictionary<string, Type>();

        IEnumerable<Type> typesInThisAssembly = Assembly.GetExecutingAssembly().GetTypes();

        foreach (Type type in typesInThisAssembly)
        {
            if (type.GetInterface(typeof(IAdSummary).ToString()) != null)
            {
                AdSummaryTypes.Add(type.Name.ToLower(), type);
            }
        }
    }

    public static List<IAdSummary> CreateAdSummaryInstance(List<AdResult> esResults)
    {
        List<IAdSummary> adSummaries = new List<IAdSummary>();
        foreach (var esResult in esResults)
        {
            // choose type of Ad Summery based on controller name
            type = GetTypeToCreate(esResult.Controller);

           // use AutoMapper to map ES result to the correct AdSummay View Model Type
            var adSummary = Mapper.Map(esResult, typeof(AdResult), type);
            adSummaries.Add((IAdSummary)adSummary);
        }

        return adSummaries;
    }

    private static Type GetTypeToCreate(string controller)
    {
        string adSummaryTypeName = controller.ToLower() + "summaryviewmodel";

        if (AdSummaryTypes.TryGetValue(adSummaryTypeName, out Type type))
        {
            return type;
        }

        throw new Exception($"{adSummaryTypeName} type was not found, or it does not implement IAdSummary.");
    }
}

This is how my AdSummery interface look like:

public interface IAdSummary
{
    string GetMainPhoto();

    string GetTitle();

    string GetLocation();

    string GetMonetaryValue();

    string GetLink();

    string GetHighlight();
}

My View Models implement this interface... they have a hierarchical structure. They all inherit from: AdBaseSummaryViewModel

public abstract class AdBaseSummaryViewModel : IAdSummary
{
    public AdBaseSummaryViewModel()
    {
        Distance = -1;
    }

    public long AdBaseId { get; set; }

    public string Title { protected get; set; }

    public int ChildCategoryId { protected get; set; }

    public string ChildCategoryName { protected get; set; }

    public int ParentCategoryId { protected get; set; }

    public string ParentCategoryName { protected get; set; }

    public string Controller { protected get; set; }

    public string MainPhotoUrl { protected get; set; }

    public string Locality { protected get; set; }

    public string Area { protected get; set; }

    public int Distance { protected get; set; } // distance from searched location, if any

    public string GetMainPhoto()
    {
        return MainPhotoUrl;
    }

    public string GetTitle()
    {
        return Title;
    }

    public string GetLocation()
    {
        if (Distance == 0)
        {
            Distance = 1;
        }

        if (Distance > 0 && Distance < 100)
        {
            return "< " + Convert.ToString(Distance) + " km";       // < 5 km
        }

        if (!string.IsNullOrEmpty(Locality))
        {
            return Locality;
        }

        return Area;
    }

    public abstract string GetMonetaryValue();

    public string GetLink()
    {
        return $"{Controller}/Display/{AdBaseId}";
    }

    public virtual string GetHighlight()
    {
        return string.Empty;
    }
}

Now each concrete Advertisement type, e.g. Car, RealEstate, etc inherit from the above base call (which implements IAdSummery), as an example this is CarSummaryViewModel:

public class CarSummaryViewModel : AdBaseSummaryViewModel
{
    public CarSummaryViewModel()
        : base()
    {
    }

    public decimal Price { protected get; set; }

    public string Make { protected get; set; }

    public override string GetMonetaryValue()
    {
        return Price.ToString("N"); // 1590.99 => 1,590.99
    }
}

I would appreciate any feedback on the above logic... one thing that I am unsure of, is the usage of static factory (I am using a static factory as it can keep object types in memory, to avoid disk IO every time)... I did think about replacing this static factory with a DI container, but could not think of any solution better that the existing factory.

\$\endgroup\$
  • 2
    \$\begingroup\$ Why do you think that this factory is better the a DI container? Could you explain what Mapper and AdResult are? These types seem to be missing. \$\endgroup\$ – t3chb0t Oct 31 '18 at 21:42
  • \$\begingroup\$ @t3chb0t, thanks. Sorry, let me clarify myself... I don't prefer this factory to DI, I just couldn't think of a better solution using DI. I will update the question... \$\endgroup\$ – Hooman Bahreini Oct 31 '18 at 21:45
  • \$\begingroup\$ that factory can still be abstracted and the implementation make use of the static initialization. \$\endgroup\$ – Nkosi Oct 31 '18 at 21:48
  • \$\begingroup\$ @Nkosi: thanks. I am not following, could you please explain how to use abstract factory? Do you mean use IAdSummaryFactory in the Service? How could I use the static initialization? \$\endgroup\$ – Hooman Bahreini Oct 31 '18 at 21:59
  • \$\begingroup\$ static constructor is invoked first time an instance is initialized. \$\endgroup\$ – Nkosi Oct 31 '18 at 22:04
4
\$\begingroup\$

That class can still be abstracted

public interface IAdSummaryMapper {
     List<IAdSummary> MapAdSummaries(List<AdResult> esResults);
}

The search service can then explicitly depend on the abstraction and not be tightly couple to static implementation concerns.

public class SearchService : ISearchService {
    private ISearchClient _searchClient;
    private readonly IAdSummaryMapper summaryMapper;

    public SearchService(ISearchClient searchClient, IAdSummaryMapper summaryMapper) {
        _searchClient = searchClient;
        this.summaryMapper = summaryMapper;
    }

    public HomeListViewModel Search(HomeSearchRequestViewModel searchRequest) {
        // map search request View Model to Elasticsearch request entity
        HomeESRequestMapper esRequestBuilder = new HomeESRequestMapper(searchRequest);
        var elasticsearchRequest = esRequestBuilder.GetESRequest();

        // do the search 
        ESResultContainer esResultContainer = _searchClient.SearchDocuments(elasticsearchRequest);

        // map ES result to view model
        List<IAdSummary> adSummaries = summaryMapper.MapAdSummaries(esResultContainer.AdResults);

        // put the summary View Models in HomeListViewModel container
        HomeListViewModel homeList = new HomeListViewModel(searchRequest.MainQuery) {
            SearchResult = {
                AdSummaries = adSummaries,
                ServerError = esResultContainer.ServerError
            },
            HomeSearchRequest = searchRequest
        };

        return homeList;
    }
}

The IAdSummaryMapper implementation can still make use of the static members. Assuming that Mapper is from Automapper, you should also use the instance API that has a IMapper abstraction.

public class AdSummaryMapper: IAdSummaryMapper {
    private static readonly Dictionary<string, Type> AdSummaryTypes;

    static AdSummaryMapper() {
        // load ad summary types
        AdSummaryTypes = new Dictionary<string, Type>();

        IEnumerable<Type> typesInThisAssembly = Assembly.GetExecutingAssembly().GetTypes();

        foreach (Type type in typesInThisAssembly) {
            if (type.GetInterface(typeof(IAdSummary).ToString()) != null) {
                AdSummaryTypes.Add(type.Name.ToLower(), type);
            }
        }
    }

    public AdSummaryMapper(IMapper mapper) {
        this.mapper = mapper;
    }

    public List<IAdSummary> MapAdSummaries(List<AdResult> esResults) {
        List<IAdSummary> adSummaries = new List<IAdSummary>();
        foreach (var esResult in esResults) {
            // choose type of Ad Summery based on controller name
            type = GetTypeToCreate(esResult.Controller);
            var adSummary = mapper.Map(esResult, typeof(AdResult), type);
            adSummaries.Add((IAdSummary)adSummary);
        }
        return adSummaries;
    }

    private static Type GetTypeToCreate(string controller) {
        string adSummaryTypeName = controller.ToLower() + "summaryviewmodel";

        if (AdSummaryTypes.TryGetValue(adSummaryTypeName, out Type type)) {
            return type;
        }

        throw new Exception($"{adSummaryTypeName} type was not found, or it does not implement IAdSummary.");
    }
}

The instance class can still have a static constructor that will be invoked the first time an instance of the AdSummaryMapper class is initialized.

\$\endgroup\$
  • \$\begingroup\$ Thanks a lot Nkosi, so by making factory non-static it can be injected... by using static member and constructor, I can take advantage of in-memory types. Thanks a lot for AutoMapper tip too. To be honest, I asked this question on stackoverflow which has led to this code review... I see the benefit of injecting the factory as you have shown above... but am slightly confused about it. \$\endgroup\$ – Hooman Bahreini Oct 31 '18 at 22:26
  • 1
    \$\begingroup\$ @Hooman I think you are too attached to the factory term. That class acts just like the mapper used from Automapper. You are basically mapping one type to another \$\endgroup\$ – Nkosi Oct 31 '18 at 22:39
  • \$\begingroup\$ Great explanation \$\endgroup\$ – Hooman Bahreini Oct 31 '18 at 22:45

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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