tunahanaliozturk/secure-dotnet-skills

Revisión de Diseño SOLID

Indica al agente que recorra una clase o módulo de C# a través de cada principio SOLID utilizando lentes concretas de .NET, nombrando el olor, el principio violado y la refactorización mínima — priorizando las violaciones que causan acoplamiento o cambios reales, no fallos teóricos.

Cuándo usarlo

• Una clase se está volviendo difícil de manejar — abarca IO, lógica de negocio, mapeo y efectos secundarios en un solo archivo.
• Un PR introduce una nueva variante de tipo y una cadena existente de switch o if-else debe editarse para adaptarla.
• Los constructores crean sus propias dependencias con new o hacen referencia directamente a DateTime.Now / helpers static.
• Una interfaz ha acumulado miembros para satisfacer a múltiples llamadores no relacionados, forzando implementaciones vacías (no-op).

Proceso

1. Identifica la clase/módulo bajo revisión. Nombra su responsabilidad declarada. Si el nombre requiere "y" o "o" para describirla, eso ya es una señal.
2. Recorre cada principio SOLID en orden utilizando las lentes de C# a continuación. Trata cada uno como una pasada separada; no te detengas después del primer hallazgo.
3. Nombra el olor y el principio violado. "Constructor pesado que crea su propio HttpClient y SqlConnection" → DIP; "override base que lanza NotSupportedException" → LSP. Sé preciso.
4. Propón la refactorización mínima con el mecanismo de C#. Nombra el tipo, patrón o cambio de registro DI — no solo "aplica el principio." El consejo genérico de "seguir SOLID" no es un hallazgo.
5. Prioriza por impacto real. Un servicio dios que mezcla persistencia EF con envío de correos electrónicos causa cambios cada sprint; un IOrderService con 12 miembros que ningún llamador usa por completo es una violación genuina de ISP. Una clase sealed que satisface dos casos de uso poco relacionados no viola SRP si tiene una sola razón para cambiar. Distingue las violaciones que perjudican de las que son meramente impuras.

Verificaciones .NET / Azure

SRP — Principio de Responsabilidad Única

• Servicios dios / clases dios. Una sola clase que toca EF Core (_context.SaveChangesAsync), envía correo (_emailSender.SendAsync), valida reglas de negocio y mapea a DTOs tiene múltiples razones para cambiar. Extrae un IOrderValidator, un IOrderRepository (o usa el DbContext directamente en una clase de persistencia dedicada) y un IOrderNotifier como colaboradores separados.
• Controladores grasos. Un controlador ASP.NET Core que contiene lógica if/validación, llama a la base de datos directamente y formatea las cargas útiles de respuesta está haciendo tres trabajos. Los controladores deben orquestar: llamar a un servicio, mapear el resultado, devolver IActionResult. Las reglas de negocio pertenecen a la capa de aplicación/dominio.
• "Y" en el nombre del tipo o la responsabilidad. OrderValidationAndPersistenceService es un olor en el nombre que señala una violación de SRP. Si solo puedes describir la clase usando "y", divídela.
• Múltiples razones para cambiar. Pregunta: "¿Necesitaría cambiar esta clase si cambiamos de proveedor de correo? ¿Si cambiamos el esquema de la base de datos? ¿Si cambian las reglas de negocio?" Más de un "sí" significa más de una responsabilidad.

OCP — Principio de Abierto/Cerrado

• switch / if-else creciente sobre un discriminador de tipo. Un bloque switch(order.Type) dentro de un servicio que debe reabrirse para cada nuevo valor de enumeración OrderType viola OCP. Reemplázalo con una estrategia: define IOrderHandler (o IOrderProcessor) y registra una implementación por tipo; resuelve mediante IEnumerable<IOrderHandler> inyectado en el coordinador, despachando con handler.Handles(order.Type).
• Expansión de comportamiento hardcodeada. Si agregar una nueva regla de descuento, canal de notificación o formato de exportación requiere editar una clase existente, en lugar de agregar un nuevo IDiscountRule o IExporter, la clase no está cerrada para modificación. Introduce la abstracción antes de que llegue el segundo caso.
• Antipatrón de discriminador de tipo. if (order is PremiumOrder) o if (typeof(T) == typeof(FooOrder)) dentro de una clase de procesamiento es un olor de código equivalente al switch anterior. El polimorfismo o un registro de estrategias eliminan estos.

LSP — Principio de Sustitución de Liskov

• Overrides que lanzan throw new NotSupportedException() / NotImplementedException(). Un subtipo que no puede honrar un contrato base (por ejemplo, ReadOnlyCollection<T> lanzando en Add) viola LSP. Si el override no se puede implementar de manera significativa, la jerarquía es incorrecta — prefiere composición o interfaces separadas.
• Subtipo que debilita postcondiciones. Si la clase base garantiza que GetOrders() nunca devuelve null, un override que devuelve null viola el contrato del que dependen los llamadores. Documenta invariantes (XML doc o contratos) y aplícalos en los overrides.
• Subtipo que fortalece precondiciones. Un override que agrega una comprobación de null no presente en la base, o que rechaza un rango de parámetros válido que la base aceptaba, rompe el código que pasa entrada válida del contrato base. El subtipo debe aceptar todo lo que aceptaba la base.
• Downcasts generalizados con is/as en el código llamador. if (handler is SpecialHandler sh) sh.SpecialMethod() en el llamador significa que la abstracción está rota — o el método pertenece a la interfaz base, o los tipos no deberían compartir una jerarquía. Los downcasts frecuentes son una señal de fallo de LSP / abstracción.

ISP — Principio de Segregación de Interfaces

• Interfaces grasas (10+ miembros) con preocupaciones no relacionadas. Un IOrderService que expone CreateOrder, CancelOrder, GetInvoice, SendConfirmation, ExportToCsv, CalculateTax, ValidateAddress y GetShippingRates obliga a cada implementador y doble de prueba a implementar métodos que no utiliza. Divídelo en interfaces de rol: IOrderWriter, IOrderReader, IOrderNotifier, IOrderExporter.
• Implementaciones vacías (no-op) / stubs con NotImplementedException. Si una implementación mock o de prueba devuelve throw new NotImplementedException() para la mitad de los miembros de la interfaz, esos miembros pertenecen a una interfaz de rol diferente. La presencia de no-ops es evidencia directa de violación de ISP.
• Clientes que importan métodos que nunca llaman. Si PaymentService depende de IOrderService pero solo llama a GetOrder, debería depender de IOrderReader. Reducir la dependencia hace que el acoplamiento sea explícito y las pruebas más económicas.

DIP — Principio de Inversión de Dependencias

• Creación de dependencias concretas con new dentro de una clase. new HttpClient() construido dentro de un servicio crea una dependencia oculta, evita el contenedor DI, pierde sockets (sin agrupación de conexiones) y hace que la clase no sea comprobable sin trucos de reflexión. Inyecta IHttpClientFactory (cliente tipado) o un cliente nombrado a través del constructor. Mismo patrón para new SqlConnection(connectionString) — inyecta IDbConnectionFactory o deja que EF Core / Dapper maneje la conexión a través del DbContext registrado.
• Depender de concretos en lugar de abstracciones. Un constructor tipado a SqlOrderRepository en lugar de IOrderRepository acopla la clase a la tecnología de almacenamiento. Cualquier clase que deba ser reutilizable o comprobable de forma aislada debe depender solo de interfaces o tipos abstractos.
• Acoplamiento oculto a DateTime.Now / estado estático. var now = DateTime.Now dentro de un método de negocio hace que la clase sea no determinista y no comprobable sin trucos de viaje en el tiempo. Introduce ISystemClock (o usa TimeProvider de .NET 8+) e inyéctalo. Lo mismo aplica a Environment.MachineName, Random.Shared o cualquier otro estático ambiental.
• Singletons mutables estáticos. Un static Dictionary<string, Order> _cache en una clase de servicio es estado compartido oculto que evita la gestión del ciclo de vida de DI y no es seguro para uso concurrente. Reemplázalo con IMemoryCache / IDistributedCache inyectado a través de DI, que soporta alcance adecuado y evacuación.

Nota de juicio: no todo new es una violación de DIP. new OrderDto(order) (un contenedor de datos simple) o new List<T>() (una colección estándar) están bien. DIP importa cuando el objeto construido tiene comportamiento, E/S externa o configuración — es decir, cuando intercambiarlo por un doble de prueba o una implementación alternativa sería valioso.

Señales de alerta

Señal	Por qué importa
new HttpClient() construido dentro de un método o constructor de servicio	Evita la agrupación de conexiones de IHttpClientFactory (agotamiento de sockets bajo carga), oculta la dependencia y hace que la clase no sea comprobable sin E/S de producción.
switch(order.Type) o if (x is ConcreteType) en una clase coordinadora	Debe reabrirse para cada nueva variante; los llamadores saben demasiado sobre la jerarquía de tipos. Reemplázalo con IEnumerable<IHandler> resuelto desde DI.
throw new NotImplementedException() o throw new NotSupportedException() en un override no abstracto	El subtipo no puede honrar el contrato base — violación de LSP. Implementa el método correctamente o reestructura la jerarquía.
Una interfaz con 10+ miembros donde los llamadores usan 2–3 de ellos	Obliga a dobles de prueba grandes; señala que múltiples interfaces de rol se han colapsado en una.
Una clase que construye su propio DbContext mediante new AppDbContext(options)	Evita la gestión del ciclo de vida de DI (Scoped vs Transient vs Singleton), previene la agrupación de conexiones y rompe la comprobabilidad. Inyecta AppDbContext o IDbContextFactory<AppDbContext>.
DateTime.Now llamado directamente dentro de un método de negocio	No determinista bajo pruebas; el acoplamiento al reloj del sistema oculta una dependencia lógica. Inyecta TimeProvider (.NET 8+) o ISystemClock.
Una acción de controlador que contiene lógica de negocio if/switch o consultas EF	El controlador ha asumido responsabilidades que pertenecen a la capa de aplicación/dominio; un solo cambio en las reglas de negocio requiere tocar el controlador.
Campo mutable static usado como caché en proceso	No es seguro para hilos sin bloqueo explícito; evita el alcance de DI; no se puede evacuar ni observar. Usa IMemoryCache.

Ejemplo

Consulta examples/solid-review/.