Refaktoryzacja kodu JavaScript

Question

Hej!

Piszę pewną aplikację i aktualnie, po szybkim pisaniu kodu przychodzi moment przeglądania co się napisało.
Mam już trochę mętlik w głowie po tygodniu kodowania po 14 godzin :) dlatego mam problem ze zmniejszeniem ilości kodu w prostym switch case :P proszę Was o pomoc, w jaki sposób mogę to przerobić, aby kod był... bardziej... fancy, beautifull :) 

Otóż, mam sobie plik dane.json, struktura taka:

{
"customSlideParams": [{
        "texture": "NameSlides_1", 
        "slideNumber": 1, 
        "isUserInput": true, 
        "questionType": "text",
        "question": "Jak masz na imię?"
    },
    {
        "texture": "NameSlides_2", 
        "slideNumber": 2, 
        "isUserInput": true, 
        "questionType": "radio",
        "question": "Zaliczyliśmy małe spóźnienie, co?",
        "answers": [
            "R,X", "Odpowiedz 1",
            "B", "Odpowiedz 2",
            "A", "Odpowiedz 3",
            "S", "Odpowiedz 4",
            "P", "Odpowiedz 5",
            "K", "Odpowiedz 6"
        ]

    },
    {
        "texture": "NameSlides_3", 
        "slideNumber": 3, 
        "isUserInput": true, 
        "questionType": "radio",
        "question": "Hej, nie bądź taki szybki!",
        "answers": [
            "K", "Odpowiedz 1",
            "S", "Odpowiedz 2",
            "A", "Odpowiedz 3",
            "B,X", "Odpowiedz 4",
            "P", "Odpowiedz 5",
            "R", "Odpowiedz 6"
        ]
    }]
}

W kodzie mam 7 zmiennych (this,R, this.B itd...) reprezentujących licznik danych odpowiedzi na. np. jak użytkownik odpowiedział na odpowiedź 4 to dodaję plus jeden this.A++.
Kolejność kategorii jest powiedzmy losowa! Nie mam wpływu na to, które pytanie gdzie będzie, więc kod musi być elastyczny.
Kolekcjonuje dane w obiekcie this.dataCollections = {R: this.R, B: this.B... itd}.

Zwiększanie licznika realizuję poprzez ten blok kodu:

var ansArr = slide.customParams.answers;
for (var i = 0; i < ansArr.length; i++) {

            var actualQuestion = 'question-' + ((i+1)/2);
            var actialQuestionString = actualQuestion.toString();
            var isActualChecked = document.getElementById(actialQuestionString);

            if (i%2 !== 0 && isActualChecked.checked) {
                switch (ansArr[i-1].charAt(0)) {
                    case 'R': 
                        self.R++;
                        break;
                    case 'B': 
                        self.B++;
                        break;
                    case 'A': 
                        self.A++;
                        break;
                    case 'S': 
                        self.S++;
                        break;
                    case 'P': 
                        self.P++;
                        break;
                    case 'K': 
                        self.K++;
                        break;
                }
                switch (ansArr[i-1].charAt(2)) {
                    case 'X':
                        self.X++;
                        break;
                }
            }
        }

Dodam, że odpowiedzi są typu radio, więc jednokrotnego wyboru. Ale mam też rodzaj odpowiedzi wielokrotnego wyboru, więc switch case tego na pewno nie zrobię (chyba), tylko na zasadzie

if (warunek){};
if(warunek){};
...

Co wydaje się już mega niefajne, bo dużo kodu :( 

Ogólnie mam pytanie, czy jest (pewnie tak) sposób na efektywniejsze, lub po prostu ładniejsze zapisanie tego?

Co sądzicie, jak oceniacie ten sposób i czy macie jakieś sugestie, podpowiedzi! Z góry dzięki za rzetelne wypowiedzi! :)

Answer 1

Na twoim miejscu zapisałbym te dane w tablicy. W tym momencie wypada ci cały switch.

Tworzysz tablicę z elementami R, B, A... i nadajesz im 0. Potem jeśli masz odpowiedź, to nie przełączasz się switchem pomiędzy parametrami, tylko używasz od razu wartości zmiennej jako indexu tablicy:

self.counters[ ansArr[i-1].charAt(0) ]++;

 

Comments

masz na myśli tablica zmiennych, w momencie gdy radio jest .checked to dodaje jeden do dokładnie tego elementu tablicy

problem jest w tym, że te A, B, R... to są tak jakby kategorie odpowiedzi, i w momencie jak ktoś stwierdzi, że kolejność chce mieć inną w tablicy z pytaniami w pliku dane.json to już zrobi się chyba problem z Twoim rozwiązaniem, dobrze myślę?

Np. w pytaniu do slajdu 2 będzie inna kolejność parametrów R,A,S itp, a w pytaniu do slajdu 3 inna kolejność np. K, P, R itp

Bo jeżeli bym założył, że kolejność tych kategorii ma być identyczna zawsze, to bym w ogóle switch case nie tworzył, ale fakt, sorki bo nie sprecyzowałem pytania

---

Nie bardzo rozumiem, co masz na myśli. Do tablicy przypisujesz te odpowiedzi do konkretnych indeksów, oznaczonych literami, więc kolejność nie ma znaczenia.

---

Ok, muszę przejrzeć wszystko na spokojnie :) dzięki!

Answer 2

1. Użyj jednego vara i przecinków.
2. ansArr.length przypisz do MAX tam gdzie deklarujesz iterator po przecinku.
3. Polecam wywalić tego switcha. Możesz go zastąpić odwołaniem do pola obiektu (obiekt[zmiennaZNazwaPola]). Pamiętaj aby użyć funkcji hasOwnProperty.  ​​​​
4. Redukcja switcha z jedną opcją to oczywiste. 
5. Nie łap elementów DOM w pętli. Jest to wysoce nieoptymalne.
6. toString niepotrzebne. 

Comments

Ad. 1 Czemu? To raczej zła praktyka: https://github.com/airbnb/javascript#variables--one-const

---

Dlaczego zła?

---

No przecież podlinkowałem Ci uzasadnienie.

---

Why? It’s easier to add new variable declarations this way, and you never have to worry about swapping out a ;for a , or introducing punctuation-only diffs. You can also step through each declaration with the debugger, instead of jumping through all of them at once.

Chodzi Ci o samo słowo var? Czy o to, że używać go pojedynczo?  

---

No o osobne deklaracje per zmienna (var to swoją drogą).

---

Moim zdaniem jest to bardziej przejrzyste. Część minifikatorów i tak zamienia deklaracje na single var.

---

Minifikatory zmieniają też nazwy zmiennych na pojedyncze litery, co to w ogóle za argument? :D Lepsze diffy, mniejsza poddatność na błąd, łatwiejsze debugowanie - to nie są rzeczy bez znaczenia.

Answer 3

'question-' + ((i+1)/2);

Możesz jawnie wywołować obiekt String i przez to nie używać metody String. Ale to bardziej zostawiam jako ciekawostkę.

alert('Hey mam' + new String((i+1)*2));

Nie musisz ifa, używać pokaże ci prosty trik.

https://codepen.io/shiroUmizake/pen/gGROJm

Robię sobie tablicę i z obiektami typOdpowiedzi i licznik. Następnie sprawdzam po moim kluczu czy któryś pasuje do klucza. Jeśli tak, to go zwracam i go zwiększam. U mnie żadnego switcha nie ma.

Jeżeli to była wielokrotnego wyboru pytanie, wystarczy bym ustawił np data-typeAnswer = multiplechoice a dla z jedną odpowiedzią 'single-choice', zależności od typu pytania użyłbym by tej bądż innej metody. 

Ja to zrobiłem na buttonach, ale na inputach działa podobnie. 

 

 

Comments

Dzięki, taki przykład mi dużo ułatwia i rozjaśnia, jak tylko przysiądę do tego to sprawdzę zastosowanie u mnie! :)