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

LogFactory - GetLogger should null-check logger name #3332

Merged
merged 1 commit into from Apr 24, 2019

Conversation

snakefoot
Copy link
Contributor

@snakefoot snakefoot commented Apr 24, 2019

Throw ArgumentNullException instead of throwing NullReferenceException here (When Name is null):

return ConcreteType.GetHashCode() ^ Name.GetHashCode();

Could also consider doing fallback to string.Empty.

@snakefoot snakefoot changed the title LogFactory - GetLogger should validate name of logger LogFactory - GetLogger should null-check logger name Apr 24, 2019
@codecov-io
Copy link

Codecov Report

Merging #3332 into dev will increase coverage by <1%.
The diff coverage is 100%.

@@          Coverage Diff           @@
##             dev   #3332    +/-   ##
======================================
+ Coverage     80%     80%   +<1%     
======================================
  Files        356     356            
  Lines      28101   28103     +2     
  Branches    3743    3744     +1     
======================================
+ Hits       22517   22535    +18     
+ Misses      4503    4486    -17     
- Partials    1081    1082     +1

@304NotModified 304NotModified merged commit ea6f665 into NLog:dev Apr 24, 2019
@304NotModified 304NotModified added this to the 4.6.3 milestone Apr 24, 2019
@304NotModified 304NotModified added enhancement Improvement on existing feature has unittests labels Apr 24, 2019
@304NotModified
Copy link
Member

Thanks!

@304NotModified
Copy link
Member

Could also consider doing fallback to string.Empty

Doubted also about this, but I think the change in this PR is good enough

@snakefoot snakefoot deleted the LogFactoryGetLoggerNullCheck branch April 4, 2020 12:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improvement on existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants