updates from review comments

doc/AG0001.md

@@ -12,13 +12,13 @@ Direct usage of `DependencyResolver` creates tight coupling and makes code harde
 - Improved maintainability by making dependencies visible
 - Stronger adherence to SOLID principles, particularly Dependency Inversion
 
-#### Don't
+#### Don't ❌
 
 ```c#
 var exampleService = DependencyResolver.Current.GetService<IExampleService>();
 ```
 
-#### Do
+#### Do ✅
 
 ```c#
 public class ExampleClass

doc/AG0002.md

@@ -6,7 +6,7 @@ Type: Code Smell
 
 When implementing an interface, classes should only expose methods that are part of their public contract. Additional public or internal methods that aren't part of the implemented interfaces create confusion about the class's responsibilities and violate the Interface Segregation Principle.
 
-#### Don't
+#### Don't ❌
 
 ```csharp
 interface ISomething 
@@ -25,7 +25,7 @@ class TestClass : ISomething
 }
 ```
 
-#### Do
+#### Do ✅
 
 ```csharp
 interface ISomething 

doc/AG0003.md

@@ -8,7 +8,7 @@ https://agoda-com.github.io/standards-c-sharp/services/framework-abstractions.ht
 
 Passing `System.Web.HttpContext` as a parameter creates tight coupling to the web infrastructure and makes unit testing significantly more difficult. Instead, pass only the specific HttpContext properties that your code actually needs.
 
-#### Don't
+#### Don't ❌
 
 ```csharp
 using System.Web;
@@ -32,7 +32,7 @@ class TestClass: ISomething
 }
 ```
 
-#### Do
+#### Do ✅
 
 ```csharp
 using System.Web;

doc/AG0004.md

@@ -8,15 +8,15 @@ https://agoda-com.github.io/standards-c-sharp/reflection/hard-coded-strings.html
 
 Hard-coded strings to identify namespaces and types make refactoring risky and move type resolution errors from compile-time to runtime. Always use the `typeof` operator to reference types, which provides compile-time safety.
 
-#### Don't
+#### Don't ❌
 
 ```csharp
 // Both fail at runtime after namespace changes
 var instance = Activator.CreateInstance("Agoda", "Agoda.MyType");
 var type = Type.GetType("Agoda.MyType");
 ```
 
-#### Do
+#### Do ✅
 
 ```csharp
 // Caught at compile time after namespace changes

doc/AG0005.md

@@ -13,15 +13,15 @@ Test names should follow this pattern:
 - Class: `<ClassNameUnderTest>Tests`
 - Methods: `<SystemUnderTest>_<PreCondition>_<PostCondition>` or `<SystemUnderTest>_<PostCondition>`
 
-#### Don't
+#### Don't ❌
 
 ```csharp
 [Test]
 public void HazardLightsTest() // Noncompliant - unclear what aspect is being tested
 {...}
 ```
 
-#### Do
+#### Do ✅
 
 ```csharp
 [Test]
@@ -43,4 +43,4 @@ If naming a test is difficult, it might indicate the test is doing too much and
 - Easy identification of failures
 - Documentation of behavior
 - Coverage visibility
-- Maintainable test suite
+- Maintainable test suite

doc/AG0006.md

@@ -6,11 +6,9 @@ Type: Code Smell
 
 Each component registered with the dependency injection container should have exactly one public constructor. Having multiple public constructors leads to ambiguity in dependency resolution and makes the codebase harder to maintain.
 
-This is a blocker rule - any component with multiple public constructors should be refactored to have only one.
-
 Reasons to avoid multiple public constructors:
 
-- Creates ambiguity for the DI container when resolving dependencies 
+- Creates ambiguity for the DI container when resolving dependencies
 - Makes it harder to track and manage dependencies
 - Increases complexity of dependency resolution
 - Makes code harder to test and maintain

doc/AG0009.md

@@ -6,7 +6,7 @@ Type: Code Smell
 
 Passing `IHttpContextAccessor` or `HttpContextAccessor` as a parameter creates tight coupling to ASP.NET Core infrastructure and makes testing difficult. Instead, pass only the specific HTTP context properties that your code actually needs.
 
-#### Don't
+#### Don't ❌
 
 ```csharp
 using Microsoft.AspNetCore.Http;
@@ -31,7 +31,7 @@ class TestClass : ISomething
 }
 ```
 
-#### Do
+#### Do ✅
 
 ```csharp
 interface ISomething

doc/AG0010.md

@@ -8,7 +8,7 @@ https://agoda-com.github.io/standards-c-sharp/unit-testing/be-wary-of-refactorin
 
 Test fixture inheritance should be avoided as it creates hidden dependencies, makes tests harder to understand, and violates the test isolation principle. Each test class should be independent and self-contained.
 
-#### Don't
+#### Don't ❌
 
 ```csharp
 [TestFixture]
@@ -18,7 +18,7 @@ public class TestClass : BaseTest  // Noncompliant - inheriting from base test c
 }
 ```
 
-#### Do
+#### Do ✅
 
 ```csharp
 [TestFixture]
@@ -46,4 +46,4 @@ Instead of inheritance, prefer:
 - Shared test data factories
 - Clear, explicit test setup within each test class
 
-This keeps tests more maintainable and easier to understand at a glance.
+This keeps tests more maintainable and easier to understand at a glance.

doc/AG0011.md

@@ -8,7 +8,7 @@ https://agoda-com.github.io/standards-c-sharp/services/framework-abstractions.ht
 
 Direct access to `QueryString` collection bypasses ASP.NET model binding, which provides validation, type conversion, and security features. Always use model binding with action method parameters instead.
 
-#### Don't
+#### Don't ❌
 
 ```csharp
 public ActionResult Index()
@@ -18,7 +18,7 @@ public ActionResult Index()
 }
 ```
 
-#### Do
+#### Do ✅
 
 ```csharp
 public ActionResult Index([FromQuery] int id)

doc/AG0012.md

@@ -8,7 +8,7 @@ https://agoda-com.github.io/standards-c-sharp/testing/tests-as-a-specification.h
 
 Each test method should contain at least one assertion to verify expected behavior. Tests without assertions don't validate anything and provide a false sense of security.
 
-#### Don't
+#### Don't ❌
 
 ```csharp
 using NUnit.Framework;
@@ -23,7 +23,7 @@ public class TestClass
 }
 ```
 
-#### Do
+#### Do ✅
 
 With NUnit:
 

doc/AG0013.md

@@ -8,7 +8,7 @@ https://agoda-com.github.io/standards-c-sharp/unit-testing/use-test-cases-approp
 
 Test methods with too many input parameters become difficult to understand and maintain. When test cases need many parameters, split them into smaller, more focused tests with clear intentions.
 
-#### Don't
+#### Don't ❌
 
 ```csharp
 [Test]
@@ -19,7 +19,7 @@ public void This_Is_NotValid(int a, int b, int c, int d, int e, int f)
 }
 ```
 
-#### Do
+#### Do ✅
 
 ```csharp
 [Test]
@@ -50,9 +50,8 @@ Having too many test parameters creates problems:
 Instead:
 
 - Split into multiple focused tests
-- Use test helper methods
 - Create test data builders
 - Use meaningful parameter names
 - Group related parameters into objects
 
-This keeps tests clear, maintainable, and easier to understand.
+This keeps tests clear, maintainable, and easier to understand.

doc/AG0018.md

@@ -17,7 +17,7 @@ When exposing collections through public APIs, you should return interface types
 ### Allowed Return Types
 
 - `IEnumerable<T>`
-- `ISet<T>` 
+- `ISet<T>`
 - `IList<T>`
 - `IDictionary<K,V>`
 - `IReadOnlyDictionary<K,V>`

doc/AG0019.md

@@ -59,5 +59,6 @@ Using `InternalsVisibleTo` leads to several problems:
 - Makes refactoring more difficult as tests depend on internal structure
 - Violates encapsulation principles
 - Can hide poor API design by allowing tests to bypass public interfaces
+- Avoids you doing whitebox testing, and focuses you more towards blackbox testing which leads to easier refactoring of code in the long run
 
-Focus on testing behavior through public interfaces rather than implementation details through internal members.
+Focus on testing behavior through public interfaces rather than implementation details through internal members.