Nie jestem pewny czy dobrze rozumiem funkcje

Question

Witam.

Uczę się JavaScript od wczoraj, a dzisiaj napotkałem pewien problem.

Chcę stworzyć skrypty, które po naciśnięciu na przycisk zmienią tło na zielone lub czerwone (zależnie od klawisza, są dwa), dodatkowo będą podliczać ich ilość i wyświetlać obok (Na zasadzie plusów i minusów przy jakimś poście). Kolejnym warunkiem jest to, że jeśli klawisz A jest wciśnięty, to po naciśnięciu klawisza B chciałbym aby efekty klawisza A zostały cofnięte, a klawisza B zapisane.

Za pomocą różnych źródeł udało mi się stworzyć funkcje które nie spełniają jednak warunku z "odkliknięciem" drugiego przycisku, oraz nie zapisują ilości kliknięć. Oto pierwotny kod:

var voteUp = document.getElementsByClassName("like")[0];
voteUp.onclick = function(){
    if (voteDown.style.backgroundColor ==="red"){
    return;
    }
    if (voteUp.style.backgroundColor===""){
        voteUp.style.backgroundColor = "green";
    }
    else
        voteUp.style.backgroundColor = "";
};
var voteDown = document.getElementsByClassName("dislike")[0];
voteDown.onclick = function(){
    if (voteUp.style.backgroundColor ==="green"){
        function(){
        };
    }
    if (voteDown.style.backgroundColor===""){
        voteDown.style.backgroundColor = "red";
    }
    else
        voteDown.style.backgroundColor = "";
};

A oto ten "ulepszony" który już po prostu nie działa, stąd pytanie, czy sposób w jaki ja chciałem to zrobić jest poprawny? Nie mogę też znaleźć błędu, który mógłby powodować, że to nie działa.

    var counter = document.getElementByClassName("licznik")[0].innerHTML = 0;

function Votes(){    
    voteUp.onclick = function(){
        if (document.getElementByClassName("dislike")[0].style.backgroundColor==="red")
            document.getElementByClassName("dislike")[0].style.backgroundColor==="",
            document.getElementByClassName("like")[0].style.backgroundColor="green",
            document.getElementByClassName("licznik")[0]+2;
            
        else
            document.getElementByClassName("like")[0].style.backgroundcolor="green",
            document.getElementByClassName("licznik")[0]++;
    };
    voteDown.onclick = function(){
        if (document.getElementByClassName("like")[0].style.backgroundColor==="green")
            document.getElementByClassName("like")[0].style.backgroundColor==="",
            document.getElementByClassName("dislike")[0].style.backgroundColor==="red",
            document.getElementByClassName("licznik")[0]-2;
            
        else
            document.getElementByClassName("dislike")[0].style.backgroundColor="red",
            document.getElementByClassName("licznik")[0]--;
    };
};

Dodaje używaną tutaj część HTML

<div class="foot">
                <div class="vote">
                    <div class="like"></div>
                    <div class="dislike" ></div>
                    <div class="licznik"></div>
                    <script src="javascript.js"></script>
                </div>
                <div class="comment">
                    <a href="adres"></a>
                </div>
            </div>

Pozdrawiam

Comments

Pokaz jeszcze kod HTML, żeby można było przetestować całość.

Linie 15-16 z pierwszego bloczka powinny rzucać SyntaxError, że nie możesz tworzyć function statement bez nadania jej nazwy. Poza tym, co w tym miejscu miało by się zadziać (gdy voteUp ma zielony kolor tła)?

W drugim bloczku linie 8, 12, 18 i 22 są bez sensu - próbujesz wykonać operacje arytmetyczne: dodawanie i inkrementacja oraz odejmowanie i dekrementacja na elemencie DOM? Jeśli już, to powinieneś najpierw (w wyższym scope) przypisać element o klasie licznik do zmiennej, na której będziesz ustawiać wartość liczbową w zależności od jakiegoś warunku i dopiero tą zmienną przypisywać (jako property textContent lub innerHTML) do elementu DOM.

Poza tym, dlaczego w ifach używasz przecinków, zamiast objąć to klamrami? I, w ogóle, przypisz sobie na górze wszystkie elementy DOM do zmiennych, a później operuj na referencjach, żebyś nie musiał przy każdej operacji szukać tych elementów w DOM (document.getElementsBy*).

---

Dziękuję za odpowiedź

W moim zamyśle linie 15-16 miały sprawdzać, czy tło jest zielone, jeśli tak to miały je usunąć, a drugiemu przyciskowi nadać czerwone i odjąć od licznika 2.

Jeśli chodzi o operacje arytmetyczne to faktycznie zrobiłem błąd, jak pisałem za pierwszym razem to wydawało się to logiczne, nie wspominając, że operacje były źle wykonane.

Poprawiłem i przypisałem elementy do zmiennych.

Na początku używałem średników, ale wyrzuciło mi błąd przy "else". I co masz na myśli, mówiąc, żebym objął  je klamrami?

Answer 1

Znaki porównania  jak ("===") używa się w warunkach, więc musisz poprawić kod dając "===" tylko w if (w innych przypadkach niż porównanie to piszesz znak przypisania "="). Poza tym jak chcesz zmieniać wartości to musisz przypisywać je do czegoś. O ile x++ i x-- działa bezproblemowo to x-2 i x + 2 nie nadpiszą Ci wartości. Musisz pozamieniać takie przypadki żeby było np x -= 2 (to samo co x = x - 2) i analogicznie x += 2 (to samo co x = x + 2).

Comments

Dziękuję bardzo za odpowiedź, faktycznie pomyliłem się ze znakiem  "===", oraz zapomniałem, że coś takiego jak "+=" istnieje. Kod poprawiłem, jednak skrypty wciąż stawiają opór co do działania i nie chcą zmienić tła diva.

---

Nie masz tu jeszcze zadeklarowanych voteUp i voteDown w kodzie, a wyżej było. Poza tym, polecam uruchamiać okienko konsoli w przeglądarce ctrl+shift+i i tam w zakładce konsola powinieneś mieć komunikaty co krzaczy błędami ;)

---

Czy z deklaracją masz na myśli:

var voteUp = document.getElementByClassName("like")[0];

I po włączeniu konsoli jak poleciłeś i zobaczyłem to:

TypeError: document.getElementByClassName is not a function

---

Tak to mam na myśli, a ten błąd zobacz gdzie występuje. Z prawej strony w konsoli razem z błędem masz też linię, w której występuje błąd, możesz do niej przejść i zobaczyć w którym miejscu się zatrzymuje skrypt.

---

Pokazuje błąd w liniach 25:27, czyli tu, gdzie zadeklarowałem zmienne

    var voteUp = document.getElementByClassName("like")[0];
    var voteDown = document.getElementByClassName("dislike")[0];
    var counter = document.getElementByClassName("licznik")[0].innerHTML = 0;

 

---

Zjadłeś "s", powinno być getElementsByClassName

---

Prędzej czy później padłbym ofiarą literówki. :p Poprawione i obok divow pojawiło się "0", które jest licznikiem, przed chwilą go nie było. Reszta dalej nie działa, a konsola nic już nie wyświetla.

---

Literówki to chleb powszedni ;D

Spróbuj kod zmienić do postaci z:

if(warunek)
  operacja1, operacja2
else
  operacja3, operacja4

Na kod:

if(warunek) {
  operacja1;
  operacja2;
} else {
  operacja3;
  operacja4;
}

 

---

Zrobiłem tak, jednak sytuacja jak wcześniej, nie działa i konsola milczy.

---

Wrzuć kod, to zerknę ;)

---

    var voteUp = document.getElementsByClassName("like")[0];
    var voteDown = document.getElementsByClassName("dislike")[0];
    var counter = document.getElementsByClassName("licznik")[0].innerHTML = 0;

function Votes(){    
    voteUp.onclick = function(){
        if (voteDown.style.backgroundColor==="red"){
            voteDown.style.backgroundColor="";
            voteUp.style.backgroundColor="green";
            counter += 2;
        }
        else {
            voteUp.style.backgroundcolor="green";
            counter++;
        }
    };
    voteDown.onclick = function(){
        if (voteUp.style.backgroundColor==="green"){
            voteUp.style.backgroundColor="";
            voteDown.style.backgroundColor="red";
            counter += 2;
        }
        else{
            voteDown.style.backgroundColor="red";
            counter--;
        }
    };
};

Proszę ;)

---

Masz stworzoną funkcję Votes ale nie wywołałeś jej nigdzie, żeby mogła podpiąć eventy pod html, musisz ją wywołać, dodaj Votes(); na końcu kodu i zobacz co teraz będzie ;)

---

Rzeczywiście, zapomniałem.

No i jest progress! voteUp reaguje na kliknięcie tylko jeśli voteDown jest aktywny. Czyli za pierwszym razem voteUp nie chce załapać, ale jeśli voteDown jest aktywny to mogę swobodnie przełączać tak jak miało to wyglądać ;)

A, no i licznik stoi w miejscu :(

---

No to gitarka, będzie z górki ;)

---

Licznika nie wyświetlasz taka podpowiedź.

---

Na to liczę :p

Deklaracja z .innerHTML nie będzie wyświetlać jeśli ta wartość się zmieni, czy w ogóle nie będzie przyjmować żadnych zmian?

---

Jak chcesz sobie podejrzeć co zawiera zmienna to umieść ją w console.log(nazwaZmiennej), w miejscu gdzie chcesz podejrzeć jej zmianę. Np. Twój counter zawiera liczbę a nie element DOM, bo do countera przypisałeś wynik innerHtml = 0, czyli zmiana jego nie wpłynie na zawartość DOM. Musisz do countera przypisać sam element i zmieniać wartość przez np counter.innerHtml +=2.

---

Wpisałem console.log(counter); do skryptu i nic się nie wyświetla. Dodawałem " ", też nic, a po wpisaniu bezpośrednio do konsoli wyrzuca "undefinied". Rzucałem też komendą w różne miejsca kodu dla pewności i dalej pusto

---

Pokaż kod ;)

---

    var voteUp = document.getElementsByClassName("like")[0];
    var voteDown = document.getElementsByClassName("dislike")[0];
    var counter = document.getElementsByClassName("licznik")[0].innerHTML = 0;
    console.log("counter");
function Votes(){    
    voteUp.onclick = function(){
        if (voteDown.style.backgroundColor==="red"){
            voteDown.style.backgroundColor="";
            voteUp.style.backgroundColor="green";
            counter.innerHTML += 2;
        }
        else {
            voteUp.style.backgroundcolor="green";
            counter.innerHTML++;
        }
    };
    voteDown.onclick = function(){
        if (voteUp.style.backgroundColor==="green"){
            voteUp.style.backgroundColor="";
            voteDown.style.backgroundColor="red";
            counter.innerHTML -= 2;
        }
        else{
            voteDown.style.backgroundColor="red";
            counter.innerHTML--;
        }
    };
};
Votes();

 

---

Daj console.log(counter) albo console.log("counter = ", counter) dla przejrzystości to tak na przyszłość ;) "counter" to napis dlatego zmiennej Ci nie wypisze tylko ten napis. Poumieszczaj też to sobie w zdarzeniach kliknięcia to będziesz widział w trakcie jakie zmiany się dzieją.

---

Tak myślałem na początku, dlatego pierwszą opcją było console.log(counter). Później dodałem " " ze względu na brak efektu ;) bo konsola z jakiegoś powodu nic nie pokazuje w obu przypadkach.

---

Akurat dla przypadku " " nic nie pokaże konsola, bo nie ma widocznych znaków ;)

---

Tak tak, wiem, ale bez względu na to czy coś w środku jest czy nie to nic nie pokazuje, albo pokazuje "undefinied" więc całkiem możliwe że niechcący coś popsułem :p

---

Undefined będzie pokazywać kiedy zmienna nie ma zdefniowanej wartości.

---

Owszem. No i było tak jak pisałem, niechcący coś popsułem, a mianowicie miałem odznaczone wyświetlanie wpisów i zdziwiony byłem, że nic mi się nie wyświetla. No ale już opanowałem sytuacje, a rekord faktycznie wyświetla ;)

---

Gitara ;)

---

Dodatkowo udało mi się rozwiązać problem dotyczący braku możliwości zaznaczenia voteUp kiedy voteDown jest nieaktywny.

Polegał on na tym, że instrukcja if sprawdzała, czy przycisk ma jakieś tło, jednak nie brała pod uwagę tego, że może tego tła nie mieć :D

Końcowy kod wygląda tak:

function Votes(){    
        voteDown.onclick = function(){
        if (voteUp.style.backgroundColor === "green" || ""){
            voteUp.style.backgroundColor = "";
            voteDown.style.backgroundColor = "red";
            counter -= 2;
        }
        else if (voteDown.style.backgroundColor === "red") {
            voteDown.style.backgroundColor = "";
            counter++;
        }
        else {
            voteDown.style.backgroundColor = "red";
            counter--;
        }
    };
        voteUp.onclick = function(){
        if (voteDown.style.backgroundColor === "red" || ""){
            voteDown.style.backgroundColor = "";
            voteUp.style.backgroundColor = "green";
            counter += 2;
        }
        else if (voteUp.style.backgroundColor === "green") {
            voteUp.style.backgroundColor = "";
            counter--;
        }
        else {
            voteUp.style.backgroundColor = "green";
            counter++;
        }
    };
};

Będę jeszcze prowadzić batalie z tym licznikiem, ale prędzej czy później to rozwiążę :D

Dziękuje bardzo za rady i poświęcony czas ^^

---

NIe ma sprawy ;) 

Te warunki z || " " Ci nie zadziałają jak trzeba, bo priorytet operatorów jest taki, że najpierw wykonują się ===, !==, >, <, operacje w nawiasach itp a potem operatory boolowskie jak ||, &&. To co masz w zamyśle powinno być zrobione w ten sposób:

teraz

if (voteUp.style.backgroundColor === "green" || "")

poprawnie

if (voteUp.style.backgroundColor === "green" || voteUp.style.backgroundColor === "")

warunki przed i po || wykonają się w pierwszej kolejności

---

To wszystko co powinienem poprawić?

Po zrobieniu tak jak napisałeś nie mogę odznaczyć przycisku

---

Tak te warunki to powinno być wszystko jeżeli chodzi o zmianę kolorów. Mam nadzieję, że ten kod co wysłałeś to nie jest cały kod tylko fragment, bo nie masz zadeklarowanych rzeczy tutaj ;)

---

To w takim razie nie wiem dlaczego ten przycisk nie chce się odznaczyć. Pójdę coś zjeść to może coś wymyślę :D

Jak najbardziej, moja wina, mogłem wspomnieć, wstawiłem samą funkcję, bez zmiennych albo jej wywołania, ale w kodzie wszystko jest :p

---

Jak teraz wkleiłem kod to zobaczyłem, że bez tych || ... w ifach to działa chyba.

---

Działa wszystko oprócz tego, że nie da się kliknąć voteUp jeśli voteDown nie jest aktywny

---

Kombinuj a jak coś to pisz ;D

---

Cały czas mnie gryzie dlaczego mimo że if (voteUp. style.backgroundColor === "green" || ""); Jest niepoprawnie, to działa jak trzeba...

---

Znaczy poprawne z punktu widzenia programu jest, tylko nic nie wnosi, bo w warunku

color === "red" || ""

warunek się wykona tylko gdy color === "red", 

opcja || "" zwraca fałsz, równie dobrze mógłbyś tam dać: || null, || 0, || false, i to da ten sam efekt.

Jak coś zawsze daje tą samą wartość prawda/fałsz, nie ma sensu tego wstawiać.

---

Stwierdziłem, że usunę to i zobaczę co i jak. No i działa. To teraz się zastanawiam jak ja to zrobiłem, że kiedy tego nie było wcześniej to nie działało :D

---

Na pewno działało z innego powodu, tego możesz być pewien ;)

---

Zapewne kiedy to dodawałem to zobaczyłem jakiś błąd który od razu poprawiłem i zapomniałem :p

Ale ważne, że działa jak należy ;)