Getting price for one firm by week

Question

I've a lot of data whit prices of a product from different firms by week. I've a class named WeekPrice witch contains a list of firms. Here you've my class structure.

Here you've a little part of the data in JSON format:

[
    {
        "ID": 1,
        "NumberOfWeek": 1,
        "Year": 2016,
        "Date": "01/04/2016",
        "Firms": [
            {
                "ID": 1,
                "Name": "Firm ABC",
                "Price": 0.73
            },
            {
                "ID": 2,
                "Name": "DEF Solutions",
                "Price": 0.97
            }
        ]
    },
    {
        "ID": 2,
        "NumberOfWeek": 2,
        "Year": 2016,
        "Date": "08/04/2016",
        "Firms": [
            {
                "ID": 3,
                "Name": "Firm ABC",
                "Price": 0.83
            },
            {
                "ID": 4,
                "Name": "DEF Solutions",
                "Price": 0.94
            }
        ]
    }
]

I'll to make a graph that shows the evolution of the price for one firm by week. to this I need to get the price and the week in a object. In the C# code below DoublePoint is used for do this. The property Data is the week number and Value is the price.

List<DoublePoint> list = (from week in MyListWitData
                          where week.Year == DateTime.Now.Year
                          group week by week.NumberOfWeek into grp
                          select new DoublePoint()
                          {
                              Data = grp.Key,
                              Value = (double)(from price in grp.ToList()
                                               where price.Year == DateTime.Now.Year
                                               select (from firm in prijs.Bedrijf
                                                       where firm.Name == "Firm ABC"
                                                       select firm.Price
                                                      ).FirstOrDefault<decimal>()
                                              ).FirstOrDefault()
                          }).ToList<DoublePoint>();

It works perfectly but the only problem is that the code is a little bit messy. Can you refactor my code so it's better in performance and easier to read it?

Thanks in advance

Answer 1

The model you chose for your data is quite far from ideal. I guess that's one of the reasons your code became a little messy. However, even with this model, the end result can be reached in a more readable, and probably more efficient way:

var pricesPerWeek = weekPrices
    .Where(weekPrice => weekPrice.Year == DateTime.Now.Year)
    .SelectMany(
        (weekPrice) => weekPrice.Firms.Where(firm => firm.Name == "ABC"), 
        (week, firm) => new {week.Week, firm.Price})
    .ToList();

In this code, weekPrices is a List<WeekPrice>. This code first selects week prices from the current year. It then uses the overload SelectMany to flatten the list of week prices to a collection of anonymous objects with properties Week and Price.

A complete and working sample of this code can be found on dotnetfiddle.net.

However, as said before, your model is not ideal. I would refactor your model to three classes: Firm, Product and ProductPrice:

// Represents a firm you track.
public class Firm
{
    public Firm() 
    {
        Prices = new List<ProductPrice>();
    }

    public int Id {get;set;}
    public String Name {get;set;}
    public List<ProductPrice> Prices {get; private set;}
}

// Keeps record of the firm's price of a product at a certain moment 
public class ProductPrice
{
    public int FirmId {get;set;}
    public int ProductId {get;set;}
    public int Year {get;set;}
    public int Week {get;set;}
    public decimal Price {get;set;}
}

// Represents a product you track.
public class Product
{
    public int Id {get;set;}
    public String Description {get;set;}
}

This model has several advantages over your model. For instance, firms do not get a new Id every week. Instead, the firm's Id and Name are stored once. The same goes for products (although that isn't part of your question). All you have to do every week, is add an entry of type ProductPrice for all product-firm combinations.

And now, with this model, back to your question. To select the weeks and prices for a certain firm (and product) you can do:

var pricesPerWeek = firms
    .Where(firm => firm.Id == 1)
    .SelectMany(firm => firm.Prices)
    .Where(
        productPrice => productPrice.Year == DateTime.Now.Year && 
        productPrice.ProductId == 1)
    .Select(productPrice => new {productPrice.Week, productPrice.Price})
    .ToList();

In this code, firms is a List<Firm>. This code first selects the correct firm, it then selects the correct weeks (and product) and then returns a collection of anonymous objects with properties Week and Price.

A complete and working sample of this code can be found on dotnetfiddle.net.