3
\$\begingroup\$

I'm making a web API using web API 2. I have three layers:

  • Business - Stores models and service classes.
  • Data - Just EF. Currently one DB. But will include 3 when complete.
  • Web API - Presentation layer.

I'm not making a UI to consume the API at the moment. The intention is for other internal apps to be able to consume the data.

I'm starting with the concept of an "Asset." The web APi has an asset controller that calls the appropriate methods in the asset service.

[RoutePrefix("api/v1/Assets")]
public class AssetController : ApiController
{
    public AssetService AssetService { get; set; } = new AssetService();

    [Route("{assetId}")]
    public async Task<IHttpActionResult> GetAsset(Guid assetId)
    {
        var asset = await AssetService.GetAssetById(assetId);

        if (asset == null)
            return NotFound();

        return Ok(asset);
    }
    [Route("RepairFrequencyLengths")]
    public async Task<IHttpActionResult> GetRepairFrequencyLengthList()
    {
        return Ok(await AssetService.GetRepairFrequencyLengthList());
    }
}

The main route is api/v1/Assets. So when they want to grab the main asset info, they call it like this: api/v1/Assets/00000000-0000-0000-0000-000000000000. Later I'll set up other routes so they can call detailed data like this: api/v1/Assets/00000000-0000-0000-0000-000000000000/identifiers and so on. Does that look correct?

One of the things I'm worried about is the structure of the business layer, and whether or not I'm using services correctly.

Here is what the structure of the business layer looks like:

enter image description here

Here is the code for the asset service class:

public class AssetService
{
    public async Task<Asset> GetAssetById(Guid assetId)
    {
        using (var db = new EdgeContext())
        {
            var assets = await (from asset in db.vwAssets
                                join customer in db.tblCustomers on asset.CustomerId equals customer.CustomerId
                                join customerlocation in db.tblCustomerLocations on asset.CustomerLocationId equals customerlocation.CustomerLocationId
                                join product in db.tblProducts on asset.ProductId equals product.ProductId
                                join taglocation in db.tblTagLocations on asset.TagLocationId equals taglocation.TagLocationId into groupjoinTagLocation
                                from leftjoinedtaglocation in groupjoinTagLocation.DefaultIfEmpty()
                                where asset.AssetId == assetId
                                select new { asset, customer, customerlocation, product, leftjoinedtaglocation }).ToListAsync();

            return assets.ToList().Select(a => new Asset(a.asset, a.customer, a.customerlocation, a.product, null, a.leftjoinedtaglocation)).FirstOrDefault();
        }
    }

    public async Task<List<RepairFrequencyLength>> GetRepairFrequencyLengthList()
    {
        return await Task.Run(() => new List<RepairFrequencyLength>()
        {
            new RepairFrequencyLength(){ DisplayMember = "Day(s)", Value = "D" },
            new RepairFrequencyLength(){ DisplayMember = "Months(s)", Value = "M" },
            new RepairFrequencyLength(){ DisplayMember = "Year(s)", Value = "Y" },
        });
    }
}

I use EF to grab the asset table and associated tables, then pass those tables to the constructor of the asset class to populate the data. The asset class looks like this:

public class Asset : IEntityModel
{
    public Guid AssetId { get; set; }
    public string Title { get; set; }
    public string FullModelNumber { get; set; }
    public int? RepairFrequencyValue { get; set; }
    public string RepairFrequencyLength { get; set; }
    public Customer.Customer Customer { get; set; }
    public CustomerLocation CustomerLocation { get; set; }
    public Product.Product Product { get; set; }
    public TagLocation TagLocation { get; set; }
    public bool Deleted { get; set; }
    public DateTime? CreatedDate { get; set; }
    public DateTime UpdateDate { get; set; }
    public string UpdatedBy { get; set; }

    public Asset() { }

    public Asset(vwAsset asset = null, tblCustomer customer = null, tblCustomerLocation customerLocation = null, tblProduct product = null, tblBrand brand = null,
                    tblTagLocation tagLocation = null)
    {
        InitializeAsset(asset);
        InitializeCustomer(customer);
        InitializeCustomerLocation(customerLocation);
        InitializeProduct(product, brand);
    }

    private void InitializeAsset(vwAsset asset)
    {
        if (asset != null)
        {
            AssetId = asset.AssetId;
            Title = asset.Title;
            FullModelNumber = asset.FullModelNumber;
            RepairFrequencyValue = asset.RepairFrequencyValue;
            RepairFrequencyLength = asset.RepairFrequencyLength;
            CreatedDate = asset.CreatedDate;
        }
    }
    private void InitializeCustomer(tblCustomer customer)
    {
        if (customer != null)
            Customer = new Customer.Customer(customer);
    }
    private void InitializeCustomerLocation(tblCustomerLocation customerLocation)
    {
        if (customerLocation != null)
            CustomerLocation = new CustomerLocation(customerLocation);
    }
    private void InitializeProduct(tblProduct product, tblBrand brand)
    {
        if (product != null)
            Product = new Product.Product(product, brand);
    }
    private void InitializeTagLocation(tblTagLocation tagLocation)
    {
        if (tagLocation != null)
            TagLocation = new TagLocation(tagLocation);
    }
}

Does all of this look right? I wanted to limit the number of layers I used so it wouldn't be too complex. A lot of the extra layers I see sometimes seems like abstracting that information for not much other reason than to just abstract it. So I want to avoid that if possible. But at the same time, I want to make sure I'm doing things the right way so I don't cause headaches for myself or others down the line.

Any help is appreciated.

\$\endgroup\$
1
\$\begingroup\$

Datacontext is already a repository pattern + to me you are using the repository as a service?

All in all it seems good, just change the name of asset repo to service. In that service, make more use of AsQueryable and parameters ( eg. ListAssets( page, pageSize, searchterm)

Personally, I also like extensions for readability of code eg. An extension ToPage(this ,Queryable assets,int page, int pageSize) bit that's subjective

Also, do a ToList after the select, this will improve the sql query generation. ToList triggers a DB call

\$\endgroup\$
4
  • \$\begingroup\$ Yeah, I think I just copied it from an example and just rolled with it. If I'm not mistaken, I'm just using them as services anyway. So I'll rename the class. \$\endgroup\$ – ernest Mar 14 '17 at 18:40
  • \$\begingroup\$ Yeah, it seems like you are using them as services, although the naming seemed off ;) I do advise to stop blocking async with await Task.Run(() => ;) \$\endgroup\$ – NicoJuicy Mar 17 '17 at 11:35
  • \$\begingroup\$ I had to add the await Task.Run(() => ;) or VS would give me a warning stating that it wouldn't run async. Do you have an alternative? \$\endgroup\$ – ernest Mar 17 '17 at 15:08
  • \$\begingroup\$ Instead of .ToList() you should be able to run .ToListAsync() and place await in front of it. So --> await myQuery.ToListAsync(); Not sure if you can by heart, i see you already used ToListAsync in the first method \$\endgroup\$ – NicoJuicy Mar 20 '17 at 14:34

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.