Zadanie key w list

Hej
Jakiś czas temu dostałem takie zadanie

funkcja przyjmuje 2 zmienne: key i list
list => ALL=CODE1;KEY1=CODE2;KEY2=CODE3;KEY3=CODE4
funkcja ma zwracać kod jeżeli wartość key jest w list, jeśli nie ma zwrócić wartość dla ALL

Zrobiłem je tylko nie wiem czy dobrze do niego podszedłem

Kod:

static int CodeFromList(int key, List<int> codeList)
        {
            if (codeList.Contains(key))
            {
                int index = codeList.IndexOf(key);
                return codeList[index];
            }

            return codeList[0];
        }

Nie mogę się zapytać, a wydaje mi się, że no nie wiem sam. Że to jest zbyt banalne a chodziło o coś trudniejszego. Moglibyście na to rzucić okiem i powiedzieć jak rozumiecie treść tego zadania?

Treść zadania jest dokładnie taka jak podałeś?
Jeżeli tak i dobrze zrozumiałem to wg mnie masz to dobrze - oczywiście bez zabezpieczeń przed pustą listą i brakiem elementu key w liście.

Ewentualnie krócej można to zapisać tak:

Kod:

static int CodeFromList(int key, List<int> codeList)
        {
            return codeList.Contains(key) ? codeList[codeList.IndexOf(key)] : codeList[0];
        }

Ok. Dzięki za potwierdzenie i skrócony zapis, obawiałem się, że chodzi o coś trudniejszego a nie taki banał. Faktycznie nie zabezpieczyłem się na wypadek pustej listy. W przypadku braku key w liście ma być zwrócony, tak przynajmniej to rozumiem, pierwszy element listy. Stąd return codeList[0] jeśli warunek if nie jest spełniony.

A co jeżeli lista jest pusta?

No pozostaje mi tylko powiedzieć: mea culpa^^ i wystawić zacną część mnie na lanie a potem przerobić kod . Może coś takiego?

Kod:

static int? CodeFromList(int key, List<int> codeList)
        {
            if (codeList.Count() == 0)
            {
                Console.WriteLine("The preceding list is empty");
                return null;
            }

            return codeList.Contains(key) ? codeList[codeList.IndexOf(key)] : codeList[0];
        }

Hej,
póki ta metoda jest prywatna i panujesz na tym co jej przekazujesz to nie ma co się gimnastykować ze sprawdzaniem zawartości listy. Ale jeżeli ta metoda ma być udostępniona jako publiczna to warto tez dodać sprawdzenie czy nie jest nullem

Jeszcze trochę bardziej namieszam, ale zadanie wydaje się być słabo sprecyzowane.

Kod:

static int? CodeFromList(int key, List<int> codeList)
        {
            if (!Equals(codeList, null) && codeList.Any())
            {
                return codeList.Contains(key) ? codeList[codeList.IndexOf(key)] : codeList[0];
            }
            Console.WriteLine("The preceding list is empty");
            return null;  
        }

To ja sie tez wypowiem. Na poczatek nalezy pamietac, ze akurat w naszym przypadku rozwiazanie im bardziej proste tym jest lepsze, jesli tylko spelnia wymagania biznesowe.
Pierwsze co sie rzuca w powyzszych rozwiazaniach to uzycie klasy Console w srodku metody. Jest to element widoku i z pewnoscia nie powinna ona znajdowac sie w tym miejscu.
W przypadku typow IEnuerable powinno sie uzywac metody .Any() zamiast przyrownania Count() == 0. Wyjasnienie tutaj: link
Poza tym tresc zadania wydaje sie nie do konca poprawna. W zadnym porzadnym systemie nie ma takich zalozen, ze zwraca sie pierwszy element listy, jesli zalozenie nei zostanie spelnione (znalezienie klucza).

Stravi napisał:Hej,
póki ta metoda jest prywatna i panujesz na tym co jej przekazujesz to nie ma co się gimnastykować ze sprawdzaniem zawartości listy. Ale jeżeli ta metoda ma być udostępniona jako publiczna to warto tez dodać sprawdzenie czy nie jest nullem

Calkowicie sie nie zgadzam. Pamietaj, ze malo kiedy piszesz cala aplikacje samodzielnie. Rownoczesnie z Toba pracuja inni programisci, ktorzy moga uzywac Twojej metody. Kod powinien byc przedewszystkim tak zabezpieczony, aby uniknac bledow w systemie, ktore pozniej ciezko wykryc. Jesli widzisz, ze mozna sie przed czyms zabezpieczyc - zabezpieczasz!

Fores napisał:

Stravi napisał:Hej,
póki ta metoda jest prywatna i panujesz na tym co jej przekazujesz to nie ma co się gimnastykować ze sprawdzaniem zawartości listy. Ale jeżeli ta metoda ma być udostępniona jako publiczna to warto tez dodać sprawdzenie czy nie jest nullem

Calkowicie sie nie zgadzam. Pamietaj, ze malo kiedy piszesz cala aplikacje samodzielnie. Rownoczesnie z Toba pracuja inni programisci, ktorzy moga uzywac Twojej metody. Kod powinien byc przedewszystkim tak zabezpieczony, aby uniknac bledow w systemie, ktore pozniej ciezko wykryc. Jesli widzisz, ze mozna sie przed czyms zabezpieczyc - zabezpieczasz!

Najpierw piszesz, że całkowicie się nie zgadzasz, a potem piszesz zupełnie coś innego!

Napisałem, że jeżeli to jest tylko jego prywatna metoda i panuje nad jego parametrami wejsciowymi to może sobie to podarować i tak się robi w wielu projektach. Jak nie wiesz co wywołujesz to tego nie używasz - proste jak Skoda.

Pozostała część wiadomości dalej utrzymuje to zdanie. W takim razie uargumentuje swoją opinie trochę bardziej szczegółowo:

• Nie mozesz zakladac, ze bedziesz jedynym uzytkownikiem swojej metody. Widzac jej slabe punkty powinienes przed nimi sie zabezpieczyc lub zwrocic odpowiedni wyjatek gdy nie zostana spelnione pewne wymagania.
  
• Idac tym tokiem...
  
• Weryfikujac w tym przypadku czy parametr nie jest nullem, mozemy zwrocic swoj wlasny wyjatek mowiacy o tym z ktorym obiektem jest problem. Bez takiej weryfikacji dostaniemy prosty NullReferenceException, ktory nam duzo mniej mowi.
  

  Kod:

   if (list == null)
  {
    throw new ArgumentNullException("list");
  }

  
  
• Juz na poczatku mozemy stwierdzic czy metoda sie wywola prawidlowo. Nie robiac takiej weryfikacji zostanie wywolany niepotrzebny kod, ktory moze niesc ze niepotrzebne problemy
  
• Jesli bedziesz przekazywal parametr dalej masz pewnosc, ze on jest prawidlowy
  
• Utrzymanie kodu jest duzo latwiejsze, kod jest bardziej przejrzysty
  
• Jak to z tego typu metodami juz bywa rzadko kiedy sa uzywane tylko jednorazowo. Osobiscie preferuje sprawdzic raz czy parametr nie jest nullem, niz w kazdej metodzie publicznej 'upewniac sie' czy jest ok. A zdaje sobie sprawe z tego, ze nie zawsze mozna byc tego pewnym. Pomaga to zarowno w zrozumieniu kodu, jak i przy pozniejszych testach jednostkowych. Piszac rozbudowane aplikacje jest tyle punktow, ktore moga nam namieszac, ze mysle, ze kilka dodatkowych linii kodu nam nie zaszkodzi dla wlasnej wygody (lub kilka znakow, jesli zrobimy extension method).
  

Hej, dzięki wam obu za pomoc. Zadanie faktycznie dotarło do mnie (nieoficjalną drogą) nie w pełni sprecyzowane, ale mogłem przynajmniej sprawdzić czy dobrze je rozumiem a potem błysnąć^^ jak pulsar typu S . Hmmm szkoda, że nie można nagradzać jakimiś plusami pomoc na forum.