Allow NON_BOUNDED value to be used as constructor parameter

[kimbirkelund]

I'd like to suggest this minor change to allow creating an unlimited queue, without having to use another constructor. This makes it easier to inherit PeriodicBatchingSink as fewer constructor overloads are necessary.

[nblumhardt]

Thanks! I've hit this when implementing sinks, too; I think weighing up the pros and cons it's a change worth making, even if it's a magic number :-)

Can we add a note to the documentation of the corresponding PeriodicBatchingSink constructor, a public const int NoQueueLimit = -1; constant, and then make the PeriodicBatchingSink constructors chained? (With this change, one can now forward to the other.)

[kimbirkelund]

Something like that?

I agree that the magic number is less than ideal.

I'd like to introduce a QueueLimit class that has a singleton value, QueueLimit.Unbounded, and instances representing specific limits. But it may be more trouble than it's worth.

[nblumhardt]

I think adding an additional class just to hold the constant might be more trouble than it's worth 👍

Added a couple of small suggestions just to try to tighten things up in the presence of this change; LGTM otherwise!

[kimbirkelund]

Sorry for the delay. Life happened :)

The requested changes should be done now.

[nblumhardt]

Thanks! I'm going to make a very small change on the way in:

            if (!(batchSizeLimit > 0))
                throw new ArgumentOutOfRangeException(nameof(batchSizeLimit), "Parameter must be greater than 0.");
            if (!(period > TimeSpan.Zero))
                throw new ArgumentOutOfRangeException(nameof(period), "Parameter must be greater than the 0 time span.");

In these conditions it'd be more usual to see if (batchSizeLimit <= 0) in Serilog projects, though I understand they're equivalent :-)

Should be merged shortly. Thanks!

[kimbirkelund]

Glad to help :-)