Optymalizacja programu obiektowego (C++)

Question

Z góry proszę tylko o podpowiedź !

Chcę zoptymalizować program korzystając z bibliotek proszę o podpowiedź jakich mógłbym do tego użyć. Program pokazuje działania stosu. Dodawanie do niego elementów i usuwanie.

Plik nagłówkowy:

#ifndef P_H_
#define P_H_

typedef int Item;
class Stos
{
        private:
                enum {MAX=10}; //stała zasięgu klasy
                Item items[MAX]; 
                int top;
        public:
                Stos();
                bool isempty() const; 
                bool isfull() const;
                bool push(const Item & item);
                bool pool(const Item & item);
};
#endif

Deklaracje:

#include "p.h"

Stos::Stos() { top=0; }
bool Stos::isempty() const { return top == 0; }
bool Stos::isfull() const { return top == MAX; }
bool Stos::push(const Item & item)
{
        if(top < MAX)
        {       items[top++] = item;
                return true; }
        else
                return false;
}

bool Stos::pool (const Item & item)
{
        if(top > 0)
        {       items[--top] = item;
                return true ; }
        else
                return false;
}

Plik główny:

#include "p.h"
#include <iostream>
#include <cctype>
int main()

{
        Stos box;
        char letter;
        int value;
        std::cout << "Wcisniej A (żeby wprowadzić deklarację), S (żeby przetworzyć deklarację) lub D (żeby zako”czyć)\n";
        while(std::cin >> letter && toupper(letter) != 'D')
        {
                while(std::cin.get() != '\n')
                        continue;
                if(!isalpha(letter))
                {       std::cout << 'a'; continue; }
                switch(letter)
                {
                        case 'A':
                                case 'a': std::cout << "Podaj nr nowej deklaracji: ";
                                      std::cin >> value;
                                      if(box.isfull())
                                              std::cout << "Stos jest pełny\n";
                                      else
                                            box.push(value);
                                      break;

                        case 'S':
                                case 's': if(box.isempty())
                                                std::cout << "Stos jest pusty\n";
                                          else {
                                                box.pool(value);
                                                std::cout << "Deklaracja nr " << value << " została zdjęta\n"; }
                                     break;
                }//switch
                        std::cout << "Wcisniej A (żeby wprowadzić deklarację), S (żeby przetworzyć deklarację) lub D (żeby zako”czyć)\n";
        }//while
        std::cout << "Koniec programu!\n\n";
        return 0;
}//main

 

Comments

I chciałbym tak tylko dodać, że ten Twój kod po optymalizacji jest strasznie brzydki. Po co Ci te zmienne globalne?

Prześlij je jak normalny programista w argumentach funkcji.

---

@RafalS,

Dziękuję nie wiedziałem o tej metodzie nagłówka.

Tak więc wprowadze ponowne zmiany ;)

---

A jakby zrobić w ten sposób

#include <iostream>
#include <cctype>
#include <stack>

void stack_empty(std::stack <int> mystack);
int main()
{
        std::stack <int> mystack1;
        char letter;
        int value;
        stack_empty(mystack1);
        std::cout << "Wcisniej A (żeby dodać element do stosu), S (żeby przetworzyć element stosu) lub D (żeby zakoczyć)\n";
        while(std::cin >> letter && toupper(letter) != 'D')
        {
                while(std::cin.get() != '\n') continue;
                if(!isalpha(letter)) { std::cout << '\a'; continue; }
                switch(letter)
                {
                        case 'A':
                                case 'a': 
                                        mystack1.push(value);
                                        stack_empty(mystack1);
                                break;

                        case 'S':
                                case 's':
                                        stack_empty(mystack1);
                                        mystack1.pop();
                                        std::cout << "Ostatni dodany element został zdjęty!\n";
                                        stack_empty(mystack1);
                                break;
                }//switch

                std::cout << "Wcisniej A (żeby dodać element stosu), S (żeby przetworzyć element stosu) lub D (żeby zakoczyć)\n";
        }//while
std::cout << "Koniec programu!\n\n";
        return 0;
}//main

void stack_empty(std::stack <int> mystack)
{
        if(mystack.empty()) 
                std::cout << "Kontener jest pusty!\n";
        else
                std::cout << "Kontener posiada elementow: " << mystack.size() << "\n";
}

 

---

Lepiej. Nie podoba mi się jeszcze ten kawałek:

                        case 'A':
                                case 'a':

zrób toupper a nie takie dziwactwa :P

void stack_empty(std::stack <int> mystack)

Nie podoba mi się nazwa funkcji. Strasznie niejasna, a jeśli już miałbym się czegoś domyślać to powiedziałbym, że będzie ona opróżniać stos. A potem widzę, że dostaje stos przez wartość, więc niczego już nie mogę się domyślić.

Zadaj sobie pytanie co ta funkcja robi i tak ją nazwij. A co robi - wypisuje rozmiar stosu. Czy nazwa print_stack_size nie byłaby bardziej odpowiednia? Dodatkowo prześlij stos przez stałą referencję, bo co jeśli będziesz operował na stosie z 1000000 elementami. Wtedy głupie przepisanie będzie wymagało skopiowania całego stosu. W przypadku referencji nic nie będzie kopiowane:
 

void print_stack_size(const std::stack<int>& mystack);

---

Dziękuję bardzo za pomoc.

Tak to wygląda po poprawkach.

#include <iostream>
#include <cctype>
#include <stack>

void print_stack_size(std::stack <int> & mystack);
int main()
{
        std::stack <int> mystack1;
        char letter;
        int value;
        print_stack_size(mystack1);
        std::cout << "Wcisniej A (żeby dodać element do stosu), S (żeby przetworzyć element stosu) lub D (żeby zakoczyć)\n";
        while(std::cin >> letter && toupper(letter) != 'D')
        {
                while(std::cin.get() != '\n') continue;
                if(!isalpha(letter)) { std::cout << '\a'; continue; }
                switch(toupper(letter))
                {
                        case 'A':
                                mystack1.push(value);
                                print_stack_size(mystack1);
                        break;

                        case 'S':
                                print_stack_size(mystack1);
                                mystack1.pop();
                                std::cout << "Ostatni dodany element został zdjęty!\n";
                                print_stack_size(mystack1);
                        break;
                }//switch

                std::cout << "Wcisniej A (żeby dodać element stosu), S (żeby przetworzyć element stosu) lub D (żeby zakoczyć)\n";
        }//while
        std::cout << "Koniec programu!\n\n";
        return 0;
}//main

void print_stack_size(std::stack <int> & mystack)
{
        if(mystack.empty()) 
                std::cout << "Kontener jest pusty!\n";
        else
                std::cout << "Kontener posiada elementow: " << mystack.size() << "\n";
}