Skip to content

Commit e9273f9

Browse files
keremispirlidennisdoomen
authored andcommitted
Revised AV1553 (#193)
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.
1 parent 5d141b8 commit e9273f9

1 file changed

Lines changed: 17 additions & 5 deletions

File tree

_pages/1500_MaintainabilityGuidelines.md

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -366,20 +366,32 @@ The class `MyString` provides three overloads for the `IndexOf` method, but two
366366

367367
**Important:** If you also want to allow derived classes to override these methods, define the most complete overload as a non-private `virtual` method that is called by all overloads.
368368

369-
### <a name="av1553"></a> Only use optional arguments to replace overloads (AV1553) ![](/assets/images/1.png)
370-
The only valid reason for using C# 4.0's optional arguments is to replace the example from rule [AV1551](#av1551) with a single method like:
369+
### <a name="av1553"></a> Only use optional parameters to replace overloads (AV1553) ![](/assets/images/1.png)
370+
The only valid reason for using C# 4.0's optional parameters is to replace the example from rule [AV1551](#av1551) with a single method like:
371371

372372
public virtual int IndexOf(string phrase, int startIndex = 0, int count = -1)
373373
{
374374
int length = (count == -1) ? (someText.Length - startIndex) : count;
375375
return someText.IndexOf(phrase, startIndex, length);
376376
}
377377

378-
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](/member-design-guidelines#av1135), you must use overloaded methods instead.
378+
Since strings, lists and collections should never be `null` according to rule [AV1135](/member-design-guidelines#av1135), if you have an optional parameter of these types with default value `null` then you must use overloaded methods instead.
379379

380-
**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.
380+
Strings, unlike other reference types, can have non-null default values. So an optional string parameter may be used to replace overloads with the condition of having a non-null default value.
381381

382-
**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. See [this post by Eric Lippert](http://blogs.msdn.com/b/ericlippert/archive/2011/05/09/optional-argument-corner-cases-part-one.aspx) for more details.
382+
Regardless of optional parameters' types, following caveats always apply:
383+
384+
1) The default values of the optional parameters are stored at the caller side. As such, changing the default argument without recompiling the calling code will not apply the new default value. Unless your method is private or internal, this aspect should be carefully considered before choosing optional parameters over method overloads.
385+
386+
2) If optional parameters cause the method to follow and/or exit from alternative paths, overloaded methods are probably a better fit for your case.
387+
388+
### <a name="av1554"></a> Do not use optional parameters in interface methods or their concrete implementations (AV1554) ![](/assets/images/1.png)
389+
390+
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.
391+
392+
When a concrete implementation of an interface method sets a default argument for a parameter, the default value is discarded during overload resolution if you call the concrete class through the interface reference.
393+
394+
See [this series by Eric Lippert](http://blogs.msdn.com/b/ericlippert/archive/2011/05/09/optional-argument-corner-cases-part-one.aspx) for more details.
383395

384396
### <a name="av1555"></a> Avoid using named arguments (AV1555) ![](/assets/images/1.png)
385397
C# 4.0's named arguments have been introduced to make it easier to call COM components that are known for offering many optional parameters. If you need named arguments to improve the readability of the call to a method, that method is probably doing too much and should be refactored.

0 commit comments

Comments
 (0)