Prosty program pilot (remote)

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

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

klasa Remote

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 ;)

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:
if (choose!=("off")) {
  // miejsce na switch
} else {
   break; // lub return;
}

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

0

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

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?

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:

                    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

1

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

        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.

0

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

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?

1

Po co sprawdzasz choose dwa razy?

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

Wystarczy kod:

                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.

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 :)

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.

0

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

1 użytkowników online, w tym zalogowanych: 0, gości: 1