Napisałem program do rozpoznawania emocji w tekstach w języku angielskim, czy mógłbym poprosić o sprawdzenie kodu i o radę co można w nim jeszcze ulepszyć?
Na CodeClimate mam rating kodu w kategorii A, ale automat nie wszystko jest w stanie wykryć co wymaga poprawy.
Program:
https://github.com/opalizoid/artificial_emotions_gradient
Możesz wrzucić rubocopa do projektu i on ci powinien powiedzieć co jest nie tak. Rubocop pomoże Ci dużo bardziej jeśli chodzi o jakość kodu niż CodeClimate
Mam już wygenerowany plik z robocopa i biorę się za poprawki kodu.
Patrzac na pierwszą metode.
if ARGV[0] == nil rubiowiec zapisałby pewnie jako if ARGV[0].nil?, a idac dalej ten if sprawdza czy zostal podany jakis argument z cmdline, wiec zapisałbym go jako if ARGV.empty? żeby oddać intencje
if ARGV[0] == nil
text = File.read("text.txt")
else
begin
text = File.read(ARGV[0])
rescue
puts "\t No file\n\n"
end
end
powielone File.read. Żeby to zrefaktorować możnaby najpierw ustalić filename i potem to wykorzystać, czyli
if ARGV.empty?
filename = "text.txt"
else
filename = ARGV.first
end
begin
text = File.read(filename)
rescue
puts "\t No file\n\n"
end
co możnaby jeszcze uprościć
filename = ARGV.empty? ? "text.txt" : ARGV.first
i jeszcze
filename = ARGV.first || "text.txt"
To nieco zmienia oryginalny flow, bo puts "\t No file\n\n" może sie wyświetlić też przy błędzie w czytaniu pliku text.txt. I tu dochodzimy do sedna sprawy bo co tak naprawde ma znaczyć No file dla użytkownika twojego programu? Bo jeśli ma sygnalizować, że plik, który podał jako cmdline argument nie istnieje, to nie spełnia swojej funkcji, bo np rescue złapie też Errno::EACCES i pokażesz “No file” mimo, że plik istnieje, tylko nie masz prawa go czytać.
2a) Jako bonus, File.read("text.txt") wywali się jeśli odpalimy script.rb, bedac w katalogu (cwd) innym niz ten w którym są oba te pliki. Pomyśl jak to naprawić.
begin
text.gsub!(/[^a-zA-Z ]/, "").downcase!
words = text.split(" ")
rescue
words = [50, 50]
end
jaki bład ten rescue łapie? Czyżby NoMethodError na nil.gsub! w przypadku kiedy powyżej File.read zawiodło? To brzydka praktyka sterowaniem flowu programu za pomocą wyjątków, kiedy mogłeś to sprawdzić ifem.
words = [50, 50]
Co to za magiczne liczby? https://en.wikipedia.org/wiki/Magic_number_(programming)
Nazwij je (stałe?), bo sie przewijają niżej wielokrotnie te same liczby i nie wiem czy to to samo, czy tylko wartość ta sama?
if @db_array[i] > 50
tmp = (50 - @db_array[i]).abs
etc.
Tam samo dla innych magic numbers, np
elsif emo >= 60 and emo < 100
return words
w ruby pomijamy slowko return w takich sytuacjach.
Patrzac kawałek dalej:
def scale_values
random = rand(0..1)
i = 0
while i < @db_array.size
if i % 2 == 0 and random == 0
scale_partial(i)
else
scale_partial(i)
end
i += 1
end
end
takiego while raczej nie widuje sie w ruby, i pewnie byłoby to zapisane np jako
@db_array.size.times do |i|
end
do tego i % 2 jest metoda.
zwykle dobrze jeśli if i else robią co innego 
etc, etc
Wywaliłem to skalowanie ponieważ nie skalowało a robiło tylko bałagan.
Resztę powolutku poprawiam.
Przepisałem całość ponieważ zwyczajnie nie działała.
Aktualne repozytorium jest na GitLabie, piszę właśnie rspec.
https://github.com/opalizoid/ArtEmo_v3
Czy mógłbym poprosić o ponowne sprawdzenie kodu? Cały projekt przepisałem, wyrzuciłem rozwiązania na które niesłusznie się upierałem, przeanalizowałem wszystkie logi RuboCopa i nie popełniłem starych błędów, ale przestałem zwracać uwagę na długość metod czy linijek kodu ponieważ większy bałagan powstawał przy niesłusznym skracaniu metod niż przy zostawianiu jak jest ponieważ czasami dłuższe metody są czytelniejsze.
Dodałem testy rspec, uruchamianie testów przez rake i dokumentację yard. Zmieniłem też sposób uruchamiania klas z .new na .call dzięki self.
Jeżeli wszystko jest OK, to będę mógł opublikować jako gema, ponieważ tym razem po testach na różnych tekstach program zwraca wyniki które zgadzają się z analizą tekstów przeze mnie.
W plikach rspec mogą być błędy i w dokumentacji ponieważ po raz pierwszy pisałem testy automatyczne i dokumentowałem porządnie kod.
Jest to mój pierwszy poważniejszy program w Ruby, wcześniej zajmowałem się pojedynczymi algorytmami typu Apriori i wiele rzeczy które stosowałem musiałem jeszcze raz przeanalizować, drugi raz nie popełniałem tych samych błędów.
Dziękuję za uwagi po publikacji tamtej wersji, ponieważ dzięki nim uzyskałem działającą wersję i dużo się nauczyłem.
plik .yardoc powinien znaleźć się w gitignore i powinien być usunięty z repo. Plik .DS_Store powinnien być umieszczony w globalnym gitignore, a nie gitignore aplikacji oraz również wszystkie wystąpienia powinny być usunięte z repo. https://robots.thoughtbot.com/global-gitignore
podzieliłeś metody na klasy, ale wszędzie używasz metod klasowych a nie metod instancji. Wnioskuję z tego, że w Ruby jesteś początkujący, w związku z tym poczytaj jak się pisze programy obiektowo. W końcu Ruby jest językiem obiektowym.
Przykład:
Metody klasowe nie uzyskają dostępu do
attr_reader :cos
odnoszę wrażenie, że wpisałeś to, bo robocop ci tak podpowiedział. Jeżeli robocop wskaże Tobie, że trzeba coś poprawić, a jest to dla Ciebie coś nowego, to proponowałbym sprawdzić najpierw w internecie jak to działa.