CodelyTV / refactoring-code_smells-design_patterns

♻️ Example projects illustrating Code Smells in order to apply Refactoring techniques
https://codely.tv/pro/cursos
569 stars 133 forks source link

Añadir proyectos de ejemplo en C# #18

Closed solvingproblemswithtechnology closed 3 years ago

solvingproblemswithtechnology commented 3 years ago

Buenas!

He pensado en ir añadiendo los ejemplos en C#.

Creo que he respetado en la medida de lo posible la estructura de proyectos y commits, pero he tenido que hacer un par de adaptaciones a la estructura de proyectos de .Net. Son las siguientes:

Si no os convence la estructura, se podría dejar de la siguiente manera (aunque quedaría más complejo):

Ahora:

csharp
    csharp.sln
    csharp-booking-01_base/
        csharp-booking-01_base.csproj
        Booking/
        Test/

Otra opción:

csharp
    csharp-booking-01_base/
        csharp-booking-01_base.sln
        src/main/
            Booking/
            csharp-booking-01_base.csproj
        src/test/
            Test/
            csharp-booking-01_base.tests.csproj

Aunque serían dos proyectos independientes e igual es un poco más dificil de usar. En función de lo que me digáis creo el resto de proyectos.

Si necesitáis cualquier aclaración o que intente explicarme mejor, no dudéis en decirlo 😉

Un saludo y gracias por el cursazo! 😁

P.D: Para ejecutar, instalar el SDK de .Net Core 3.1 y ejecutar dotnet test

Leanwit commented 3 years ago

Hola smartcodinghub En principio lo ideal es tener un solution file por cada ejemplo ya que la idea es que se analice cada caso de a uno a la vez. Asi que la segunda opcion me parece mas adecuada aunque le harias algunos cambios minimos:

csharp
    csharp-booking-01_base/
        csharp-booking-01_base.sln
        src/
            Booking/
                Booking.csproj
        test/
            Booking.Tests/
                Booking.Tests.csproj

Basicamente omitiendo src/main/ y src/test en solamente src y test donde ambas folder a su vez son solution folder en la estructura y generando un proyecto para el codigo y otro para el testing.

Que opinas al respecto? Te parece bien, alguna sugerencia o feedback?

Por si sirve, te paso unos comandos de CLI para replicar esta estructura como tal y crear las solutions folder.

// Creacion del proyecto Booking y asociacion con la solution
- dotnet new classlib -n 'Booking' -o 'csharp-booking-01_base/src/Booking' --framework netcoreapp3.1
- dotnet sln csharp-booking-01_base.sln add csharp-booking-01_base/src/Booking/Booking.csproj -s 'src'

// Creacion del proyecto Booking.Tests y asociacion con la solution
- dotnet new xunit -n 'Booking.Tests' -o 'csharp-booking-01_base/test/Booking.Tests' --framework netcoreapp3.1
- dotnet sln csharp-booking-01_base.sln add csharp-booking-01_base/test/Booking.Tests/Booking.Tests.csproj -s 'test'

Con respecto al namespace, sugerio editar el proyecto y editar el root namespace como: <RootNamespace>CodelyTv.Booking</RootNamespace>para Booking.csproj <RootNamespace>CodelyTv.Booking.Tests</RootNamespace> para Booking.Tests.csproj

Aunque aca @rgomezcasas y @JavierCane van a poder opinar mejor si CodelyTv.Booking esta bien como namespace o cambiarlo a Tv.Codely.Booking como esta en los ejemplos de Java. Sobre todo hago esta pregunta porque recuerdo haber escuchado que Tv.Codely estaba armado especificamente por unas limitaciones o requerimientos de Java / framework.

Por ultimo y no menor, muchisimas gracias por el PR, la verdad que esta muy bueno poder contar con ejemplos de c# tambien para todos los que venimos de este mundillo.

solvingproblemswithtechnology commented 3 years ago

Me parece bien, aunque preferiría tener una solución para Booking, otra para Chat, etc...

Pero lo cambio sin problemas.

Sobre el namespace, yo opino de cambiarlo, sí. No me gusta nada como se hace en Java, así que encantado ^^.

Por cierto, pensaba actualizarlos a .Net 5.0.

Leanwit commented 3 years ago

Me parece bien, aunque preferiría tener una solución para Booking, otra para Chat, etc...

Entiendo, tiene como punto a favor evitar tener muchas soluciones y agrupar ejemplos que son del mismo contexto. Dejo a criterio de Rafa y Javi con respecto a este punto. Entiendo que ellos apuntaban a un ejemplo a la vez, pero es una posibilidad.

Por cierto, pensaba actualizarlos a .Net 5.0.

Claro que si, estaria genial si lo haces en .Net 5.0 :tada:

solvingproblemswithtechnology commented 3 years ago

Lo he cambiado. Espero a ver que comentan sobre lo de tener una solución por cada contexto.

Por ultimo y no menor, muchisimas gracias por el PR, la verdad que esta muy bueno poder contar con ejemplos de c# tambien para todos los que venimos de este mundillo.

Creo que no lo dije antes, pero encantado por poder ayudar. Creo que .Net tiene muchísimo potencial y mucha gente lo "ignora" por el pasado que ha tenido y lo acoplado que estaba a Windows y Microsoft. No obstante, creo que el cambio de mentalidad le ha sentado muy bien y las mejoras son impresionantes.

solvingproblemswithtechnology commented 3 years ago

Por cierto @Leanwit , que opinas de usar records (C# 9.0) en lugar de clases? Hay bastantes clases que se podrían reemplazar.

También he añadido un script powershell para crear la estructura de ejemplo (compatible con Powershell Core para linux). Debería estar en un makefile?

Leanwit commented 3 years ago

Con respecto a los namespaces, ahí corrobore que en java es un estandar definir el namespace como 'dominio.compañía.projecto', por ese caso es tv.codely... pero en .Net no es el caso asi que podemos dejarlo como CodelyTv.Booking.

En caso de la aplicación de clases como records me parece que estaria muy bueno implementarlo, ya que te asegura la inmutabilidad y es un concepto al que CodelyTv siempre apunta. Asi que si tenes que agregar ejemplos de como tiene que quedar una clase correctamente, utiliza records sin ningún problema. Porque de otra forma, estarias creando una clase que tiene el mismo comportamiento que un record pero agregando más código.

Con respecto a lo de powershell, en CodelyTv es un estandar el makefile asi que si no te lleva mucho trabajo pasar el script de powershell a un makefile, sería estupendo.

solvingproblemswithtechnology commented 3 years ago

He cambiado el powershell. Por otra parte, vistos los ejemplos, creo que usar records los puede hacer más confusos entre pasos, así que no los voy a añadir.

Por mí, podéis mergear esta PR y os crearé otra con el resto de ejemplos cuando los tenga.

JavierCane commented 3 years ago

¡Buenas!

Al turrón 🤟:

  1. Mil millones de gracias por la PR. Se agradece infinito que los ejemplos que colgamos en GitHub no sólo sean algo unidireccional que dependa de Codely, si no que la gente también pueda contribuir. De verdad, muchas gracias por el esfuerzo y tiempo invertido tanto en la propuesta por parte de @smartcodinghub como en la review de @Leanwit ❤️

  2. +1 a todo el feedback comentado por @Leanwit. @Leanwit tiene todo el contexto de los cursos ya que desde el inicio de Codely ha estado junto con el proyecto, y tiene experiencia sobrada en C# (cosa que a nosotros nos falta 😅). Con lo cuál, por nuestra parte autonomía a la comunidad para poder seguir avanzando e integrar PRs de C# sin que nosotros seamos bloqueantes 😊

  3. Se agradece enormemente el esfuerzo por seguir los patrones establecidos con los ejemplos en otros lenguajes. Cosas como tener un sln por cada etapa de los ejemplos es algo que buscábamos intencionadamente para permitir que alguien se clone el repo pero pueda abrir el IDE en una etapa determinada de un ejemplo para seguir a partir de ahí. Por facilitar review al resto, ahora mismo quedaría así y está de lujo 🙌

image

  1. Dado que esta PR ya lleva 9 días pendiente (mea culpa), no lo bloqueemos más y, si @Leanwit está OK con ella, integrémosla sin problema

  2. A nivel de iteraciones a posteriori del merge, +1 total a la propuesta de @Leanwit para pasar a usar Records de C# 9. Al igual que con las novedades en otros lenguajes (cambio a uso de property promotion y otras novedades de PHP 8 recientemente en el repo de php-ddd-example), sería de lujo si los repositorios de ejemplos siguieran las reglas más idiomáticas actualmente en cada ecosistema.

Es cierto lo que comenta @smartcodinghub que, a día de hoy, el uso de Records (como el de property promotion en PHP) puede resultar confuso y hasta que no se extiendan más en las bases de código con las que trabajamos día a día seguirá pasando. La postura al respecto de este tipo de cosas es que, siempre que sean cambios por los que el ecosistema (PHP, C#, Java…) apuesta fuertemente (integrándolos a nivel de lenguaje como los ejemplos que hablamos), molaría que se vieran reflejados en los ejempos para que, pasado 1 año, el repo siga siendo "actual" 😊

Un abrazo a todos y mil disculpas en la demora de nuevo. Se agradece infinito este tipo de PRs porque le dan sentido a todo el esfuerzo que hacemos desde Codely por el Open Source. 🙌

solvingproblemswithtechnology commented 3 years ago

¡Buenas!

No pasa nada por la demora, se entiende.

Sobre el tema de los records: cuando suba los siguientes ejemplos, actualizo el paso de tell_dont_ask a usar records. Aunque no tengo claro que aplique del todo, ya que las propiedades serían publicas y no es ese el objetivo del refactor. Por eso me arrepentí de haberlo sugerido, @JavierCane ^^. Si se os ocurre otra forma de añadirlo, lo cambio en un momento.

Un saludo!

Leanwit commented 3 years ago

Perfecto, gracias Javi y Smart por las aclaraciones. Procedo a mergear la branch.