Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I am trying to model a domain of metrics for use in a BI application. I recently read the Designing with Types series, and felt that it could be useful, so I wanted to give it a try. What I'm not sure is if I'm trying too hard....

  1. The metrics are application-defined, meaning they are finite and won't change during runtime. A dozen metrics would probably be a lot.
  2. New metrics will be introduced as the application grows
  3. A user will be selecting which metrics they want to use, and the application has to take that and go "do stuff", like building a query to the backend store (MongoDB, in this case).
  4. There may be application logic associated with metrics that I would like enforced through pattern matching so that when I introduce a new metric, I want the compiler to tell me that I need to do some work.

Here's what I have:

type Metrics =
  | Revenue
  | Volume
  | PriceAsp
  with
    member m.FormatGroupings =
      match m with
        | Revenue -> [("revenue", """$sum: { "$Amount" }" """)]
        | Volume  -> [("volume", """$sum: { "$Quantity" }" """)]
        | PriceAsp -> List.append Revenue.FormatGroupings Volume.FormatGroupings

    member m.FormatProjections =
      match m with
        | Revenue -> [("revenue", "$revenue")]
        | Volume -> [("volume", "$volume")]
        | PriceAsp -> [("priceAsp", """ $divide: ["$revenue", "$volume"]  """)]


let buildQuery groupBy (metrics:Metrics list) =
  let concatenate f =
    let x = metrics
            |> List.collect f
            |> List.map (fun m -> sprintf "{%s: %s}" (fst m) (snd m) )
    System.String.Join(",", x)

  let groupings = concatenate (fun m -> m.FormatGroupings)
  let projections = concatenate (fun m -> m.FormatProjections)

  sprintf """{$group: {_id: {%s: "$%s"}}, %s}, $project: {%s}}""" groupBy groupBy groupings projections

With examples of usage:

buildQuery "foo" [Revenue; Volume]  //Simple
buildQuery "foo" [PriceAsp]         //Compound

Note that PriceAsp is "made up of" both Revenue and Volume.

Please provide any general comments. Some specific questions I have are:

  1. Have the business rules for the metric calculations be embedded within the types
  2. Show that some metrics are built off of other metrics (for example, PriceAsp), and enforce the explicit relationship. Above, we see that PriceAsp is made up of Revenue and Volume because it's in that list. But it would be better to make it enforced explicitly through typing.
  3. There are formatting rules around an "instance" of a specific metric. For example, when Revenue is shown to a user, it should be rounded to the nearest whole dollar, with thousand separators and a currency symbol (ex: $1,234,567).
  4. Is there an elegant way (using List.fold, perhaps) for doing what String.Join does? String.Join is just so convenient...

A base metric would look like this (no $project):

{
  $group: {
    _id: {
      foo: "$Foo"
    },
    revenue: { $sum: "$Amount" }
  }
}

A compound metric may look like this:

{
  $group: {
    _id: {
      foo: "$Foo"
    },
    revenue: { $sum: "$Amount" },
    volume: { $sum: "$Quantity" }
  },
  $project: {
    priceAsp: { $divide: ["$revenue", "$volume"] }
  }
}
share|improve this question
    
Could you give an example of the desired output of formatMetric for a compound metric? Also, I don't understand what do you mean by [Revenue;PriceASP], should formatMetric also accept lists of metrics? What should be output in that case? –  svick Jan 24 '13 at 10:24
    
The last output is from the compound metric. I edited the post to make that more clear. Yes, I think that formatMetric would take a list instead of just a single Metric. –  M Falanga Jan 24 '13 at 14:39
    
I have been rethinking this whole approach, and I think I have a better solution which I would like feedback on. I will update the post with that information in a few hours. –  M Falanga Jan 24 '13 at 14:41
    
So, you want formatMetric to output the whole query? Because that's not what it currently does, that's what confuses me. –  svick Jan 24 '13 at 18:15
    
@svick thanks for sticking with me... I edited the question, putting in code that better reflects what I am doing. –  M Falanga Jan 25 '13 at 1:16
add comment

2 Answers 2

Regarding your types, I think that simple metrics and compound metrics should have their own types, to enforce their requirements and to avoid repeating yourself. One way to do that is to define Metric as:

type Metric =
  | SimpleMetric of SimpleMetric
  | CompoundMetric of CompoundMetric

Here, SimpleMetric and CompoundMetric are separate discriminated unions, and both of them have members specific to their case.

The whole code could look like this:

type SimpleMetric =
  | Revenue
  | Volume
  with
    member m.Name =
        match m with
          | Revenue -> "revenue"
          | Volume -> "volume"

    member m.Grouping =
        match m with
          | Revenue -> """$sum: { "$Amount" }" """
          | Volume -> """$sum: { "$Quantity" }" """

    member m.Projection =
        "$" + m.Name

type CompoundMetric =
  | PriceAsp
  with
    member m.Name =
        match m with
          | PriceAsp -> "priceAsp"

    member m.Metrics =
        match m with
          | PriceAsp -> [SimpleMetric Revenue; SimpleMetric Volume]

    member m.Projection =
        match m with
          | PriceAsp -> """ $divide: ["$revenue", "$volume"]  """
// CompoundMetric references Metric, so the two types have to be declared together
and Metric =
  | SimpleMetric of SimpleMetric
  | CompoundMetric of CompoundMetric
  with
    member m.Name =
        match m with
            | SimpleMetric sm -> sm.Name
            | CompoundMetric cm -> cm.Name

    member m.Projection =
        match m with
            | SimpleMetric sm -> sm.Projection
            | CompoundMetric cm -> cm.Projection

    member m.FormatGroupings =
      match m with
        | SimpleMetric sm -> [(sm.Name,sm.Grouping)]
        | CompoundMetric cm -> cm.Metrics |> List.collect (fun m -> m.FormatGroupings)

    member m.FormatProjections = [(m.Name, m.Projection)]

let buildQuery groupBy (metric:Metric list) =
  let concatenate f =
    let x = metric
            |> List.collect f
            |> List.map (fun (name, value) -> sprintf "{%s: %s}" name value)
    System.String.Join(",", x)

  let groupings = concatenate (fun m -> m.FormatGroupings)
  let projections = concatenate (fun m -> m.FormatProjections)

  sprintf """{$group: {_id: {%s: "$%s"}}, %s}, $project: {%s}}""" groupBy groupBy groupings projections

Usage:

buildQuery "foo" [SimpleMetric Revenue; SimpleMetric Volume]
buildQuery "foo" [CompoundMetric PriceAsp]

The whole code is longer than your version, but I think adding new metrics will be easier this way.

Also, I don't like that I have to define Name and Projection in Metric, but I don't see a way around that.

Now, to your questions:

Any general comments?

I think the type shouldn't be called Metrics, it should be Metric, because a value of that type defines a single metric, not some collection.

Also, your concatenate function can be simplified by using pattern matching instead of fst and snd (see the above code).

Is there an elegant way for doing what String.Join does?

I don't think so. There is a String module, but it doesn't have anything like Join. (And its documentation specifically refers methods of the System.String type.)

share|improve this answer
    
I like the distinction between Simple and Compound. Good point on the concatenate - much more readable. If the formatting for the query were separate from the types, like @Grundoon suggests, then maybe Name and Projection go away. I think what I'm struggling with now is if all the operations should be with the type, or separated out in other modules. –  M Falanga Jan 26 '13 at 16:14
add comment

Do you have only a limited number of metric types?

In which case, you might want to make a union case of the possible types. The compound cases can be recursive, referring to the type.

type Metric = 
    | Revenue of string * string 
    | Volume  of string * string
    | Compound  of string * Metric list

Then your formatMetric function needs to handle each case separately

let formatMetric metric =
    match metric with
    | Revenue (label,formula) -> 
        sprintf "%s: {%s}" label formula
    | Volume  (label,formula) -> 
        sprintf "%s: {%s}" label formula
    | Compound (label,metrics) -> 
        sprintf "%s: {%s}" label "other metrics"

Note that that if you use this approach you don't really need the label any more, as the cases are the label.

type Metric = 
    | Revenue of string 
    | Volume  of string
    | Compound  of Metric list

And then you could also make formatMetric recursive, printing all the child formulas for a compound formula

let rec formatMetric metric =
    match metric with
    | Revenue formula -> 
        sprintf "Revenue={%s}" formula
    | Volume  formula -> 
        sprintf "Volume={%s}" formula
    | Compound metrics -> 
        let childFormats = 
            metrics 
            |> List.map formatMetric
            |> List.reduce (+) 
        sprintf "Compound: {%s}" childFormats

The formatting is not exactly what you want, but I'm sure you get the general idea.

EDIT

General Comment

Personally, I wouldn't add those member functions to the type. I would focus on designing the types in their own module independent of any particular representation, and then have a separate module that mapped them into the appropriate representation for MongoDB. That way you are not mixing up different requirements.

String join.

A simple string join is trivial to write:

let stringJoin sep list :string = 
    let append s t = s + sep + t
    list |> List.reduce append

stringJoin ", " ["a"; "b"; "c"; ]

Separate the types from the representation

But if you separate the types from the representation, you may find that you naturally create MongoDB specific functions that generate the appropriate representation. The idea is to build up a mini-language that you can compose together to make bigger blocks. I don't know the MongoDB syntax, but maybe something like this?

let mSum p = sprintf "{ $sum: ""%s"" }" p
let mDivide p q = sprintf "{ $divide: [""%s"", ""%s""] }" p q
let mLabel p q = sprintf "%s : %s" s
let mGroup id ps = sprintf "$group: { %s, %s }" id (stringJoin ", " ps)
let mProject p q = sprintf "$project: { %s }" (stringJoin ", " ps)

If you are doing a lot of this, you might be better off using a JSON library. In which case the workflow would be:

domain type ==>  convert to intermediate type for Mongo API ==> convert to JSON
share|improve this answer
    
This is more inline with my original thinking. I think the article I linked to in the original question made my try too hard. I edited the question with this updated code. I would be interested in your feedback on that. thanks –  M Falanga Jan 25 '13 at 1:18
    
I really like the thought behind this answer. Distilling the types down to "regular" and "compound" is good. Regarding my question #2, could I do PriceAsp of [Revenue;Volume]? The separation of building the query from the type makes a lot of sense. –  M Falanga Jan 26 '13 at 15:52
add comment

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.