Hej
###Background
Niestety napisany przeze mnie kod niespodobał się “seniorom” w firmie do której startowałem o robotę.
Powiedzieli że nie wygląda to na “senior” level, więc nie są w stanie zaproponować mi współpracy.
Poprosiłem o więcej info, ale mój mail został zignorowany, więc zdecydowałem się tutaj założyć temat, aby pomóc sobie (i innym) w szlifowaniu skilla w Ruby.
###Repo
[usuniete]
problem #1
[usuniete]
rozwiązanie
[usuniete]
problem #2
[usuniete]
rozwiązanie
[usuniete]
Zadania są na tyle proste, że skoro były one wymagane do rekrutacji na poziom “senior”, to znaczy że nie chodziło tylko o umiejętność rozwiązania problemu, a sposób.
Twój kod jest nieobiektowy i brzydki (szczególnie drugi, trudniejszy problem).
Dzieki za odp.
Ciekawe, mi się bardziej niepodoba #1.
Więcej info? Które linie są do poprawy, co można poprawić, JAK, co konkretnie itd?
Generalnie to przypadek pierwszy jest przerażający odrobinę.
Przede wszystkim skorzystałbym z dzielenia modulo i łączyłbym na koniec zanim łączyć łańcuchy.
def convert(number)
case number
when 0...20 then NAMES[number]
when 20...100 then [TENS[number/10], convert(number % 10)].join(" ")
when 100...1000 then [NAMES[number / 100], "hundred", convert(number % 100].join(" ")
when 1000...10**6 then [NAMES[number / 1000], "thousand", convert(number % 1000].join(* ")
else
[NAMES[number / 10**6], "milion", convert(number % 10**6].join(* ")
end
Oczywiscie do tego dochodzą endy itd. itp. ale w ten sposób cała funkcjonalność sprowadza się +- do jednej metody.
Tworzenie klasy i 30 metod do rozwiązania problemu nie koniecznie oznacza że jest to "obiekotowo"
Do czego ja bym się przyczepił:
* BIG_NUMERALS nie ma sensu, tym bardziej robienie BIG_NUMBERALS[2] zamiast "milion", wyobraź sobie że ta metoda jest w jakimś większym pliku po czym siadasz i widzisz BIG_NUMERALS[2] i myślisz, co to **** jest ? Jak już chcesz być porządny i nie uzywać łańcuchów które coś znaczą (np. żeby uniknąć literówek), to definiujesz stałą MILLION = "million"
* initialize używający hasha i fetch dla dwóch parametrów, czemu po prsotu nie zrobić `def initialize(from, to)`? Generalnie konwencja nakazuje 1..3 parametrów powinno być w metodzie przekazywane bezpośrednio
* `if` i `elsif` zamiast `case` ale to już raczej kwestia stylu
* Duplikacja pomiędzy `spellout_tens` `spellout_hundreds` i innymi, jak widać schemat jest praktycznie ten sam, więc można by było to jakoś zdeduplikować myślę
* Cała seria `output_*` jest bezyzyteczna i zajmuje tylko miejsce
* Ponieważ `and` jest uzywany tylko jeżeli numer jest w przedziale (1..19) a zero nie jest praktycznie rzecz biorąc nigdy wypisywane, można by spokojnie przenieść funkcjonalność sprawdzania i dodawania _and_ do `spellout_tens` i uzywać z innych metod a w case/if głównym po prostu włoać TENS[x]
* Używanie stałych. Obecnie trend jest taki żeby uzywać metod klasowych, z tego względu że łatwiej zrobić refactoring metody niż stałej. Świetnym przykłądem tutaj jest VAT, jeżeli zdefiniujesz go jako stałą, po czym VAT sięzmieni jesteś w 4 literach, bo teraz każdą referencjedo stąłej musisz zmieniać na wywołanie metody która sprawdza który jest rok i podaje VAT odpowiedni do okresu. Jeżeli zdefiniujesz jako metodę klasową wystarczy dodać parametr (rok)
Co do drugiego rozwiązania:
* Zahardkodowany host i port
* `correct_parameter` = VALID_PASSWORDS.include?(str)
* `correct_parameter?` właściwie jest do kitu nazwaną metodą, jak już to `correct_api_key?`
* Nie przekazujesz dalej oryginalnego statusu (co jeżeli odpowiedź z API jest 500 albo 423)?
* Zastanawiam się czy usuwanie api_key z parametrów w ogóle działa.
Możliwe że padłęś na 3 pytaniu ale na niego nie patrzyłem.Ja spojrzałem na initialize w #1 i już mi się nie chciało dalej czytać:
Generalnie zatrudnianie na podstawie “kod mi się [nie] podoba” to nie jest najlepszy pomysł w ogólnym przypadku, ale to już dyskusja na zupełnie inny temat.
Poza tym, mogło się chłopakom z Global Personal też nie spodobać, że umieszczasz na githubie ich zadania rekrutacyjne. Pomijając już nawet kwestie legalności tego procederu (prawa autorskie).
@sharnik co do githuba to spotkałem się z tym że standardowo można sobie forknąć tego typu repozytoria, co do dzielenia się rzeczywiscie moze to być niemile widziane.
Każde umieszczenie w miejscu indeksowanym przez przeglądarki to dzielenie się.
Ja standardowo się spotkałem z prośbą, żeby nigdzie nie umieszczać. Szczególnie z podaniem nazwy firmy.