Jak uniknąć DRY w tym kodzie?

Cześć, chciałbym zapytać o wskazówke/poradę w jaki sposób najlepiej wydzielić kod, żeby nie powtarzać copy pasta w tych metodach, które obecnie różnią się jedynie nazwą i rozszerzeniem LogLevel?

{
    public static class LoggerExtensions
    {
        public static void LogError(this ILogger logger, Exception exception, string message, IHandlerContext<IBasicMessage> ctx)
        {
            logger.Log(LogLevel.Error, exception, $"{message}. Message: {message}", JsonConvert.SerializeObject(ctx.Message));
        }

        public static void LogWarning(this ILogger logger, Exception exception, string message, IHandlerContext<IBasicMessage> ctx)
        {
            logger.Log(LogLevel.Warning, exception, $"{message}. Message: {message}", JsonConvert.SerializeObject(ctx.Message));
        }

        public static void LogInfo(this ILogger logger, Exception exception, string message, IHandlerContext<IBasicMessage> ctx)
        {
            logger.Log(LogLevel.Information, exception, $"{message}. Message: {message}", JsonConvert.SerializeObject(ctx.Message));
        }
    }
}

tak czy siak na jakims poziomie bedziesz tego potrzebował - możesz zrobić coś a'la dispatcher

1. Błąd logiczny - DRY się nie unika, tylko na odwrót, stara się go osiągnąć. DRY - do not repeat yourself. Logicznie rzecz biorąc unikanie DRY = powtarzanie się jak najwięcej.
2. Robisz jedną metodę i dodajesz typ wiadomości dla loga: error / warning / info zamiast robić trzy metody; ewentualnie robisz to niestatycznie i określasz to w zmiennej a potem set/get

tak w ogóle, to jest raczej KISS - zostawienie w obecnej formie imo byłoby w pełni akceptowalne

A w czym ci przeszkadza, że masz tutaj coś zduplikowane?
To jest tak mały i niezależny od innych elementów fragment, że zupełnie bym się nie przejmował jakąś minimalną duplikacją.

Jedyne co warto byłoby zrobić to wydzielić szablon komunikatu do jakiejś zmiennej i tyle.

some_ONE napisał(a):

A w czym ci przeszkadza, że masz tutaj coś zduplikowane?

Zauważ, że czym więcej zduplikowanego kodu, tym więcej pracy np. przy refektoryzacji i jakichkolwiek innych zmianach. Można nawalić setki tysięcy linii niepotrzebnie skopiowanego kodu, bo tak łatwiej i wygodniej, ale potem oznacza to wiele dodatkowej pracy przy zmianach.

Dlatego warto myśleć o DRY

No chyba, że ktoś tak specjalnie robi, żeby więcej zarobić, ale to wątpliwe etycznie.

Z drugiej strony dbanie o DRY oznacza zazwyczaj więcej pracy na początku, więc to zależy - kod nigdy nie będzie zmieniany? Wtedy można DRY olać.

Yyy, po prostu stwórz prywatną metodę do której będziesz przekazywał LogLevel?

public static void LogInfo(this ILogger logger, Exception exception, string message, IHandlerContext<IBasicMessage> ctx)
{
    Log(LogLevel.Information, logger, exception, message, ctx);
}

private static Log(LogLevel logLevel, this ILogger logger, Exception exception, string message, IHandlerContext<IBasicMessage> ctx)
{
   logger.Log(logLevel, exception, $"{message}. Message: {message}", JsonConvert.SerializeObject(ctx.Message));
}

Ukrywasz szczegóły formatowania, nie masz powtórzeń, nie masz większych udziwnień, nie wiem co tu można by lepszego wymyślić w tak prostym kodzie.

W tym kodzie DRY jest już zupełnie dobrze uniknięte.

TomRZ napisał(a):

Zauważ, że czym więcej zduplikowanego kodu, tym więcej pracy np. przy refektoryzacji i jakichkolwiek innych zmianach. Można nawalić setki tysięcy linii niepotrzebnie skopiowanego kodu, bo tak łatwiej i wygodniej, ale potem oznacza to wiele dodatkowej pracy przy zmianach.

Dlatego warto myśleć o DRY

No chyba, że ktoś tak specjalnie robi, żeby więcej zarobić, ale to wątpliwe etycznie.

Wszystko fajnie, ale teraz do konkretów.
To jest jedna klasa, która na 99,99% nie przekroczy nigdy 100 linii kodu.

Można wszystko wydzielić do metod, z tym się zgadzam i na pewno to nie zaszkodzi.
Ale nie przesadzałbym z tym, że kilka powtórzonych linii w takim miejscu to wielki problem i trzeba to za wszelką cenę wydzielać.
Drobne pogwałcenie DRY tutaj nie wpływa na inne części systemów. To nie tak, że teraz przy zmianie formatu logowania będzie szukał po całym projekcie odwołań do loggera. Ma to zamknięte w jednej małej klasie, a że w środku jest minimalna duplikacja? Jak dla mnie taka duplikacja to najmniejszy problem jaki może być.

Dlatego zapytałem jaki konkretnie widzi problem tutaj z powtórzonymi trzema linijkami kodu.

some_ONE napisał(a):

TomRZ napisał(a):

[...]

Ale nie przesadzałbym z tym, że kilka powtórzonych linii w takim miejscu to wielki problem i trzeba to za wszelką cenę wydzielać.
[...]
Dlatego zapytałem jaki konkretnie widzi problem tutaj z powtórzonymi trzema linijkami kodu.

Ja piszę ogólnie, a nie o tej konkretnej klasie.

Tyle tylko, że kiedy takich małych klas pozbawionych DRY zgromadzi się setka, to znowu mamy problem.

To jak z klasycznym "jeden włos wyrwany z głowy nie robi różnicy" - gdyby tak było, kudłaci byliby łysymi, a łysi kudłatymi.

No to w ogólności się zgadzam, ale ten wątek dotyczy konkretnego przypadku :P

Nawet jak będzie miał i 100 klas i w każdej coś zduplikowane (ale wciąż duplikacja tylko na poziomie tej klasy) to nie jest to jeszczę największy problem.
IMO dużo większym problemem jest duplikacja/pogwałcenie DRY na poziomie całych komponentów/modułów aplikacji, a nie w ramach pojedynczej zamkniętej klasy.

Tukanowski napisał(a):

Cześć, chciałbym zapytać o wskazówke/poradę w jaki sposób najlepiej wydzielić kod, żeby nie powtarzać copy pasta w tych metodach, które obecnie różnią się jedynie nazwą i rozszerzeniem LogLevel?
[...]

public void LogError(Exception exception, string message, IHandlerContext<IBasicMessage> ctx)
{
    Log(LogLevel.Error, exception, message, ctx);
}

public void LogWarning(Exception exception, string message, IHandlerContext<IBasicMessage> ctx)
{
    Log(LogLevel.Warning, exception, message, ctx);
}

public void LogInfo(Exception exception, string message, IHandlerContext<IBasicMessage> ctx)
{
    Log(LogLevel.Information, exception, message, ctx);
}

private void Log(LogLevel level, Exception exception, string message, IHandlerContext<IBasicMessage> ctx)
{
    logger.Log(leveln, exception, $"{message}. Message: {message}", JsonConvert.SerializeObject(ctx.Message));
}

nie wiem, wymuszony ten refactor.

DRY imo bardziej odnosi się do intencji (dwa kody robiące biznesowo to samo), niż dosłownie kodu (dwa kody robiące dwie różne biznesowo rzeczy, ale jakimś cudem kod podobny)

i tak - Error, Warning oraz Info mają swoje własne formaty, a to że akurat wyszedł ten sam, to już , co najwyżej wywaliłbym go do jakiegoś const template, ale generalnie nie jest to wybitnie dobry przykład na tego typu refactor

Przede wszystkim, czemu Ty tego w ogóle potrzebujesz? Co to za ILogger jest, że trzeba mu takie banały dospawywać samodzielnie?