Shop



  • Hi @ all!

    Ich möchte ein Shop implementieren (mit Stacks). Das Warenhaus soll dabei organisiert werden, s.d. alle Produkte kategorisiert sind. D.h. man soll Produkte ins Warenhaus laden können, und wieder davon wegnehmen.
    Der Laden soll die Güter "in a stack-fashion" organisieren.

    Zu den Codeteilen zum Warenhaus und dem Laden habe ich Fragen, welche ich jeweils direkt als Kommentar reingeschrieben habe.

    Warenhaus:

    #include "warehouse.hpp"
    
    #include <array>
    template <typename Object>
    Object *S; //ist das korrekt so? :S
    
    warehouse::warehouse();
    
    bool warehouse::empty(void) const
    {
    	if (warehouse.size() = 0) //ist warehouse hier wirklich richtig? :S
    	{
    		return true;
    	}
    	else
    	{
    		return false;
    	}
    }
    
    unsigned int warehouse::max_capacity(void) const
    {
    	return MAX_WAREHOUSE_CAPACITY;
    }
    
    void warehouse::pop(void)
    {
    	S.pop(); // Ist S hier wirklich korrekt?
    }
    
    void warehouse::push(product& product_)
    {
    	S.push(product_); //wieder das "S"...
    }
    
    warehouse::product& top(void) //hier bin ich mir auch nicht so sicher..
    {
    	return S.top(); // "S"...
    }
    
    unsigned int warehouse::used_capacity(void) const
    {
    	return S.size(); //"S"...
    }
    
    array<product, MAX_WAREHOUSE_CAPACITY> warehouse::*_products
    {
    	for(int i=0, i<MAX_WAREHOUSE_CAPACITY +1, i++)
    	{
    			return S[i]->product.get_name(); // nicht sicher.
    	}
    }
    
    int warehouse::_top
    {
    	return S._top();
    }
    

    Laden:

    #include "product.hpp"
    #include "warehouse.hpp"
    // STL
    #include <iostream>
    
    /* simulation of a shop-environment */
    int main(void)
    {
        //  create the new warehouse
    	warehouse *haus = new warehouse;
    
        //  create a product
    	product *salami = new product;
    	salami::set_name("Salami"); //wie sähe das richtig aus? 
    	//........
    
        //  print the products, print max capacity, check emptiness,
        //       used capacity (how many elements are stored)
    	std::cout << "Wir haben das Produkt: " << product.get_name << " mit der max. Kapazitaet: " << warehouse.max_capacity << " und " << warehouse.used_capacity << "gespeicherte Elemente." << std::endl;
    
        // ADD products; check for size and emptiness
    	if(warehouse.used_capacity < (warehouse.max_capacity+1)) //hier stimmt auch was nicht..
    	{
    		warehouse->S.add(product); //auch hier: Wie sähe es richtig aus?
    	}
    
        //  REMOVE products; check for size and emptiness
    	if(warehouse.empty() = false) //auch hier stimmt was nicht..
    	{
    		warehouse->S.remove(product);
    	}
    
        return 0;
    }
    

    Für die Hilfe und Tipps wäre ich sehr dankbar.
    Liebe Grüsse, Thomas



  • Ist das eine Hausaufgabe "Finde so viele Fehler wie möglich?". In dem Code ist ja fast jede Zeile falsch.
    Setz dich erstmal mit einem guten Buch hin und zieh dir ein paar Grundzlagen rein. Die meisten Fehler sind so offensichtlich, dass Du sie dir dann selbst erklären kannst. Und die Frage "Ist das so richtig" kann dir sehr gut Dein Compiler beantworten.



  • Warum glaubst du wohl wird bei libs immer die .h mitgeliefert? Riiichtig, weil da alles wichtige drinsteht - sozusagen die Schnittstelle. Du fragst uns hier was für ne Farbe dein Auto hat und präsentierst uns den Zerlegten Motor und um es leichter zu machen, packst du die Hälfte der Teile auch noch in zue Schachteln... ansonsten - siehe Brotsbernds Kommentar.



  • Wo soll ich anfangen?

    Also die erste Frage, die ich mal so stelle:
    Warum verwendest du in deiner Klasse warehouse ein globales Element Object?
    Das müssten wir mal zuerst klären. Noch dazu ist das ein Zeiger der nie initialisiert wird.
    Sprich, das würde eh crashen mit einer NullPointerException.

    Dann:
    Warum erzeugst du in deiner Main lauter dynamische Objekte auf dem heap mit new? Warum erzeugst du die Objekte
    nicht einfach auf dem Stack? Würde die Memoryleaks verhindern.

    Kommst du von Java zufällig oder C#?



  • Von Java, ja.
    Ok, werde das nochmals durchgehen.



  • Scorcher24 schrieb:

    Sprich, das würde eh crashen mit einer NullPointerException.

    Nein, sowas gibts in C++ zum Glück nicht. Selbst wenn der Code kompilieren würde, wäre die Dereferenzierung eines Nullzeigers undefiniertes Verhalten (auf vielen Systemen allerdings ein sofortiger Laufzeitfehler).

    ThomiS schrieb:

    Ok, werde das nochmals durchgehen.

    Am besten nochmals ganz von vorne. Okay, Schleifen und die prozeduralen Sachen sind in C++ und Java fast gleich. Aber sobald Klassen ins Spiel kommen, sind die Gemeinsamkeiten nur noch oberflächlich. Es lohnt sich nicht, dein Java-Denken auf C++ anwenden zu wollen, da in C++ vieles anders gelöst wird. Diese Konzepte musst du neu lernen, oder du machst dir das Leben ständig schwieriger als nötig.



  • Okay, mal eine Regel für C++:
    Jedes new braucht ein delete.
    Jedes new[] braucht ein delete[].
    Es gibt keinen Garbage Collector. Jeder angeforderte Speicherplatz bleibt bis zum delete belegt. Auch wenn dein Programm sich beendet. Inwiefern hier noch das OS eingreifen kann bei einem Release Build, vermag ich jetzt nicht zu sagen (würde mich aber auch mal interessieren, btw. Also genaue Mechaniken der modernen OS zwecks solcher Leaks).

    Steck dein Objekt in die Klasse als privaten Member. Sollte man aber in Java auch so machen :D. Oder nicht?

    edit:

    Nein, sowas gibts in C++ zum Glück nicht. Selbst wenn der Code kompilieren würde, wäre die Dereferenzierung eines Nullzeigers undefiniertes Verhalten (auf vielen Systemen allerdings ein sofortiger Laufzeitfehler).
    

    Ja, nicht direkt. Ich meinte damit, dass es eben crasht mit einer hohen Chance. Da ich schon den Verdacht hatte, dass er Java User ist, wollte ich einen ihm bekannten Begriff verwenden.



  • salami::set_name("Salami"); //wie sähe das richtig aus?

    salami->set_name("Salami");
    

    Ein Zeiger verwendet den -> Operator. Das :: ist ein Scope. In C++ musst du alles was du mit new anlegst mit delete freigeben/löschen, das stört mich beinahe am meisten.

    Zeiger können nur auf schon existierende Objekte zeigen.

    //stack hier muss nichts gelöscht werden
    int main()
    {
       int a = 5;
       int *p = &a; //weist die bestehende Adresse der Stack-Variable dem Zeiger zu
    
       cout << *p;  //* dereferenziert die Adresse auf den Inhalt
    }
    
    //Heap hier muss gelöscht werden
    int main()
    {
       int *p = new int(5); //erstellt eine neue variable auf dem Heap mit dem Wert 5
    
       cout << *p;  //* dereferenziert die Adresse auf den Inhalt
    
       delete p; //den Speicher wieder freigeben
    }
    

  • Mod

    Scorcher24 schrieb:

    Inwiefern hier noch das OS eingreifen kann bei einem Release Build, vermag ich jetzt nicht zu sagen (würde mich aber auch mal interessieren, btw. Also genaue Mechaniken der modernen OS zwecks solcher Leaks).

    Die Mechanik ist ganz einfach: Prozess beendet, Prozesspeicher wird frei. Da Speicher und Prozesse vollständig vom OS verwaltet werden, gibt es keinen Weg daran vorbei und das funktioniert daher immer perfekt (Ja, auch bei OS wie Win95, das hatte andere Probleme, aber nicht beim Freigeben von Speicher).

    Die Betriebssysteme die das nicht tun (und die in diesem Zusammenhang genannt werden) sind ganz seltene Exoten die entweder uralt (>30 Jahre) oder mit Absicht so designed sind (z.B. OS für ältere Spielkonsolen, wo es kein "danach" gibt, weil die Konsole sowieso neu gestartet werden muss).



  • SeppJ schrieb:

    ...

    Was allerdings nicht als Rechtfertigung dafür verstanden werden soll, auf delete zu verzichten.

    Aber bei der Benutzung von RAII stellt sich die Frage eh nicht.



  • Soooo 🙂
    Mittlerweile funktioniert mein Code eigentlich.
    Allerdings habe ich noch 1, 2 additional questions. 🙂

    Mein warehouse sieht nun wie folgt aus:

    #include "warehouse.hpp"
    
    #include <array>
    
    warehouse::warehouse();
    
    bool warehouse::empty() const
    {
    	if (_top > -1) 
    	{
    		return false;
    	}
    	else
    	{
    		return true;
    	}
    }
    
    unsigned int warehouse::max_capacity(void) const
    {
    	return MAX_WAREHOUSE_CAPACITY;
    }
    
    void warehouse::pop()
    {
    	if(!empty())
        {
            product* p1 = &_products[_top];
            /*delete p1;
            p1 = NULL;*/
            _top = _top - 1; // = _top--;
        }
    }
    
    void warehouse::push(product& product_)
    {
    	product_ = &_products[_top+1];
    	_top = _top++;
    }
    
    product& top()
    {
    	return &_products[_top];
    }
    
    unsigned int warehouse::used_capacity() const
    {
    	warehouse *w = new warehouse;
    	return w.size();
    }
    
    array<product, MAX_WAREHOUSE_CAPACITY> _products
    {
    	// products stored in the warehouse
    	// steht bei warehouse.hpp unter "private", wie soll das genau implementiert werden? 
    	for(int i=0, i<MAX_WAREHOUSE_CAPACITY +1, i++)
    	{
    			return array<_products[i], MAX_WAREHOUSE_CAPACITY>;
    	}
    }
    

    Bei der Abfrage, welches das oberste Element ist (also die top-Methode), habe ich nicht wirklich raus gekriegt, wie man das korrekt implementieren müsste. Mein Compiler sagt zwar nichts, allerdings wird "_top" unterstrichen und vorgeschlagen, dass _top nicht definiert wurde.
    Das bringt mich gleich zur nächsten Frage: Im warehouse.hpp steht unter private einfach int _top (also der Index des obersten Elementes) - muss ich das auch in .cpp definieren? ..und wenn ja, dass frage ich mich eben, wie..?

    Eine Frage noch zur used-capacity-Methode: Mir wird das "w" unterstrichen und gesagt, es müsse sich um einen Klassentypen handeln. Aber das tut es doch eigentlich, oder?

    Der letzte Punkt hierzu ist das Array. Ich habe es zwar hingekriegt, dass der Compiler nicht reklamiert. Aber die Implementation ist nicht wirklich korrekt - daher meine dumme, aber wirklich ernst gemeinte Frage: Wie macht man das korrekter?

    Mein Shop looks like this:

    // Task2
    #include "product.hpp"
    #include "warehouse.hpp"
    // STL
    #include <iostream>
    
    /* simulation of a shop-environment */
    int main(void)
    {
        // create the new warehouse
    	warehouse *haus = new warehouse;
    
        // create few products
    	product *salami = new product[0];
    	salami->set_name("Salami");
    	//........
    
        // print the products, print max capacity, check emptiness,
        //       used capacity (how many elements are stored)
    	std::cout << "Wir haben das Produkt: " << product.get_name << " mit der max. Kapazitaet: " << warehouse.max_capacity << " und " << warehouse.used_capacity << "gespeicherte Elemente." << std::endl;
    
        //  ADD products; check for size and emptiness
    	if(haus->used_capacity() < (haus->max_capacity()+1))
    	{
    		haus->push(*salami); //Wie krieg ich das hin, ohne dass ich die Produkte explizit nennen muss?
    	}
    
        //  REMOVE products; check for size and emptiness
    	if(haus->empty())
    	{
    		haus->pop();
    	}
    
        return 0;
    }
    

    Meine Fragen hierzu sind: Wie kriegt man das mit der Produkterstellung hin, damit man die Produkte jeweils nicht explizit nennen muss?
    Meine Idee wäre, ein Array fester Grösse zu erstellen, und darin die Produkte speichern. Wäre das korrekt?
    Zudem: Müsste man nicht irgendwo noch die Produkte wieder deleten?
    Und allgemein: Sieht der Code einigermassen ok aus? 🙂

    Liebe Grüsse und vielen, vielen Dank!



  • Zum Löschen, ja, alles, was du mit new erstellst, musst du mit delete löschen. Ich sehe hier aber gar keinen Grund, new zu verwenden, mach doch einfach ein ganz normales Objekt.

    Wenn du einen Zeiger auf ein Objekt erstellst, musst du die Memberfunktionen mit -> ansprechen. Es muss also w->size() heißen 😉

    Was dieses _products da sein soll, weiß ich nicht.

    new T[0] erstellt ein Array für 0 Elemente auf dem Heap, das müsste eigentlich abstürzen, was du da tust.

    Zusammenfassend lässt sich sagen, dass du dir noch vieles ansehen solltest. Dein kompletter Code ließe sich wahrscheinlich ohne einen einzigen Zeiger schreiben.



  • Besten Dank für die wertvollen Tipps!

    Ich habe den Code nun soweit korrigiert, dass alle Klassen, bis auf shop und warehouse, sicher alle stimmen. Bei shop und warehouse haben sich noch 1, 2 Fehler reingeschlichen, die ich schlicht und einfach nicht finde...
    Daher wäre ich froh, wenn sich jemand meine beiden Source-Codes anschauen könnte.

    warehouse.cpp:

    #include "warehouse.hpp"
    
    #include <array>
    
    int _top = -1;
    
    bool warehouse::empty() const
    {
    	if (_top > -1) 
    	{
    		return false;
    	}
    	else
    	{
    		return true;
    	}
    }
    
    unsigned int warehouse::max_capacity(void) const
    {
    	return MAX_WAREHOUSE_CAPACITY;
    }
    
    void warehouse::pop()
    {
    	if(!empty())
        {
            product* p1 = &_products[_top];
            /*delete p1; // ist falsch!
            p1 = NULL;*/
            _top = _top - 1; // = _top--;
        }
    }
    
    void warehouse::push(product& product_)
    {
    	product_ = &_products[_top+1];
    	_top = _top++;
    }
    
    product& top()
    {
    	return &_products[_top];
    }
    
    unsigned int warehouse::used_capacity() const
    {
    	return _top;
    }
    
    array<product, MAX_WAREHOUSE_CAPACITY> _products
    {
    	// products stored in the warehouse
    	// Gehört dieser Abschnitt überhaupt hier hin? 
    	for(int i=0, i<MAX_WAREHOUSE_CAPACITY +1, i++)
    	{
    			return array<_products[i], MAX_WAREHOUSE_CAPACITY>;
    	}
    }
    
    // TODO
    

    warehouse.hpp:

    #ifndef WAREHOUSE_H
    #define WAREHOUSE_H
    
    #include "../../../shared/array.hpp"
    
    #include "product.hpp"
    
    #define MAX_WAREHOUSE_CAPACITY    100
    
    class warehouse
    {
    
        public:
            /**
            * @brief constructor
            *
            * definition of a constructor for initializing the top
            * index keeping track of the element on top of the stack
            */
            warehouse();
    
            /**
            * @brief checks of emptiness
            *
            * check if the warehouse is empty or not.
            */
            bool empty(void) const;
    
            /**
            * @brief max capacity of the warehouse
            *
            * returns the maximal number of products that can be
            * stored into the warehouse.
            */
            unsigned int max_capacity(void) const;
    
            /**
            * @brief remove last element
            *
            * removes the element on top of the stack
            */
            void pop(void);
    
            /**
            * @brief add an element
            *
            * adds an element on top of the stack
            */
            void push(product& product_);
    
            /**
            * @brief
            *
            * retrieves the element on top of the stack
            */
            product& top(void);
    
            /**
            * @brief used capacity of the warehouse
            *
            * returns the number of products that are store into the
            * warehouse.
            */
            unsigned int used_capacity(void) const;
    
        private:
            /* products stored in the warehouse  */
            array<product, MAX_WAREHOUSE_CAPACITY> _products;
            /* index of the element on the top*/
            int _top;
    
    };
    
    #endif  // WAREHOUSE_H
    

    shop.cpp:

    #include "product.hpp"
    #include "warehouse.hpp"
    // STL
    #include <iostream>
    #include <array>
    
    /* simulation of a shop-environment */
    int main(void)
    {
        // create the new warehouse
    	warehouse *haus;
    
        //  create few products
    	product *fleisch;
    	fleisch->set_name("Salami");
    	//........
    
        // print the products, print max capacity, check emptiness,
        //       used capacity (how many elements are stored)
    
    	std::cout << "Wir haben das Produkt: " << product.get_name << " mit der max. Kapazitaet: " << warehouse.max_capacity << " und " << warehouse.used_capacity << "gespeicherte Elemente." << std::endl;
    
        // ADD products; check for size and emptiness
    	if(haus->used_capacity() < (haus->max_capacity()+1))
    	{
    		haus->push(*fleisch);
    	}
    
        // REMOVE products; check for size and emptiness
    	if(!haus->empty())
    	{
    		haus->pop();
    	}
    
        return 0;
    }
    

    Vielen herzlichen Dank im Voraus!
    Liebe Grüsse, Thomas



  • product *fleisch; 
    fleisch->set_name("Salami");
    

    Zeiger initialisieren.

    Aber klüger wäre es, gar nicht erst Zeiger zu verwenden. Denn dann brauchst du auch keine manuelle Speicherverwaltung.

    product fleisch;
    fleisch.set_name("Salami");
    


  • Ok. Stimmt, so geht's viel einfacher 😃

    ..und das Array im ersten Snippet - ist das wirklich ok so?



  • Was funktioniert denn nicht? "Es haben sich 1-2 Fehler eingeschlichen" ist nicht die beste Beschreibung. Hast du es schon mal mit dem Debugger versucht?

    Wobei ich bei

    product_ = &_products[_top+1];
    

    nicht das Gefühl habe, dass der Code überhaupt kompiliert. Du weist einen Zeiger einer Referenz zu.


Anmelden zum Antworten