Skip to content
This repository has been archived by the owner on Apr 23, 2021. It is now read-only.

Instrument "box" naming #43

Closed
ktoso opened this issue Jun 22, 2020 · 2 comments · Fixed by #53
Closed

Instrument "box" naming #43

ktoso opened this issue Jun 22, 2020 · 2 comments · Fixed by #53
Assignees
Labels
enhancement New feature or request

Comments

@ktoso
Copy link
Collaborator

ktoso commented Jun 22, 2020

(ignoring how the inject/extract to/from are expressed).

We'd want to write Instrument as the generic rather than I since that's common swift style to use full words.
But we today use Instrument for the wrapper type:

    init<Instrument>(instrument: Instrument)
        where Instrument: InstrumentProtocol,
        Instrument.InjectInto == HTTPHeaders,
        Instrument.ExtractFrom == HTTPHeaders {

        self.instrument = Instrument(instrument) // conflict

What would be a better naming strategy for this type:

/// A box-type for an `InstrumentProtocol`, necessary for creating homogeneous collections of `InstrumentProtocol`s.
public struct Instrument<InjectInto, ExtractFrom>: InstrumentProtocol {
    private let inject: (BaggageContext, inout InjectInto) -> Void
    private let extract: (ExtractFrom, inout BaggageContext) -> Void

    /// Wrap the given `InstrumentProtocol` inside an `Instrument`.
    /// - Parameter instrument: The `InstrumentProtocol` being wrapped.
    public init<I>(_ instrument: I)
        where
        I: InstrumentProtocol,
        I.InjectInto == InjectInto,
        I.ExtractFrom == ExtractFrom {

TracingInstrument does come to mind, but these ones are not specifically tracing, they are generic.

Having looked at this again I guess this should be AnyInstrument and the protocol can become InstrumentProtocol (OR BaggageInstrument)... WDYT?

@slashmo
Copy link
Owner

slashmo commented Jun 22, 2020

Having looked at this again I guess this should be AnyInstrument and the protocol can become InstrumentProtocol

Agreed. Calling the generic type argument I felt like a necessary workaround rather than a good solution. I think AnyInstrument more clearly communicates the intent 👍 I'd prefer Instrument for the protocol, but then we'd have to use I or something similar again, so I guess calling it InstrumentProtocol is worth the trade-off.

@ktoso
Copy link
Collaborator Author

ktoso commented Jun 22, 2020

Yeah, sounds like reasonable tradeoffs 👍

@slashmo slashmo self-assigned this Jun 26, 2020
@slashmo slashmo added the enhancement New feature or request label Jun 26, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants