From 018ee9db7b377ee5b713bbe7a74babc6cc000965 Mon Sep 17 00:00:00 2001 From: Bart Koelman Date: Sun, 22 Oct 2017 15:55:05 +0200 Subject: [PATCH 1/2] Removed AV1570 because it advocates outdated language usage. --- Src/Cheatsheet/Cheatsheet.md | 1 - Src/Guidelines/1500_MaintainabilityGuidelines.md | 3 --- 2 files changed, 4 deletions(-) diff --git a/Src/Cheatsheet/Cheatsheet.md b/Src/Cheatsheet/Cheatsheet.md index a2c8f59..7b00bd4 100644 --- a/Src/Cheatsheet/Cheatsheet.md +++ b/Src/Cheatsheet/Cheatsheet.md @@ -90,7 +90,6 @@ NOTE: Requires Markdown Extra. See http://michelf.ca/projects/php-markdown/extra * Don’t allow methods and constructors with more than three parameters (AV1561) * Don’t use `ref` or `out` parameters (AV1562) * Avoid methods that take a `bool` flag (AV1564) -* Always check the result of an `as` operation (AV1570) * Don’t comment-out code (AV1575)
diff --git a/Src/Guidelines/1500_MaintainabilityGuidelines.md b/Src/Guidelines/1500_MaintainabilityGuidelines.md index e146dd2..5a94a91 100644 --- a/Src/Guidelines/1500_MaintainabilityGuidelines.md +++ b/Src/Guidelines/1500_MaintainabilityGuidelines.md @@ -404,8 +404,5 @@ Often, a method taking such a flag is doing more than one thing and needs to be ### Don't use parameters as temporary variables (AV1568) ![](images/3.png) Never use a parameter as a convenient variable for storing temporary state. Even though the type of your temporary variable may be the same, the name usually does not reflect the purpose of the temporary variable. -### Always check the result of an `as` operation (AV1570) ![](images/1.png) -If you use `as` to obtain a certain interface reference from an object, always ensure that this operation does not return `null`. Failure to do so may cause a `NullReferenceException` at a much later stage if the object did not implement that interface. - ### Don't comment out code (AV1575) ![](images/1.png) Never check in code that is commented out. Instead, use a work item tracking system to keep track of some work to be done. Nobody knows what to do when they encounter a block of commented-out code. Was it temporarily disabled for testing purposes? Was it copied as an example? Should I delete it? From 583a80896e7d09183556ca625a9f393abd581b72 Mon Sep 17 00:00:00 2001 From: Bart Koelman Date: Sun, 22 Oct 2017 21:01:01 +0200 Subject: [PATCH 2/2] Review feedback --- Src/Cheatsheet/Cheatsheet.md | 1 + Src/Guidelines/1500_MaintainabilityGuidelines.md | 16 ++++++++++++++++ 2 files changed, 17 insertions(+) diff --git a/Src/Cheatsheet/Cheatsheet.md b/Src/Cheatsheet/Cheatsheet.md index 7b00bd4..ea9a585 100644 --- a/Src/Cheatsheet/Cheatsheet.md +++ b/Src/Cheatsheet/Cheatsheet.md @@ -90,6 +90,7 @@ NOTE: Requires Markdown Extra. See http://michelf.ca/projects/php-markdown/extra * Don’t allow methods and constructors with more than three parameters (AV1561) * Don’t use `ref` or `out` parameters (AV1562) * Avoid methods that take a `bool` flag (AV1564) +* Prefer `is` patterns over `as` operations (AV1570) * Don’t comment-out code (AV1575)
diff --git a/Src/Guidelines/1500_MaintainabilityGuidelines.md b/Src/Guidelines/1500_MaintainabilityGuidelines.md index 5a94a91..2c102b9 100644 --- a/Src/Guidelines/1500_MaintainabilityGuidelines.md +++ b/Src/Guidelines/1500_MaintainabilityGuidelines.md @@ -404,5 +404,21 @@ Often, a method taking such a flag is doing more than one thing and needs to be ### Don't use parameters as temporary variables (AV1568) ![](images/3.png) Never use a parameter as a convenient variable for storing temporary state. Even though the type of your temporary variable may be the same, the name usually does not reflect the purpose of the temporary variable. +### Prefer `is` patterns over `as` operations (AV1570) ![](images/1.png) + +If you use 'as' to safely upcast an interface reference to a certain type, always verify that the operation does not return `null`. Failure to do so may cause a `NullReferenceException` at a later stage if the object did not implement that interface. +Pattern matching syntax prevents this and improves readability. For example, instead of: + + var remoteUser = user as RemoteUser; + if (remoteUser != null) + { + } + +write: + + if (user is RemoteUser remoteUser) + { + } + ### Don't comment out code (AV1575) ![](images/1.png) Never check in code that is commented out. Instead, use a work item tracking system to keep track of some work to be done. Nobody knows what to do when they encounter a block of commented-out code. Was it temporarily disabled for testing purposes? Was it copied as an example? Should I delete it?