dennisdoomen / CSharpGuidelines

A set of coding guidelines for C# 9.0, design principles and layout rules for improving the overall quality of your code development.
https://www.csharpcodingguidelines.com
Other
745 stars 272 forks source link

Revised AV1553 #193

Closed keremispirli closed 5 years ago

keremispirli commented 5 years ago

The current guideline looks like a blanket ban which also affects legitimate uses of optional parameters that current justifications mentioned in the guideline do not apply.

If the optional parameter is a reference type then it can only have a default value of null. But since strings, lists and collections should never be null according to rule AV1135, you must use overloaded methods instead.

The claim in the first sentence is already wrong about String's: Even though “String” is reference type, an optional string parameter can have non-null default values. Also, the first sentence of the justification doesn't support the suggestion in the second sentence.

Note: The default values of the optional parameters are stored at the caller side. As such, changing the default value without recompiling the calling code will not apply the new default value.

Note: When an interface method defines an optional parameter, its default value is discarded during overload resolution unless you call the concrete class through the interface reference.

These two justifications written as notes do not apply to private or internal methods, neither does Eric Lippert's series of posts linked at the end.

Proposed changes:

  1. Removed the first sentence of the first justification. Not only that it's wrong with respect to string type, but also it doesn't add value even if we fix it.
  2. Added explanation for using optional string parameters.
  3. Extended first note to exclude private and internal methods, since it doesn't apply to them.
  4. Changed first note to a caveat, added a second caveat against abusing optional parameters to merge different methods.
  5. Converted second note to a separate guideline against using optional parameters in interfaces and their concrete implementations.
dennisdoomen commented 5 years ago

Great suggestions indeed. Thanks for taken the time.

keremispirli commented 5 years ago

My pleasure. Thank you for creating this in the first place.