Jak się pozbyć Kopiego-Pasta?

bool trajEgejn = false;
do
{
    try
    {
        SkomplikowanaKlasa coś = BardzoSkomplikowanaMetoda();
        if (this.CheckCoś(coś))
        {
            Console.WriteLine("ok");
            return true;
        }
        else
        {
            Console.WriteLine("dupa");
            if (!this.SpróbujNaprawićDupę(jakieś parametry))
            {
                return false;
            }
            else
            {
                trajEgejn = true;
            }
        }

    }
    catch (BardzoZłyWyjątek)
    {
        Console.WriteLine("dupa");
        if (!this.SpróbujNaprawićDupę(jakieś parametry))
        {
            return false;
        }
        else
        {
            trajEgejn = true;
        }
    }    
} while(trajEgejn);
 

Mam jakieś zaćmienie umysłu... Da się jakoś sensownie pozbyć powielenia tego kodu?

 else throw new BardzoZłyWyjątek();

Wydzielić to do funkcji? Wyjątki powinno się rzucać tylko w sytuacjach wyjątkowych. Dodatkowo na CLR wyjątki działają wolno. Ponadto flagę + pętlę do while możesz zastąpić rekurencją ogonową :) CLR, ZTCW optymalizuje rekurencję ogonową do pętli (w przeciwieństwie do HotSpota).

Ech, magistry. ;P

bool trajEgejn = false;
do
{
    bool shit = false;
    try
    {
        SkomplikowanaKlasa coś = BardzoSkomplikowanaMetoda();
        if (this.CheckCoś(coś))
        {
            Console.WriteLine("ok");
            return true;
        }
        else
        {
            shit = true;
        }
    }
    catch (BardzoZłyWyjątek)
    {
        shit = true;
    }  
    
    if (shit)
    {
        Console.WriteLine("dupa");
        if (!this.SpróbujNaprawićDupę(jakieś parametry))
        {
            return false;
        }
        else
        {
            trajEgejn = true;
        }
    }  
} while(trajEgejn);
 

Ale dalej mi się nie podoba.

EDIT:
Przeróbka zgodnie z pomysłem z komentarza:

bool trajEgejn = false;
do
{
    try
    {
        SkomplikowanaKlasa coś = BardzoSkomplikowanaMetoda();
        if (this.CheckCoś(coś))
        {
            Console.WriteLine("ok");
            return true;
        }
    }
    catch (BardzoZłyWyjątek)
    {
    }  
    
    Console.WriteLine("dupa");
    if (!this.SpróbujNaprawićDupę(jakieś parametry))
    {
        return false;
    }
    else
    {
        trajEgejn = true;
    }
} while(trajEgejn);
 

To jest cała metoda? Jeśli tak to proponuję wersję z rekurencją ogonową (zakładając short circuiting, który jest w standardzie):

bool metoda() {
  try {
    SKlasa coś = BSMetoda();
    if (checkCoś(coś))
      return true;
  } finally {
    return spróbujNaprawić() && metoda();
  }
}

Ewentualnie można pójść w nieco inną stronę i napisać:

bool metoda() {
  try {
    return checkCoś(BSMetoda()) || (spróbujNaprawić() && metoda());
  } finally {
    return spróbujNaprawić() && metoda();
  }
}

ale tu już się kod powtarza.

PS:
Nie wiem jak zachowa się return w finally. Być może trzeba go przenieść za finally.

Update:
Finally zawsze się wykonuje się jednak trzeba przenieść.

@Wibowit: podejście ładnie wyglądające w edytorze, w praktyce dokładające jeszcze jeden problem: kiedy błąd występuje za każdym razem kończy się stos. nieskończona pętla jest milion razy lepsza
@somekind: po kiego grzyba tyle niepotrzebnych zmiennych?

while (true)
{
    try
    {
        SkomplikowanaKlasa coś = BardzoSkomplikowanaMetoda();
        if (this.CheckCoś(coś))
            return true;
    }
    catch (BardzoZłyWyjątek ex) { }  

    if (!this.SpróbujNaprawićDupę(jakieś parametry))
        return false;
}

Sorka z deka, że odkopuję (uznałem, że nie będę zakładał nowego, a może ktoś obserwuje jeszcze ten), ale też mam problem z powielaniem tego samego kodu przy różnych wyjątkach, jak to przerobić żeby kawałek kodu zawsze się wykonywał podczas jakiegokolwiek wyjątku?

try
{
    // coś ryzykownego
}
catch (Exception)
{
    zróbCoś();
    coś("1");
}
catch (OtherException1)
{
    zróbCoś();
    coś("1");
}
catch (OtherException2)
{
    zróbCoś();
    coś("2");
}
catch (OtherException3)
{
    zróbCoś();
    coś("3");
}
catch (OtherException4)
{
    zróbCoś();
    coś("4");
}

zróbCoś() powinno wywołać się zawsze przy jakimkolwiek wyjątku. Pomysły? Pierwsza myśl zrobić zewnętrzny try i w każdym z tych catchów wywalać nowy inny, który zostanie złapany w zewn. try i tam obsłużony, tylko i tak w każdym catchu będzie throw new. :|

@xeo545x39, a nie możesz tego dać do finally? Co się dzieje w tym róbCoś?

Ale finally zawsze się wykona co nie? A mi potrzeba tylko przy wyjątkach. róbCoś to tylko w sumie return, aby wyszedł z metody, nie robiąc nic dalej, czyli nie odpalając socket'a, bo np. nie ma połączenia z bazą czy coś.

Możesz zrobić flagę, przestawić ją przed pierwszym catchem, a potem sprawdzić ją w finally.

Myślałem nad flagą, ale w inny sposób, bo tego nie rozumiem. Jak do przed pierwszym catchem? Tzn.? Możesz podać przykład?

Zrób sobie tak:

int err = -1;
try
{
	//...
}
catch (Exc1)
{
	err = 1;
}
catch (Exc2)
{
	err = 2;
}
//itd..
finally
{
	if ( err >= 0 )
	{
		zróbCoś();
		coś(err);
	}
}

@stfu Taka była pierwotna koncepcja, ale i tak bym miał powielenie, bo wszędzie trzeba by ustawiać dodatkowo err na jakąś wartość. Zrobiłem tak:

bool error = true;

try
{
    // coś ryzykownego

    error = false;
}
catch (Ex1)
{
    Log.Write("treść dla I");
}
// itd.

if (error)
    return;

Nie mogłem dać ifa do finally, bo "Cannot leave finally clausule...".

try
{
    // coś ryzykownego
}
catch (Exception e)
{
    zróbCoś(e);
}

ale ładne to to moim zdaniem nie jest.

chyba że zróbCoś to jakaś obsługa błędów, np zapis do loga — wtedy sensowne jest, by funkcja przyjmowała wyjątek jako parametr.

a może tak:

try
{
    // coś ryzykownego
}
catch (Exception e)
{
    zróbCoś();
    try
    {
        throw e;
    }
    catch (OtherException1)
    {
      coś("1");
    }
    catch (OtherException2)
    {
        coś("2");
    }
    catch (OtherException3)
    {
        coś("3");
    }
    catch (OtherException4)
    {
        coś("4");
    }
}

@up -

try
{
    throw;
}

nie

try
{
    throw e;
}

to drugie usuwa stacktrace.

To przerzucanie wątków jakieś takie brzydkie i wtfowe trochę.

to drugie usuwa stacktrace.
wiem, chociaż świadomie użyłem throw e. zresztą to zależy co pytający chce osiągnąć.

To przerzucanie wątków jakieś takie brzydkie i wtfowe trochę.
bo i problem jest wtf-owy.

Pytanie za 100 punktów - czemu w zależności od typu wyjątku jest logowane co innego?

Bo jak się rzucaloguje wyjątek to można podać powód.

Kod w Scali:

class Exception1 extends Exception
class Exception2 extends Exception
class Exception3 extends Exception
 
object Main {
  def main(args: Array[String]) {
    try {
      throw new Exception2()
    } catch { 
      case x => {
        println("Exception")
        println(x match {
          case _: Exception1 => "1"
          case _: Exception2 => "2"
          case _: Exception3 => "3"
          case _ => "Unknown"
        })
      }
    }
  }
}

http://ideone.com/BxlRY
Dawno nie pisałem w Scali więc może trochę nieoptymalnie.

To nie jest przekonujące uzasadnienie, wyjątek ma zapewne metodę getMessage(), może tez getLocalizedMessage().
Natomiast nie wszystkie wyjątki maja takie same metody, przykład z Javy:

        catch (SQLException e)
        {
            wyświetl/zapisz "Błąd SQL: "+e+" "+e.getErrorCode()+" "+e.getSQLState();
        }
        catch (Exception e)
        {
            wyświetl/zapisz "Błąd: "+e;
        }

No, ale nawet jak masz zwykłe Exceptiony bez specjalnych metod to możesz zrobić:

        catch (Exception1 e)
        {
            wyświetl/zapisz "Błąd 1";
        }
        catch (Exception2 e)
        {
            wyświetl/zapisz "Błąd 2";
        }

I tutaj przecież "w zależności od typu wyjątku jest logowane co innego".

PS: Walnąłem się nieco z jednym słowem :)

Ale nie musisz tak robić, masz do wyboru:

catch(Exception e)
{
     wyświetl/zapisz e.getMessage();
     wyświetl/zapisz e.getLocalizedMessage();
     wyświetl/zapisz e;
}

Ale co gdy ktoś chce inne opisy dla różnych typów exceptionów?

Ciężko się z Tobą dyskutuje, jak Ci się obali jeden argument, to wyciągasz drugi. Zacząłeś od

Bo jak się rzuca loguje wyjątek to można podać powód.

Teraz wyciągasz argument

ktoś chce inne opisy
.
Dotychczas sądziłem, że z Tobą nie da się dyskutować tylko o polityce, gospodarce, kościele, JKM i monarchii. Teraz skłaniam się do poglądu, że nie da się o niczym.

Opis powodu.

Jak zapiszesz NPE w logach to chyba niewiele ci to będzie mówić. Więc trzeba dać opis prawdziwego powodu, np funkcja nie znalazła czegoś tam, zwróciła nulla i potem ten null gdzieś został zdereferencjonowany.

Dotychczas sądziłem, że z Tobą nie da się dyskutować tylko o polityce, gospodarce, kościele, JKM i monarchii. Teraz skłaniam się do poglądu, że nie da się o niczym.

Kolejny zakuty łeb?

Wibowit, przecież od tego są argumenty konstruktora / właściwości i przeładowane getmessage.

Jak exception leci z kosmosu (tzn np VMki) czy z zewnętrznej biblioteki (zresztą nawet jakbyś miał źródła danej biblioteki, to ona sama niekoniecznie wie jaki jest kontekst wywołania) to nie użyjesz tych metod.

Dobra, jakiś przykład z d**y:

catch (FileNotFoundException ex) {
  loguj("nie można znaleźć skryptu", ex);
}
catch (AuthenticationException ex) {
  loguj("złe hasło", ex);
}

Naprawdę nigdy nie robiliście czegoś takiego?

Loguję tylko własne wyjątki, którym ustawiam InnerException.

Tzn tworzysz wyjątki tylko po to, żeby je logować? Wyjątki tworzy się po to, żeby nimi rzucać ;)

Wibowit napisał(a)

Dobra, jakiś przykład z d**y:

catch (FileNotFoundException ex) {
  loguj("nie można znaleźć skryptu", ex);
}
catch (AuthenticationException ex) {
  loguj("złe hasło", ex);
}

Naprawdę nigdy nie robiliście czegoś takiego?

O tym, co się stało, mówi właściwość Message. Po co powielać informację?