Bring Formatters and their options closer together

[PragTob]

Right now, there is a disconnect between defining the formatters and their respective arguments/configuration options. Formatters are defined in a list as formatters: [&function1, &function2] while their eventual arguments end up in formatter_options: [namespace: [actual: "option"]]. I define the formatters in one place, but I configure them in another. This seems suboptimal to me.

Possible solution

( I am very open to other suggestions, this is the "best" I came up with by myself so far)

The tuple

Instead of just a function (or a module ideally after #53) allow a tuple of a function/module and options. When calling the formatters the options are passed as a second argument after the suite.

formatters: [fn(suite) -> IO.puts "still works" end, {&HTML.output/2, file: "some.html", other: :option}, {HTML, file: "you know"}, CSV] 

If implemented, formatter_options should still work (for now and potentially the future if not too hard to handle).

Fun question would be that if formatter_options are kept around, how to deal with conflicting values in those/merging. Easy options are when "local" options are supplied formatter_options are ignored, or they are merged with a preference for local.

The option scope captured in a function

Basically needs the API adjustment in the formatters as well, but people could also do this then:

html_options = [file: "some.html", other: :option]

# ... other benchee stuff

formatters: [fn(suite) -> HTML.format(suite, html_options) end]

Feedback gladly welcome, before I go off an implement the wrong thing :D

[devonestes]

Ok, just throwing out all the options I can think of here. Maybe they can serve as further inspiration for something, or maybe they're just bad ideas. Couldn't hurt to mention them, though.

A map!

We could use a map with either functions or module as the key, and then the values could be a kwlist of configuration options. So, something like this:

formatters: %{
  HTML =>                                    [file: "file.html", awesome: true],
  &CSV.output/2 =>                           [file: "file.csv"]
  fn(suite) -> Mod.thing_to_do(suite) end => []
}

Pros

• I think maps have a really clear syntax. The relationship between formatters and options is super clear.
• Ensures that we only have one instance of each key so we don't double print if a user adds a formatter twice by accident.

Cons

• You always need to give a value for each key, so that makes relying on defaults kind of tricky.
• It's kind of odd when you're using a function as a key. It works, but it still just seems weird.
• In general, it seems kind of weird. I don't really like it.

Configurable Formatter Structs/maps!

I found myself kind of wishing for some state here, since that's the issue we're having - keeping relevant state together. How about if we could do something like the following:

html = Benchee.Formatters.HTML.new
       |> Benchee.Formatters.HTML.output_file("file.html")
       |> Benchee.Formatters.HTML.awesome(true)

user_formatter = %{
  function: fn(suite, options) -> do_a_thing(suite, options) end
}

formatters: [html, Benchee.Formatters.CSV.new, user_formatter]

We could ensure that the map/struct that we got has a key for function, and then any other keys could represent the configuration options for the given formatter function.

pros

• Easy setting and managing of default options thanks to new/0
• I think the idea of configuring a formatter in this way make sense (but that could just be my OO knowledge talking)

cons

• Lots of configuration - kind of verbose
• Very much a stolen pattern from OO world

Other thoughts

For some reason, the anonymous function/closure idea seems really odd to me. As a new user that would seem weird to me.

The thing that kind of irks me about the tuple example are all the various different types of things in that list. The typespec for that list is basically list(tuple | module | fun), and we have tuples with the structure of {fun, kwlist} and {mod, kwlist}.

I think that it might make sense to tackle #53 first so we can at least remove those functions from the conversation and only deal with modules.

Assuming that's done, how about this sort of structure:

formatters: [
  {HTML, file: "file.html", awesome: true},
  CSV,
  UserDefinedModule
]

That way the only things that we accept are list(module | {module, kwlist}), which I think would be pretty easy for a user to remember. Always a module, and if you need configuration options then you use a tuple. That's not bad.

In general, I think the list of modules or tuples will probably be best, but maybe you have some good ideas after seeing my rough ideas above!

[PragTob]

👋

Thanks for the input 😁

I'm sort of enthralled by the "OOP" idea, didn't think of that, it's pretty neat imo but might also be the OOP speaking ;P

I think in the end the list with tuple/no tuple might be best indeed. Although, while the typespec variations get bigger I wanna keep functions in there at least for now. I don't want to have a breaking change if I can avoid it and elixir pattern matching/basic type checks have often provided for a relatively easy way to support these. Depending on how that shapes up I think we might or might not deprecate pure functions in 1.0 (or 0.99 or whenever). I sort of like the idea though that people can just pass in a simple function and do whatever they want with the final suite without implementing a whole module :)

[PragTob]

Trying to tackle this now with our tuple variant

[PragTob]

#242