Prosty program pilot (remote)

Prosty program pilot (remote)
BornStubborn
  • Rejestracja:ponad 5 lat
  • Ostatnio:ponad 2 lata
  • Postów:60
0

Hej,
próbuję napisać prosty program działający na zasadzie pilota do telewizora. Robię jeszcze sporo błędów i estetyczność kodu jest jaka jest. Chciałbym poprosić o pomoc w jego optymalizacji i naprawieniu jego obecnych błędów.
Program miałby działać w następujący sposób : Po wydaniu polecenia (wpisaniu) telewizor powinien się włączyć. Pilot powinien mieć możliwość przełączania między dziewięcioma zapętlonymi kanałami za pomocą przycisków typu "channel +", "channel -". Powinien mieć też możliwość zmiany głośności w przedziale 1-15,za pomocą podobnych przycisków jak w przypadku zmiany kanału (ale bez zapętlania). Wyłączenie telewizora powinno kończyć pracę programu.
Jak dotychczas mój kod wygląda tak:

klasa TV

Kopiuj
    public static void main(String[] args) {
        Remote remote = new Remote();
        remote.onOff();
    }
}

klasa Remote

Kopiuj
import java.util.Scanner;
public class Remote{
    boolean onOff = false;
    int channel = 0;
    int volume = 1;
    public Scanner scanner =new Scanner(System.in);

    public void onOff(){
        System.out.println("TV is off");
        String turning =scanner.next();
        if(turning.equals("off"))
            onOff = false;
            System.out.println("TV is off");

        if (turning.equals("on")) {
            onOff = true;
            System.out.println("TV is on");
        }
        while (onOff){
            System.out.println("TV działa. Co chcesz teraz zrobić?\n" +
                    "Masz do dyspozycji 9 kanałów i głośność w przedziale 1-15.\n" +
                    "Kanały możesz zmieniać za pomocą polecenia 'chup' oraz 'chdn', \n" +
                    "głośność natomiast za pomocą 'vup' i 'vdn'.\n" +
                    "Możesz wyłączyć TV przy pomocy 'off'");

            String choose = scanner.next();
            while (choose!=("off")){
                switch (choose) {

                    case "chup":
                        if (choose.equals("chup"))
                            for (int i = 0; i < 9; i++) {
                                channel = channel++;
                            }

                    case "chdn":
                        if (choose.equals("chdn"))
                            for (int i = 0; i < 9; i--) {
                                channel = channel--;
                            }

                    case "vup":
                        if (choose.equals("vup"))
                            for (int i = 1; i < 15; i++) {
                                volume = volume++;
                            }

                    case "vdn":
                        if (choose.equals("vdn"))
                            for (int i = 1; i < 15; i++) {
                                volume = volume--;
                            }
                }
                System.out.println("Obecny kanał to "+ channel + "\n Obecna głośność to "+ volume);
                break;
            }
        }
    }
}

Program program włącza telewizor, ale informacja o tym nie wyświetla się do końca poprawnie.
Pętla odpowiedzialna za kanały i głośność wykonuje się tylko jeden raz. Kanały również wiem, że w tej formie nie będą przełączać się w sposób zapętlony.
Jeśli ktoś by mi pomógł w naprawie tych błędów i wyjaśnieniu co zrobiłem źle, to będę wdzięczny ;)

KamilAdam
  • Rejestracja:ponad 6 lat
  • Ostatnio:około miesiąc
  • Lokalizacja:Silesia/Marki
  • Postów:5505
1

Tak na szybko:

  • Po if(turning.equals("off")) brakuje ci nawiasów
  • Ten break w pętli jest bez sensu. W sensie zamienia pętle w zwykłego ifa powinno być bardziej:
Kopiuj
if (choose!=("off")) {
  // miejsce na switch
} else {
   break; // lub return;
}

lub wręcz "off" umieścić w switchu, ale wtedy musisz użyć return;


Mama called me disappointment, Papa called me fat
Każdego eksperta można zastąpić backendowcem który ma się douczyć po godzinach. Tak zostałem ekspertem AI, Neo4j i Nest.js . Przez mianowanie
edytowany 4x, ostatnio: KamilAdam
BornStubborn
  • Rejestracja:ponad 5 lat
  • Ostatnio:ponad 2 lata
  • Postów:60
0

Rzeczywiście, nie zauważyłem braku tych nawiasów.
Ok, zmieniłem te kilka rzeczy, ale coś jest jeszcze nie tak.

Kopiuj
import java.util.Scanner;
public class Remote{
    boolean onOff = false;
    int channel = 0;
    int volume = 1;
    public Scanner scanner =new Scanner(System.in);

    public void onOff(){
        System.out.println("TV is off");
        String turning =scanner.next();
        if(turning.equals("off")) {
            onOff = false;
            System.out.println("TV is off");
        }

        if (turning.equals("on")) {
            onOff = true;
            System.out.println("TV is on");
        }
        while (onOff){
            System.out.println("TV działa. Co chcesz teraz zrobić?\n" +
                    "Masz do dyspozycji 9 kanałów i głośność w przedziale 1-15.\n" +
                    "Kanały możesz zmieniać za pomocą polecenia 'chup' oraz 'chdn', \n" +
                    "głośność natomiast za pomocą 'vup' i 'vdn'.\n" +
                    "Możesz wyłączyć TV przy pomocy 'off'");

            String choose = scanner.next();
            if (choose!=("off")){
                switch (choose) {

                    case "chup":
                        if (choose.equals("chup"))
                            for (int i = 0; i < 9; i++) {
                                channel = channel++;
                            }

                    case "chdn":
                        if (choose.equals("chdn"))
                            for (int i = 0; i < 9; i--) {
                                channel = channel--;
                            }

                    case "vup":
                        if (choose.equals("vup"))
                            for (int i = 1; i < 15; i++) {
                                volume = volume++;
                            }

                    case "vdn":
                        if (choose.equals("vdn"))
                            for (int i = 1; i < 15; i++) {
                                volume = volume--;
                            }
                }
                System.out.println("Obecny kanał to "+ channel + "\n Obecna głośność to "+ volume);
            }
            else{
                break;
            }
        }
    }
}

Pętla teraz się powtarza, ale wybierając którekolwiek z poleceń spod switch'a, otrzymuję jedynie jednorazową odpowiedź. Każde następne polecenie, nie zmienia ani głośności ani kanału. Jak to mogę naprawić? I jak wyrazić przedział tych kilku kanałów, żeby były one zapętlone?

KamilAdam
  • Rejestracja:ponad 6 lat
  • Ostatnio:około miesiąc
  • Lokalizacja:Silesia/Marki
  • Postów:5505
1
  • Jeśli sprawdzasz zmienną za pomocą switch to nie ma sensu jeszcze raz sprawdzać jej za pomocą ifa
  • switch jest bardzo wredną konstrukcją. Jeśli chcesz żeby przerwał się po wykonaniu jednego case musisz dać break przed zaczęciem nowego case.

Czyli:

Kopiuj
                    case "chup":
                            for (int i = 0; i < 9; i++) {
                                channel = channel++;
                            }
                   break;
                    case "chdn":
                            for (int i = 0; i < 9; i--) {
                                channel = channel--;
                           }
                   break;

Swoją drogą te pętle też są błędnie napisanie i robią bez sensu rzeczy


Mama called me disappointment, Papa called me fat
Każdego eksperta można zastąpić backendowcem który ma się douczyć po godzinach. Tak zostałem ekspertem AI, Neo4j i Nest.js . Przez mianowanie
edytowany 1x, ostatnio: KamilAdam
PR
  • Rejestracja:około 5 lat
  • Ostatnio:około 5 lat
  • Postów:2
1

Wyrażenie channel = channel++, nie zrobi inkrementacji na zmiennej.
Sprawdz sobie różnice między wyrażeniami:

Kopiuj
        int channel = 0;
        channel = channel++;
        channel = ++channel;
        channel += channel; 
        channel++;
        channel = channel + 1;   

Warunek if przed switch'em też niepotrzebny, możesz dodać kolejny case - "off" do obsłużenia wyłączania, czyli zmiany warunku wyjścia z pętli while.
Nie używaj pętli for do walidacji czy numer jest w odpowiednim zasięgu, o ile o to chodzi w tym przypadku.

edytowany 2x, ostatnio: prwn
BornStubborn
  • Rejestracja:ponad 5 lat
  • Ostatnio:ponad 2 lata
  • Postów:60
0

Ogromnie dziękuje wszystkim za rady i podpowiedzi :)
Poprawiłem na ich podstawie kod, obecnie działa i wygląda tak:

Kopiuj
import java.util.Scanner;
public class Remote{
    boolean onOff = false;
    int channel = 1;
    int volume = 0;
    public Scanner scanner =new Scanner(System.in);

    public void onOff(){
        System.out.println("TV is off");
        String turning =scanner.next();
        if(turning.equals("off")) {
            onOff = false;
            System.out.println("TV is off");
        }

        if (turning.equals("on")) {
            onOff = true;
            System.out.println("TV is on");
        }
        while (onOff){
            System.out.println("TV działa. Co chcesz teraz zrobić?\n" +
                    "Masz do dyspozycji 9 kanałów i głośność w przedziale 1-15.\n" +
                    "Kanały możesz zmieniać za pomocą polecenia 'chup' oraz 'chdn', \n" +
                    "głośność natomiast za pomocą 'vup' i 'vdn'.\n" +
                    "Możesz wyłączyć TV przy pomocy 'off'");

            String choose = scanner.next();
            switch (choose) {

                case "chup":
                    if (choose.equals("chup"))
                        if(channel<9) {
                            channel++;
                        }
                    else
                    break;

                case "chdn":
                    if (choose.equals("chdn"))
                        if(channel>1) {
                            channel--;
                        }
                    else
                    break;

                case "vup":
                    if (choose.equals("vup"))
                        if (volume<15) {
                            volume++;
                        }
                    else
                    break;

                case "vdn":
                    if (choose.equals("vdn"))
                        if (volume>0) {
                            volume--;
                        }
                    else
                    break;

                case "off":
                    if (choose.equals("off"))
                        onOff = false;
                    break;
            }
            System.out.println("Obecny kanał to "+ channel + "\n Obecna głośność to "+ volume);
        }
    }
}

Chciałbym Was jeszcze zapytać, czy kod w obecnej formie jest jakkolwiek zjadliwy? Czy może warto byłoby zastosować tu inne, schludniejsze mechanizmy? I czy znajduje się w nim coś co jest zbędne?

KamilAdam
  • Rejestracja:ponad 6 lat
  • Ostatnio:około miesiąc
  • Lokalizacja:Silesia/Marki
  • Postów:5505
1

Po co sprawdzasz choose dwa razy?

Kopiuj
                case "chup":
                    if (choose.equals("chup"))
                        if(channel<9) {
                            channel++;
                        }
                    else
                    break;

Wystarczy kod:

Kopiuj
                case "chup":
                        if(channel<9) {
                            channel++;
                        }
                    break;

BTW Niezły mindf**k. Musiałem się chwilę zastanowić zanim doszedłem jak to w ogóle się kompiluje. Co mnie utwierdza w przekonaniu że switch w obecnej formie powinien zostać zaorany z Javy XD
else wiąże break i nadmiarowy if przestaje być nadmiarowy.


Mama called me disappointment, Papa called me fat
Każdego eksperta można zastąpić backendowcem który ma się douczyć po godzinach. Tak zostałem ekspertem AI, Neo4j i Nest.js . Przez mianowanie
edytowany 1x, ostatnio: KamilAdam
BornStubborn
Serio? :D Nie używa się przeważnie java switch'a na co dzień w większości projektów, czy to raczej osobiste preferencje? ;)
KamilAdam
Nie chodzi mi o to czy się używa czy się nie używa, ale o to że switch jest brzydką niestrukturalną konstrukcją. I łatwo popełnić błąd. Klasycznego switcha z C nie ma ani w Kotlinie, ani w Scali. Także w Javie 12 jest dostępny ulepszony switch bez tego problemu https://dzone.com/articles/jdk-12-switch-statementsexpressions-in-action
BornStubborn
No to już rozumiem ;) może i rzeczywiście ten ulepszony switch jest ładniejszy
BornStubborn
  • Rejestracja:ponad 5 lat
  • Ostatnio:ponad 2 lata
  • Postów:60
0

Faktycznie sprawdzam java choose dwa razy, nie dotarło to do mnie xo Poprawiłem to już. Wszystko ładnie działa i lepie wygląda :) Bardzo dziękuję za pomoc :)

DR
  • Rejestracja:około 12 lat
  • Ostatnio:około 21 godzin
  • Postów:1131
0

@BornStubborn: Ja jeszcze Scanner bym nie trzymał jako pole klasy. I albo zmienił nazwę metody onOff, albo wydzielił np. wypisywanie informacji na ekran i wybieranie kanałów/głośności do innej, bo teraz dla kogoś z zewnątrz nazwa jest myląca.

BornStubborn
  • Rejestracja:ponad 5 lat
  • Ostatnio:ponad 2 lata
  • Postów:60
0

@Dregorio: Dzięki wielkie za podpowiedź : )

Zarejestruj się i dołącz do największej społeczności programistów w Polsce.

Otrzymaj wsparcie, dziel się wiedzą i rozwijaj swoje umiejętności z najlepszymi.