Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature: list contextProperties of Logger #3422

Closed
304NotModified opened this issue May 24, 2019 · 8 comments
Closed

Feature: list contextProperties of Logger #3422

304NotModified opened this issue May 24, 2019 · 8 comments
Labels
Milestone

Comments

@304NotModified
Copy link
Member

304NotModified commented May 24, 2019

Make _contextProperties public on the Logger object.

Discussion: read-only or mutable?

asked here: https://stackoverflow.com/questions/56278360/nlog-list-property-values

@snakefoot
Copy link
Contributor

Discussion: read-only or mutable?

I vote for readonly using IReadOnlyDictionary (Only have the property on platforms where interface is supported).

@304NotModified
Copy link
Member Author

I checked the code. I think IDictionary is flexible enough. If we need to know changes in the dictionary, we could implement this.

@snakefoot
Copy link
Contributor

snakefoot commented May 24, 2019

Doesn't make sense. You can modify using the IDictionary-interface. It is way too flexible. You will get Collection was modified-exceptions.

Yes I know the Logger-object is very nice, and not touching the internal Dictionary. But you need to protect NLog from its evil users (Just like with the LoggingRules-collection that has been a generator of exceptions for a long time, and still is).

@304NotModified
Copy link
Member Author

Do we need also a clone then? (I think so)

@snakefoot
Copy link
Contributor

Do we need also a clone then? (I think so)

Would be very confusing to get a mutable dictionary returned, and your changes doesn't have any effect.

I continue to vote for IReadOnlyDictionary

@304NotModified
Copy link
Member Author

That's clear ;)

But besides IReadOnlyDictionary we need a clone then. Otherwise your could cast it back. Unfortunately, you could always change and object living in the dictionary

@snakefoot
Copy link
Contributor

Trying to protect yourself from a developer is hard work, and seldom gives any bonus.

Developer can also do reflection and access the dictionary that way.

The goal is to provide an easy to understand interface.

@304NotModified
Copy link
Member Author

fixed by #3430

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants