davezych / shience

A .NET port(ish) of Github's Scientist library. (https://github.com/github/scientist)
MIT License
9 stars 1 forks source link

Publisher should not be global #12

Closed MovGP0 closed 8 years ago

MovGP0 commented 8 years ago

i just did some unit tests and noted a problematic behavior:

because publisher is global, switching the publisher is problematic in multi-threaded application where multiple publishers are required. instead, the user should provide the publisher as needed via dependency injection.

MovGP0 commented 8 years ago

suggested change: add the following method:

        public static Science<TResult> New<TResult>([NotNull]string name, [NotNull]IPublisher publisher)
        {
            if (string.IsNullOrWhiteSpace(name))
            {
                throw new ArgumentNullException(nameof(name));
            }

            if (publisher == null)
            {
                throw new ArgumentNullException(nameof(publisher));
            }

            return new Science<TResult>(name, publisher);
        }
davezych commented 8 years ago

I'm fine with this, as long as a "global" publisher is still supported. My original goal was to be able to set the publisher once and not force the user to instantiate a publisher for each experiment.

davezych commented 8 years ago

It looks like you added this overload in #13 . :+1:

davezych commented 8 years ago

Resolved in #13. Closing.