Prośba o sprawdzenie kodu

Cześć!
Z nieznanego mi powodu mój programik ze zgadywaniem kolorów nie działa jak należy.
Niezależnie od tego co odpowiem nie widzi odpowiedzi w tablicy.
Pomożecie? :-)

var target
var answer
var x
var y
var finished = false
kolory = ["pomarańczowy", "żółty", "niebieski", "granatowy", "biały", "czerwony", "morelowy", "błękit", "czarny", "szary"]
kolory.sort()
function gra() {losuj(); pytaj();}
function losuj() {var x = Math.random() * kolory.length; 
	              target = Math.floor(x) + 1}

function pytaj() {while (!finished) {
	var answer = prompt("Jak myślisz...\n jaki kolor wymyślił komputer?\n Masz do wyboru:\n pomarańczowy, żółty, niebieski, granatowy, biały, czerwony, morelowy, błękit, czarny, szary");
	 var finished = sprawdzaj()}
}
function sprawdzaj () {if (kolory.indexOf(answer) > kolory.indexOf(target)) 
	               {alert("Twoja odpowiedz jest alfabetycznie za wysoko"); return false}
	               else if (kolory.indexOf(answer) < kolory.indexOf(target))
	               	{alert("Twoja odpowiedz jest alfabetycznie za nisko"); return false}
	               else if (kolory.indexOf(answer) == -1 )
	               	{alert("Tego nie ma na liście"); return false}
	               else {alert("Brawo trafiłeś!"); return true}
}

proste :-)

w metodzie pytaj() tworzysz nową (var), lokalną zmienną answer, więc ta globalna z początku pliku nie jest aktualizowana bo JS jej już nie widzi.
Usuń var wewnątrz metody i ten sam problem wyszedłby Ci niedługo zresztą co do zmiennej finished.

prompt("Jak myślisz...\n jaki kolor wymyślił komputer?\n Masz do wyboru:\n pomarańczowy, żółty, niebieski, granatowy, biały, czerwony, morelowy, błękit, czarny, szary");

Możesz zamienić na

prompt("Jak myślisz...:\n"+kolory.join('\n'))

Przedmówcy mają rację, unikaj zmiennych globalnych.

Po usunięciu wszystkich zmiennych globalnych (poza finished) działa jeszcze gorzej.
Zmieniałem globalne i lokalne w różnych konfiguracjach i nie znalazłem opcji żeby zaczęło działać poprawnie.

wrzuć to np. na https://plnkr.co

Pokaż kod

Pozbyłem się zmiennych globalnych ale dalej nie działa:

//var target
//var answer
//var x
//var y
//var finished = false
kolory = ["pomarańczowy", "żółty", "niebieski", "granatowy", "biały", "czerwony", "morelowy", "błękit", "czarny", "szary"]
kolory.sort()
function gra() {losuj(); var finished = false; pytaj();}
function losuj() {var x = Math.random() * kolory.length; 
                  target = Math.floor(x) + 1}
 
function pytaj() {while (!finished) {
	var answer = prompt("Jak myślisz...\n Jaki kolor wymyślił komputer?\n Masz do wyboru:\n" + kolory.join('\n'));
     var finished = sprawdzaj()}
}
function sprawdzaj () {if (kolory.indexOf(answer) > kolory.indexOf(target)) 
                   {alert("Twoja odpowiedz jest alfabetycznie za wysoko"); return false}
                   else if (kolory.indexOf(answer) < kolory.indexOf(target))
                    {alert("Twoja odpowiedz jest alfabetycznie za nisko"); return false}
                   else if (kolory.indexOf(answer) == -1 )
                    {alert("Tego nie ma na liście"); return false}
                   else {alert("Brawo trafiłeś!"); return true}
}

teraz np. answer jest widoczna tylo w pytaj, więc nie masz jej dostępnej w sprawdzaj(), możesz ją dodać do sprawdzaj(answer).
Przeanalizuj sobie, gdzie która zmienna jest potrzebna.
Korzystaj też z podpowiedzi dev toolsów.
Wrzuciłem to na https://plnkr.co/edit/fVv9xGuy0Xo5e11ak5HY?p=preview.

Poczytaj o zmiennych lokalnych i o przysłanianiu zmiennych globalnych, a także o funkcjach pierwszej klasy. Zrozum dobrze te tematy i rozpisz sobie logikę tego programu gdzieś na kartce, czyli co się po kolei wykonuje, a następnie popraw kod. Zwracaj też uwagę na błędy, które pokazuje Ci przeglądarka w konsoli - dzięki nim łatwiej Ci będzie dojść do ładu.

Ogólnie w JavaScript należy unikać zmiennych globalnych, czyli takich, które deklarujesz poza funkcjami.

Rada o usuwaniu zmiennych globalnych była raczej kulą w płot, bo OP właśnie chce zrobić zmienną globalną (czy raczej: widoczną w wielu funkcjach), więc idąc za tą radą pozbawił się możliwości komunikacji między funkcjami. Tę radę należy jednak uzupełnić o zaprezentowanie innego rozwiązania (co zamiast zmiennych globalnych), czyli np. przekazywanie argumentów do funkcji:

someFunction(someArgument) {
   // mamy dostep do zmiennej someArgument w funkcji
}
...
someFunction(23); //podanie 23 jako someArgument

Ale z drugiej strony w małym programie zmienne globalne nie są aż tak bardzo szkodliwe, wystarczy je otoczyć w jakieś IIFE (albo w moduł CommonJS / ES6) , żeby nie zaburzały globalnej przestrzeni nazw i myślę, że często będzie mieć to więcej sensu niż bawienie się w podawanie argumentów wszędzie. Szczególnie jak ktoś zaczyna to zmienne globalne są bardziej przewidywalne jednak od rozgryzania zasięgów zmiennych w JS.

Masz tu rozwiązanie bez globali (jest tylko globalna "stała" - tablica kolorów), mozna to zrobić dużo lepiej, ale nie chcę mieszać:

var KOLORY = [
  "biały",
  "błękit",
  "czarny",
  "czerwony",
  "granatowy",
  "morelowy",
  "niebieski",
  "pomarańczowy",
  "szary",
  "żółty",
]

function gra() {
  var answer
  var finished = false
  var targetIndex = losuj(KOLORY.length)
  
  while (!finished) {
    answer = pytaj()
    finished = sprawdzaj(answer, targetIndex)
  }
}

function losuj(max) {
  var x = Math.random() * max;
  
  return Math.floor(x) + 1
}

function pytaj() {
  return prompt(
    "Jak myślisz...\n jaki kolor wymyślił komputer?\n Masz do wyboru:\n" +
    KOLORY.join(', ')
  )  
}

function sprawdzaj (answer, targetIndex) {
  var answerIndex = KOLORY.indexOf(answer)
  
  if (answerIndex === targetIndex) {
    alert("Brawo trafiłeś!")
    return true
  }
  
  if (answerIndex === -1 ) {
    alert("Tego nie ma na liście")
  } else if (answerIndex > targetIndex) {
    alert("Twoja odpowiedz jest alfabetycznie za wysoko")
  } else {
    alert("Twoja odpowiedz jest alfabetycznie za nisko")
  }
  
  return false
}

CodePen: https://codepen.io/caderek/pen/vJGpxV?editors=0010

Przeanalizuj, w razie czego pytaj (btw miałeś skopaną kolejność ifów, dodatkowo kolory.indexOf(target) to bezsens, bo target u Ciebie to już indeks).

Przy okazji wykombinowaliście prompta, którego można zamknąć tylko przez zgadnięcie koloru - przycisk anuluj nie spełnia swojej roli :)

Nie wiem czy kiedykolwiek w realnej aplikacji użyłem prompta XD Kojarzy mi się to głównie ze spamerskimi stronami typu "Wygrałeś iPhona". W sumie bardzo rzadko się to przydaje (jedyny use case jaki mi przychodzi, to coś jak na Facebooku i Gmailu jest - że jeśli zaczniesz piszesz posta i będziesz chciał wyjść, to wyskoczy ci okienko o tym, czy naprawdę chcesz wyjść ze strony, jeśli masz niezapisane zmiany).

Jednak to są rzadkie przypadki i generalnie lepiej użyć zwykłych buttonów czy całych formularzy do tego typu rzeczy, niż gwałcić użytkownika (modalne okna to gwałt na użytkowniku, jakby nie było)

Tylko po co robić Offtop. Kolega uczy się JS i ma problem (jak każdy na tym etapie) ze zrozumieniem działania scope w JS.
Prompty są złe, globalne są złe ale nie wszystko na raz, lepiej uczyć się krok po kroku.

Maciej Cąderek napisał(a):

Masz tu rozwiązanie bez globali (jest tylko globalna "stała" - tablica kolorów), mozna to zrobić dużo lepiej, ale nie chcę mieszać:

Kopiuj

(...)
function losuj(max) {
  var x = Math.random() * max;
  
  return Math.floor(x) + 1
}
( ...)

Kopiuj

Tak sobie na to patrze i myślę że ta funkcja nigdy nie wylosuje wartości 0, czyli koloru białego.
Do tego czasem losuje wartość z poza tablicy.
Czy nie lepsze byłoby:

function losuj(max) {
var x = Math.random() * max;
return Math.floor(x)

?

@adams0
Możliwe, nie sprawdzałem działnia tej funkcji, przepisałem Twoją.