-
Notifications
You must be signed in to change notification settings - Fork 25
Description
The Analyzer logger takes triggers as a first parameter, which effectively forces a naive user to pass at least the on_improvement trigger for the logger to work. I would rather see the triggers defaulted near the end of the constructor parameters, so that a naive user would not have to bother with this feature (which is more for advanced users).
The Analyzer also makes a difference between Properties and Attributes, but I don't understand why. analyzer::structures::Attribute stores a key-value pair, I guess to log the algorithm's parameters. But this is also the objective of Properties, why have a specific class for Analyzer? The aim of properties is also to allow to log any algorithm parameter in any logger.
The properties were also designed so as to be a generic view about anything that would be logged. In that sense, they are also a way to control if the x are stored (or not). But loggers have a store_position flag, which necessitates additional code to check if one wants to log log_info.current.x. It would seem more logical to let a property handle this. That way, one does not need additional code, but only to pass (or not) a Property handling the current x.
However, given that a constructor with a boolean is surely easier to grasp by a newcomer, we should keep a separated —simplified— constructor with this flag, and add the related property if it is true.
Additionally, hiding the default necessary properties is a good idea, and I would suggest going further so as to do the same with triggers: have the on_improvement as hidden default, and allow for additional triggers in the full constructor.
The constructor that I would find easier to grasp for a newcomer would put the most sensible parameters first, something like:
//! Simple constructor.
Analyzer(
const std::string &folder_name = "ioh_data",
const std::string &algorithm_name = "algorithm_name",
const std::string &algorithm_info = "algorithm_info",
const fs::path &root = fs::current_path(),
const bool store_positions = false,
const Properties &additional_properties = {}
) {}
//! Constructor with full control.
Analyzer(
const std::string &folder_name = "ioh_data",
const std::string &algorithm_name = "algorithm_name",
const std::string &algorithm_info = "algorithm_info",
const fs::path &root = fs::current_path(),
const Properties &additional_properties = {},
const Triggers &additional_triggers = {}
) {}