Code review: rzutowanie

Co powiedzie na taki fragment kodu?

FileInfo fileInfo;
if (sender is FileInfo)
    fileInfo = sender as FileInfo;

Konstrukcja jest dość popularna i:

– oczywiście skompiluje się,

– jest bezpieczna na wartość NULL (tzn. nie wyrzuci wyjątku),

– jeśli sender jest innego typu niż FileInfo, kod nie wyrzuci wyjątku.

Co jest w końcu nie tak? Chodzi tutaj o good practice i nie wprowadzanie czytelnika kodu w błąd. Operator is sprawdza czy obiekt jest danego typu. Zatem w instrukcji IF wiemy już,  że sender MUSI być typu FileInfo. Z kolei słówko as próbuje zrzutować dany obiekt na określony typ. Jeśli takowe rzutowanie jest niemożliwe, wtedy po prostu zwracana jest wartość NULL. W przeciwieństwie do castowania typu  fileInfo = (FileInfo)sender, kod nie wyrzuci wyjątku a wartość NULL. Z tego wzglądu lepszym podejściem jest:

FileInfo fileInfo = sender as FileINfo;
if (fileInfo != null)
{
}

Ponadto użycie is oraz as powoduje podwójne rzutowanie – ale o tym w następnym poście…

13 thoughts on “Code review: rzutowanie”

  1. No spoko, ale dla mnie bez sensu.
    A nie lepiej:

    FileInfo fileInfo = sender as FileInfo;
    if(fileInfo!=null){…}

    dwie linijki i bezpieczeństwo że w bloku if() na pewno masz w fileInfo A) nie nulla, B) obiekt interesującego Cię typu.

    Zupełnie nie rozumiem w czym fileInfo = (FileInfo)sender; to rozwiązanie miałoby być lepsze od tego pierwszego?

  2. Właśnie miałem napisać inny sposób na to, ale Paweł mnie uprzedził.

    Rozwiązanie Pawła jest wydajniejsze – rzutowanie występuje tylko 1 raz.

  3. @Paweł
    Dla mnie taki zapis jest mniej czytelny, choć pewnie wydajnościowo będzie to taki sam jak drugie rozwiązanie autora.

  4. Rozwiazanie typu:
    FileInfo fileInfo = sender as FileInfo;
    if(fileInfo!=null){…}

    jest również poprawne. Nie pisalem o nim bo post byl o code review a konstrukcja is z as jest brzydka. Podany przez kogos przyklad uzycia as jest jak najbardziej OK.

    FileInfo fileInfo;
    if (sender is FileInfo)
    fileInfo = (FileInfo)sender;

    To rowniez podane w moim poscie jest poprawne. Jesli sender jest NULL to IS zwroci false i nie musimy sie martwic o nulla jak ktos to pisal w komentarzu.

    Jedynie brzydkie rozwiazanie to:
    FileInfo fileInfo;
    if (sender is FileInfo)
    fileInfo = sender as FileInfo;

    @Michal
    O wydajnosci w nastepnym poscie bo to zalezy od konkretnego przykladu i nie mozna powiedziec po prostu ze as jest szybszy od bezposredniego – bardzo czesto odwrotnie.

  5. @Pawel
    Nie jest “lepsze”. A na pewno nie zawsze. Zalezy od przykladu, ale o tym napize wiecej w nastepnym poscie bo widze ze troche zamieszania jest z as\is itp:)
    Pzdr

  6. To jest dobra praktyka i sprawdza się w codziennej pracy z kodem, review itp. Ale co do wydajności to możnaby dyskutować bez końca 😉

  7. Dokladnie.
    Generalnie “as” jest wydajniejszy ale nie zawsze i ma to bardzo male znaczenie. Ponadto, chodzilo mi glownie aby unikac konstrukcji (is,as) a nie czy as jest lepszy od direct casting 🙂 Polaczenie is z as po prostu jest brzydkie i nadmiarowe.

  8. @Piotr Zieliński:

    Masz 100% racji. Wszystko zależy od scenariusza użycia. W tym konkretnym przykładzie użyłbym as. I jestem pewien, że będzie to wydajniejsze.

    Natomiast nikt nie wspomniał jeszcze o takiej konstrukcji:

    Object obj = …;
    int i;
    try
    {
    i = (int) obj;

    }
    catch (InvalidCastException)
    {

    }

    W niektórych przypadkach istnieje uzasadnienie jej wykorzystania.

    Z minusów as można wymienić to, że nie można go użyć do obiektów, które nie mogą być nullem.

    Innych scenariuszy wymagają też sytuacje, gdy dochodzi dziedziczenie, własne konwertery pomiędzy typami, itp.

  9. @Michał

    Napisałeś, że w niektórych przypadkach istnieje uzasadnienie wykorzystania podanej konstrukcji. Czy mógłbyś przedstawić “przykładowy przykład”?

Leave a Reply

Your email address will not be published.