rust-prometheus icon indicating copy to clipboard operation
rust-prometheus copied to clipboard

Consider changing `Registry::register/unregister` to remove the `Box` in the argument

Open nagisa opened this issue 4 years ago • 3 comments

Currently register and unregister methods of Registry take Box<dyn Collector>, however in practice this means that most calls of this method will end up looking like this:

let metric = /* a concrete type of some sort */;
registry.register(Box::new(metric.clone()));

This feels unsatisfactory in multiple ways, for example I cannot sleep well knowing there’s an unnecessary double indirection in my code base (Box to Arc to the actual metric, could instead be just Arc<dyn Collector>). But also the API is not too convenient and fails to hide implementation details. It could instead be like this

fn register<C: Collector>(&self, collector: C) { /* do the boxing inside */ };
// use as `registry.register(metric.clone());`

fn unregister<C: Collector>(&self, collector: &C) { /* does this even need a box???  */ };
// use as `registry.unregister(&metric);`

and no longer force users to deal with the manual Boxing.

nagisa avatar Oct 01 '19 22:10 nagisa