I'm looking for guidance/review on an approach that I've taken.
Structure
- Windows Service
- Shopify Module (API)
- Official Shopify API
The Scenario Our Windows Service connects to the Shopify Module which in turn connects to the Shopify API - I chose to build a custom Micro Service pattern where as the e-commerce module (Shopify module + others) pulls down/maps a common set of objects, which ultimately is pushed into our data module.
Everything works perfectly and the Windows Service doesn't need to know where the data is coming from, just that it needs to conform to a common set of objects.
Snippet (Method)
As described, the Windows Service will check some configuration, call a module, which maps into data module objects, to pass to the data module.
public List<Models.Model.DataAPI.Order> ParsedOrders()
{
//Get orders- deserializes into shopify objects
var orders = Orders().Where(x => x.Order_Number != _identity.LastOrderNumber);
var dataApiOrders = new List<Models.Model.DataAPI.Order>();
//Map/Parse orders into readable format for the data api
foreach (var item in orders)
{
////Randomly generate key as, a shipping line can be null and it needs to be unique passing up to the data api
var shipping_method_id = Guid.NewGuid().ToString();
//Get Shipping Total
var shippingLineItems = item.Shipping_Lines.FirstOrDefault();
if (shippingLineItems != null)
{
if (string.IsNullOrWhiteSpace(shippingLineItems.Price))
{
shippingLineItems.Price = "0";
}
shippingLineItems.Id = shipping_method_id;
}
else
{
//Generate fake as shopify sometimes passes null line item
shippingLineItems = new Models.Model.Shopify.ShippingLineItem();
shippingLineItems.Title = "No Shipping Method";
shippingLineItems.Id = shipping_method_id;
shippingLineItems.Price = "0";
}
//Randomly generate key as shopify does not create a unique id for shipment
var shipment_id = Guid.NewGuid().ToString();
//Map shipping method
var dataApiShippingMethod = Models.Mapping.DataAPI.MapShippingMethod.ModelToEntityCollection(shippingLineItems);
//Map Data Api Shipments
var dataApiShipments = Models.Mapping.DataAPI.MapShipment.ModelToEntityCollection(item.Shipping_Address, shippingLineItems.Id, shipment_id);
//Map DataApiCustomer
var dataApiCustomer = Models.Mapping.DataAPI.MapCustomer.ModelToEntity(item.Customer);
var dataApiproducts = new List<Models.Model.DataAPI.Product>();
var productLineItems = item.Line_Items.OrderByDescending(x => x.Price);
foreach (var l in productLineItems)
{
//Code Omitted, needs to call shopify again because the core request does not fetch image or variance
//Map product into data api collection
var dataApiProductModel = Models.Mapping.DataAPI.MapProduct.ModelToEntity(l, shipment_id, mainImageSRC);
dataApiproducts.Add(dataApiProductModel);
}
//Map order offers
var dataApiOrderOffers = Models.Mapping.DataAPI.MapOrderOffer.ModelToEntityCollection(item.Discount_Codes);
//Get complete mapped order
var dataApiOrder = new Models.Model.DataAPI.Order()
{
order_number = item.Order_Number,
order_created_at = item.Created_At,
total_shipping_price = Decimals.Parse(shippingLineItems.Price),
currency = item.Currency,
total_price = Decimals.Parse(item.Total_Price),
total_tax = Decimals.Parse(item.Total_Tax),
billing_address = new Models.Model.DataAPI.BillingAddress()
{
city = (item.Billing_Address != null) ? item.Billing_Address.City : "",
first_name = (item.Billing_Address != null) ? item.Billing_Address.First_Name : "",
last_name = (item.Billing_Address != null) ? item.Billing_Address.Last_Name : "",
address1 = (item.Billing_Address != null) ? item.Billing_Address.Address1 : "",
address2 = (item.Billing_Address != null) ? item.Billing_Address.Address2 : "",
phone = (item.Billing_Address != null) ? item.Billing_Address.Phone : "",
zip_postalcode = (item.Billing_Address != null) ? item.Billing_Address.Zip : "",
},
customer = dataApiCustomer,
shipping_methods = dataApiShippingMethod,
order_offers = dataApiOrderOffers,
shipments = dataApiShipments,
products = dataApiproducts
};
dataApiOrders.Add(dataApiOrder);
}
return dataApiOrders;
}
To reiterate, I call the Shopify API, it then deserializes into Shopify objects, which I then map into data module objects. The Windows Service receives the data module objects which are pushed to the data module.
2 Answers 2
Many people suggest using var, but I still prefer using type. Because I could see immediately what those variable type is.
And to simplify this code, since you are using a lot of mapping between object. I would suggest you to get some object to object mapper. I have been using Automapper to design one MVVM application. Be warned though this is not fast compare to manually mapping those objects (manually = millisecond, mapper = ten or hundred millisecond).
Automapper basically work by configuring your type to type map. Then you could just call the method Mapper.Map<destinationType>(objectToMap)
(this one line is all needed to get that destinationType object). It support same property name map or built everything using your on action.
This is what it looks like if you use automapper.
public List<Models.Model.DataAPI.Order> ParsedOrders()
{
var orders = Orders().Where(x => x.Order_Number != _identity.LastOrderNumber);
var dataApiOrders = new List<Models.Model.DataAPI.Order>();
foreach (var item in orders)
dataApiOrders.Add(Mapper.Map<Models.Model.DataAPI.Order>(item));
return dataApiOrders;
}
I do want to give you a complete example, but I didn't know most of that object type.
-
\$\begingroup\$ Hi, I appreciate your answer but unfortunately speed is vital thus why I'm not using AutoMapper. I have used in the past and works tremendously well but again speed is vital, and anywhere I can improve the speed is a must. \$\endgroup\$Tez Wingfield– Tez Wingfield2017年05月19日 07:40:13 +00:00Commented May 19, 2017 at 7:40
-
\$\begingroup\$ @TezWingfield just curious, why didn't you separate those syntax into its own method? For example create a function that will return fake ShoppingLineItems. This can be put into its own partial class. \$\endgroup\$Ariwibawa– Ariwibawa2017年05月22日 06:16:22 +00:00Commented May 22, 2017 at 6:16
-
\$\begingroup\$ I suppose something I neglected, Still needs a clean up. \$\endgroup\$Tez Wingfield– Tez Wingfield2017年05月22日 08:15:33 +00:00Commented May 22, 2017 at 8:15
-
\$\begingroup\$ @TezWingfield do you have any measurements that would make Automapper look bad? I'm using it here and there so I'm a little bit curious. \$\endgroup\$t3chb0t– t3chb0t2017年08月17日 14:59:05 +00:00Commented Aug 17, 2017 at 14:59
From top to bottom:
var orders = Orders().Where(x => x.Order_Number != _identity.LastOrderNumber);
I see two things here. Orders()
is named like a property but it is actually a method. Make it a property or name it as a method (using a verb). order
variable name does not capture the meaning of that query, either move it to a separate method (GetAllOrdersButLastOne()
or something similar) or change variable name to make it clear. It's an important information you don't want to bury inside the query predicate.
// Map/Parse orders into readable format for the data api foreach (var item in orders)
If you need the comment about what code is doing then probably you need to extract a method for that. Consider, for example, something like this:
foreach (var order in orders)
dataApiOrders.Add(MapOrderToDataApi(order));
You actually can do better:
dataApiOrders.AddRange(orders.Select(MapOrderToDataApi));
The same is true for the inner loop over productLineItems
and the other big blocks of code.
city = (item.Billing_Address != null) ? item.Billing_Address.City : "",
Can be simplified:
city = item.Billing_Address?.City ?? "",
This kind of mapping is tedious to write, error-prone and painful to maintain. If you really can't use an OOM (as Ariwibawa suggested) because you have a measured performance issue then you may consider to use a T4 transformation to generate this code. Nice example: http://t4-editor.tangible-engineering.com/blog/blog-series-model-driven-development-with-t4-templates-part-3.html