Conversation
…e changes Co-authored-by: dennisdoomen <572734+dennisdoomen@users.noreply.github.com> Agent-Logs-Url: https://github.com/dennisdoomen/CSharpGuidelines/sessions/a0a48af1-729d-4c09-a388-33a5be941ba6
…es in 2202 Co-authored-by: dennisdoomen <572734+dennisdoomen@users.noreply.github.com> Agent-Logs-Url: https://github.com/dennisdoomen/CSharpGuidelines/sessions/a0a48af1-729d-4c09-a388-33a5be941ba6
|
@copilot can you also check if all links in the Markdown files are still working? |
Co-authored-by: dennisdoomen <572734+dennisdoomen@users.noreply.github.com> Agent-Logs-Url: https://github.com/dennisdoomen/CSharpGuidelines/sessions/0fe17cce-0033-4213-89f8-7608edd9d0fb
Done in commit Dead sites replaced with working alternatives:
Updated to current URLs:
Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
|
@copilot Introduce a new "General guidelines" section numbered AV0100 and up and add the following guidelines with a rationale
|
@copilot address this |
Co-authored-by: dennisdoomen <572734+dennisdoomen@users.noreply.github.com> Agent-Logs-Url: https://github.com/dennisdoomen/CSharpGuidelines/sessions/adb91ce2-6aa7-4b11-86fb-74fe17432427
Done in commit
Also:
|
There was a problem hiding this comment.
@copilot address the comments
Add new rules to the AV1000 series for:
- Don't create a record with a constructor that takes more than 4 parameters. Use properties instead
- When to use a record, and when to use a class
- Consider a delegate instead of an interface with a single method
- Use Primary constructors
In the AV1100 series, create new rules for:
- Use extension blocks
- Avoid local functions
- Use the field keyword
Under the AV1500 series, create new rules for:
- Align projects with the depllyment units, not layers
- Considering duplicating code across different modules to reduce coupling
- Use raw string literals
- Make properties required when applicable
_includes/0001_Introduction.md
Outdated
| @@ -21,7 +23,7 @@ In general, if I have a discussion with a colleague about a smell that this docu | |||
| - The Principle of Least Surprise (or Astonishment): you should choose a solution that everyone can understand, and that keeps them on the right track. | |||
There was a problem hiding this comment.
Move this to the new general guidelines section
There was a problem hiding this comment.
Moved to the General Guidelines section as rule AV0112 (Apply the Principle of Least Surprise) in commit d20b723.
_includes/0001_Introduction.md
Outdated
| @@ -21,7 +23,7 @@ In general, if I have a discussion with a colleague about a smell that this docu | |||
| - The Principle of Least Surprise (or Astonishment): you should choose a solution that everyone can understand, and that keeps them on the right track. | |||
| - Keep It Simple Stupid (a.k.a. KISS): the simplest solution is more than sufficient. | |||
There was a problem hiding this comment.
Move this to the new general guidelines section
There was a problem hiding this comment.
Moved to the General Guidelines section as rule AV0115 (Keep It Simple Stupid) in commit d20b723.
_includes/0001_Introduction.md
Outdated
| @@ -21,7 +23,7 @@ In general, if I have a discussion with a colleague about a smell that this docu | |||
| - The Principle of Least Surprise (or Astonishment): you should choose a solution that everyone can understand, and that keeps them on the right track. | |||
| - Keep It Simple Stupid (a.k.a. KISS): the simplest solution is more than sufficient. | |||
| - You Ain't Gonna Need It (a.k.a. YAGNI): create a solution for the problem at hand, not for the ones you think may happen later on. Can you predict the future? | |||
There was a problem hiding this comment.
Move this to the new general guidelines section
There was a problem hiding this comment.
Moved to the General Guidelines section as rule AV0120 (You Ain't Gonna Need It) in commit d20b723.
_includes/0001_Introduction.md
Outdated
| - Keep It Simple Stupid (a.k.a. KISS): the simplest solution is more than sufficient. | ||
| - You Ain't Gonna Need It (a.k.a. YAGNI): create a solution for the problem at hand, not for the ones you think may happen later on. Can you predict the future? | ||
| - Don't Repeat Yourself (a.k.a. DRY): avoid duplication within a component, a source control repository or a [bounded context](http://martinfowler.com/bliki/BoundedContext.html), without forgetting the [Rule of Three](http://lostechies.com/derickbailey/2012/10/31/abstraction-the-rule-of-three/) heuristic. | ||
| - Don't Repeat Yourself (a.k.a. DRY): avoid duplication within a component, a source control repository or a [bounded context](http://martinfowler.com/bliki/BoundedContext.html), without forgetting the [Rule of Three](https://en.wikipedia.org/wiki/Rule_of_three_(computer_programming)) heuristic. However, when sharing code across architectural or team boundaries would introduce tight coupling, consider duplicating simple logic rather than creating an unwanted dependency. |
There was a problem hiding this comment.
Move this to the new general guidelines section
There was a problem hiding this comment.
Moved to the General Guidelines section as rule AV0125 (Don't Repeat Yourself) in commit d20b723.
_includes/0001_Introduction.md
Outdated
| - You Ain't Gonna Need It (a.k.a. YAGNI): create a solution for the problem at hand, not for the ones you think may happen later on. Can you predict the future? | ||
| - Don't Repeat Yourself (a.k.a. DRY): avoid duplication within a component, a source control repository or a [bounded context](http://martinfowler.com/bliki/BoundedContext.html), without forgetting the [Rule of Three](http://lostechies.com/derickbailey/2012/10/31/abstraction-the-rule-of-three/) heuristic. | ||
| - Don't Repeat Yourself (a.k.a. DRY): avoid duplication within a component, a source control repository or a [bounded context](http://martinfowler.com/bliki/BoundedContext.html), without forgetting the [Rule of Three](https://en.wikipedia.org/wiki/Rule_of_three_(computer_programming)) heuristic. However, when sharing code across architectural or team boundaries would introduce tight coupling, consider duplicating simple logic rather than creating an unwanted dependency. | ||
| - The [four principles of object-oriented programming](https://github.com/TelerikAcademy/Object-Oriented-Programming/tree/master/Topics/04.%20OOP-Principles-Part-1): encapsulation, abstraction, inheritance and polymorphism. |
There was a problem hiding this comment.
Move this to the new general guidelines section
There was a problem hiding this comment.
Moved to the General Guidelines section as rule AV0130 (Apply the four pillars of object-oriented programming) in commit d20b723.
_includes/0001_Introduction.md
Outdated
| - Both ReSharper and Rider have a File Structure window that displays an overview of the members of your class or interface, and allows you to easily rearrange them using a simple drag-and-drop action. | ||
| - [CSharpGuidelinesAnalyzer](https://github.com/bkoelman/CSharpGuidelinesAnalyzer) verifies over 40 of our guidelines, while typing code in Visual Studio 2017-2022 and during CI builds. An updated Resharper settings file is included. | ||
| - Many of these guidelines are also enforced by [Roslyn analyzers](https://learn.microsoft.com/en-us/visualstudio/code-quality/roslyn-analyzers-overview). You can configure them centrally in a `Directory.Build.props` file to apply them to all projects in your solution. | ||
| - Using AI-generated code is fine, as long as you treat all that code as your own. You are responsible for understanding, reviewing and maintaining the code, just as if you had written it yourself. |
There was a problem hiding this comment.
Move this to the new general guidelines section
There was a problem hiding this comment.
Moved to the General Guidelines section as rule AV0135 (Treat AI-generated code as your own) in commit d20b723.
_rules/1125.md
Outdated
| A classic example of this is the `HttpContext.Current` property, part of classic ASP.NET. Many see this as a source of a lot of ugly code, because it hides an implicit dependency on a global state object. Instead, make dependencies explicit by injecting them through a constructor or method parameter. | ||
|
|
||
| A classic example of this is the `HttpContext.Current` property, part of ASP.NET. Many see the `HttpContext` class as a source of a lot of ugly code. | ||
| In general, a stateful object is an object that contains many properties and lots of behavior behind it. If you expose such an object through a static property or method of some other object, it will be very difficult to refactor or unit test a class that relies on such a stateful object. Introducing a construct like that is a great example of violating many of the guidelines of this chapter. |
_rules/1220.md
Outdated
There was a problem hiding this comment.
Please remove the entire rule
There was a problem hiding this comment.
Entire rule deleted in commit d20b723.
_rules/1523.md
Outdated
|
|
||
| var countryLookupTable = new Dictionary<string, string>(); | ||
| countryLookupTable.Add("NL", "Netherlands"); | ||
| countryLookupTable.Add("US", "United States"); |
There was a problem hiding this comment.
Removed in commit d20b723. The rule now starts directly with the positive example.
|
@copilot address the code review comments |
|
@copilot Add a new section called "Testability Guidelines" in the AV1600 series with the following guidelines
|
… General Guidelines Co-authored-by: dennisdoomen <572734+dennisdoomen@users.noreply.github.com> Agent-Logs-Url: https://github.com/dennisdoomen/CSharpGuidelines/sessions/1b87f45f-f681-444c-ba56-3a9995b9f2a3
Done in commit Class Design (AV1000 series):
Member Design (AV1100 series):
Maintainability (AV1500 series):
All new rules are also reflected in the Cheatsheet.
All code review comments addressed in commit
|
Co-authored-by: dennisdoomen <572734+dennisdoomen@users.noreply.github.com> Agent-Logs-Url: https://github.com/dennisdoomen/CSharpGuidelines/sessions/9476bc29-3344-48cd-a2f6-086cf07c8cf2
for loop or a collection in a foreach loop
#363
|
Superseded by smaller replacement PRs. |
Broad refresh of the guidelines to align with C# 14 and current .NET ecosystem practices. Covers version bumps, introduction rewrites, rule additions/updates/merges/removals, cheatsheet sync, and link maintenance.
Version & Site-wide
_config.yml, cover, cheatsheet, index)Introduction
Directory.Build.propstipNew: General Guidelines (AV0100)
A new section covering fundamental software design principles, numbered AV0100 and up:
New Rules Added
Class Design (AV1000 series)
Member Design (AV1100 series)
fieldkeyword in auto-properties when additional logic is needed (C# 14)Maintainability (AV1500 series)
requiredwhen they must be set during initialization (C# 11+)Testability (AV1600 series) — new section
Specsinstead ofTests; nested class example updated toOrder_placementFramework Guidelines (AV2200 series)
var (x, y) = point;), list-pattern matching for arrays/collections, andforeachdeconstructionDocumentation Guidelines (AV2300 series)
Rules Updated
IFetchSomething) overIClassNamea(n abstract)→an abstractrecordfor DTOsforeachmutation??,??=,?.examplesDefaultInterpolatedStringHandlerimprovementsIO), >2 chars title-case (Http)IComponent→`IComponent`)-ing/-edpre/post-event namingAsyncwhen a synchronous counterpart exists!= null/HasValuewithis null/is not null; add collection expression example ([]); add null-conditional assignment (??=) example for C# 14inheritdocis not needed on overriding membersRules Removed
async/awaitis the default nowSeverity Changes
Link Maintenance
Fixed all broken and outdated links across the documentation:
lostechies.comlinks with Wikipedia equivalents (Rule of Three, ISP, SOLID)8thlight.comSRP article with Wikipedia SRPdomaindrivendesign.orgwith Wikipedia DDDsites.google.com/site/unclebobconsultingllc/withcleancoder.comtiobe.comC# 1.0 paper link (2002)blogs.msdn.microsoft.comLINQ Guidelines post from Resources pagemsdn.microsoft.com/magazineasync article tolearn.microsoft.comarchivedocs.microsoft.comlinks tolearn.microsoft.com2021.3) to version-neutral URLhttp://→https://for JetBrains andcontinuousimprover.comlinkshttps://protocol onwww.continuousimprover.comCheatsheet footer linksOriginal prompt
This section details on the original issue you should resolve
<issue_title>Updates for the new version</issue_title>
<issue_description>Make the following changes on the
developbranchGeneral
Update version numbers to C# 14
Update cheat sheet
Introduction
Add something about the use of AI. E.g. using is fine, as long as you treat all that code as your own
Replace ".NET Framework" with just .NET
Mention my .NET Starter Kit as a good companion
Mention that this guideline is also used by all my open-source projects
Explain to apply DRY within boundaries, but duplicate code across boundaries when it's simple
Add the most critical coding guidelines on your PR template
Check all links to see if they point to valid files
Rider has a file structure window as well
Mention all my open-source projects and how they influenced the guidelines
Remove Gitter reference
Update social media links
Mention the many Roslyn analyzers and how directory.props can be used
General Guidelines
New: Understand the boundaries of your code base
New: Use design patterns to refactor code into a structure people can recognize by name
New: Prefer compoisiton over inheritance
Class Design Guidelines: Rename to "Type Design Guidelines" or "Class/Record Design Guidelines"
AV1000
Tip: If you can't find a good name for a class, or struggle to document, it's doing to much.
Tip: If you explain what the class does to an AI, it may help detect multiple responsibilities
For primitive types, consider an immutable record type
AV1001?: Only pass things to a constructor that most or all members need
AV1003: Avoid taking the name of the class and slapping an I in front of it
Consider using role-based names like IFetchSomething or IProvideClusterwideLock
AV1004: Typo in "a(n abstract)"
AV1005: Understand the boundaries of your code base. Use interfaces across boundaries, but avoid them within the boundary
AV1013: Rephrase as "Don't cast a base class to one of its derived classes". Also add an example.
AV1025: For DTOs, a record is probably a better example
AV1026: Consider removing
New: Don't create a record with a constructor that takes more than 4 parameters. Use properties instead
New: When to use a record, and when to use a class
New: Consider a delegate instead of an interface with a single method
New: Use Primary constructors
Member Design Guidelines
AV1115: Consider removing as it is difficult to explain or validate
AV1125: Rewrite and start with the example of HttpContext.Current
AV1130: Revisit, as IEnumerable is often perceived as lazy-evaluated. And why not just return an array?
AV1137: Rewrite as "Keep parameters as specific and narrow as possible"
AV1140: Rewrite as "Consider created domain-specific types rather than primitives"
New: Use extension blocks
New: Avoid local functions
New: Use the field keyword
Miscellaneous Design Guidelines
AV1220: Remove as nobody uses events anymore
AV1250: Rename to "Materialize the result of a Iqueryable or Ienumerable expression before returning it"
Maintainability Guidelines
AV1500: Increase to 15
New: Align projects with the depllyment units, not layers
Remove: AV1510
AV1521: Remove the reference to Visual Basic and C
AV1523
Extend with array initializers, including spread operator
Remove the old example
AV1525: Remove as it is not something I see a lot
AV1530: Extend with foreach as well
AV1532: Remove, as LINQ statements are not always clearer. Use the right tool at the right time
AV1545
Take into considering the ? operator (conditional assignment and conditional expressions)
Also see if we can keep that section shorter
AV1546: Do some investigation on what string mechanism is the fastest in .NET 8 and later
AV1551: Consider removing it as it is not something I see a lot
AV1553: Check if this still makes sense and rewrite it to be shorter
AV1554: Still applicable?
AV1568: Remove. It's valid, but I don't see this a lot.
AV1580: Remove, as all debuggers support showing the individual sections
New: Considering duplicating code across different modules to reduce coupling
New: Use raw string literals
New: Make properties required
Testability Guidelines (1600)
Use short concise functional names
Show what's important, hide what's not
Avoid private helper methods to build various test objects
Use test Data Builders to build flexi
Exception: Sometimes a simple object mother is sufficent
Test behavior, not implementation details
Prefer inline literals over constant variables
Don't use production code in assertions
Use short fact-based test names
Postfix test classes with Specs instead of Tests
Test things designed to be reusable separately
Test implementations as part of a bigger scope
Naming Guidelines
AV1701: Remove the reference to other programming languages
AV1706: Mention the naming for abbreviations longer than 2 characters
AV1708: Not all code elements are rendered in code style
AV1737: Merge int...
💬 Send tasks to Copilot coding agent from Slack and Teams to turn conversations into code. Copilot posts an update in your thread when it's finished.