Meine erste Klasse CMenu

[c4f]

Hi Community
Ich bin noch relativ neu und unerfahren in C++ und hab keine Ahnung was ich so schreiben kann un nahm deswegen ein ähnliches Beispiel das ich dieses Schuljahr in PASCAL geschrieben hatte. Die Klasse heist CMenu , der Benutzer der Klasse kann dynamisch ein Menü erstellen. Das besondere allerdings (wie ich finde) die einzelen Menüpunkte kann man in verschiedener Farbe erstellen und das Menü lässt sich auch per Tastatur steuern (funktioniert jedenfalls bei mir).

Nun meine bitte ist es (natürlich wenn ihr wollt) die Klasse mal durchzuschaun und mir eventuell Verbesserungsvorschläge und/oder Tipps geben könnt was ich besser machen könnte, das würde mich sehr freuen

Hier ist dann mal der Code der Klasse + unten ist ein fertiges Beispiel zur Handhabung

CMenu.h

#ifndef _CMENU_H
#define _CMENU_H

// ------------------------------------------------------------
// AUTOR	-	Laurent
// DATEI    -	CMenu.h
// ------------------------------------------------------------

// ------------------------------------------------------------
// INCLUDES
// ------------------------------------------------------------

#include "stdafx.h"
#include <string>
#include <iostream>
#include <conio.h>

#include <windows.h>

#define UP_KEY		72
#define DOWN_KEY	80
#define ENTER_KEY	13

// ------------------------------------------------------------
// AUFZÄHLUNGEN
// ------------------------------------------------------------

enum COLOR_TABLE { BLACK,				
				   DK_BLUE,				
				   DK_GREEN,			
				   DK_BLUEGREEN,			
				   DK_REDWHITE,
				   DK_VIOLET,
				   DK_YELLOW,
				   GRAY,
				   DK_GRAY,
				   BLUE,
				   GREEN,
				   BLUEGREEN,
				   RED,
				   VIOLET,
				   YELLOW,
				   WHITE
				 };

// ------------------------------------------------------------
// STRUKTUREN
// ------------------------------------------------------------

struct MENUITEM {
	std::string menuText;
	COLOR_TABLE clrNorm;
	COLOR_TABLE clrOver;
};

// CMENU
// ------------------------------------------------------------
// CMenu erstellt dynamisch ein Menü und gibt die Menüpunkte aus
// Liest die Wahl des Benutzers ein
// Gibt diese wieder zurück

// Extras : Farbiges Menü
//        : Live-Steuerung mit Tasten 

class CMenu {
public:
	CMenu(int menuCount, const char *appName, COLOR_TABLE appNameClr);
	~CMenu();
	CMenu(const CMenu &rhs);

	void SetMenuItem(const char *menuItemTxt, COLOR_TABLE clrNorm, COLOR_TABLE clrOver);
	void ShowMenu() const;
	int GetUserChoice();

private:
	int			myMax;			// Gesamtanzahl der zu erstellenden Menüpunkte
	int			myCounter;		// Inkrementiert sich selbst jedes mal pro Aufruf
								// von createMenuItem() / dient später zum Vergleich
	int			myUserChoice;   // Benutzerauswahl
	bool		error;
	std::string myAppName;		// Enthält den Namen des Programms
	int			key;			// "Beinhaltet" die Tastencodes
	MENUITEM	*p_sMenuItem;	// Verwaltet die Daten 
	COLOR_TABLE myAppNameClr;

	// Private Elementfunktionen
	void IncreaseCounter() { ++myCounter; }
	void SetTextcolor(COLOR_TABLE clr) const;
	int ReadKey();
	bool Compare();
	void GetData() const;
	void ShowError() const { std::cout << "\n\n\t########## Zugriff auf nicht existierendes Menueelement! ##########" 
									 << "\n\t\t\t     Aufruf #" << myCounter+1 << " von SetMenuItem" 
									 << "\n\t\t\t    Maximale Menueelemente :  " << myCounter << std::endl; }
};

// ------------------------------------------------------------

#endif // _CMENU_H

CMenu.cpp

// ------------------------------------------------------------
// AUTOR	-	Laurent
// DATEI    -	CMenu.cpp
// ------------------------------------------------------------

// ------------------------------------------------------------
// INCLUDES
// ------------------------------------------------------------

#include "stdafx.h"
#include "CMenu.h"

// KONSTRUKTOR
// ------------------------------------------------------------

CMenu::CMenu(int menuCount, const char *appName, COLOR_TABLE appNameClr) :
	myMax(menuCount), myAppName(appName), myAppNameClr(appNameClr),
	myCounter(0), error(false) {

		p_sMenuItem = new MENUITEM[myMax];

		// std::cout << "CMenu(int, const char *)-Konstruktor..." << std::endl;
}

// KOPIERKONSTRUKTOR
// ------------------------------------------------------------

CMenu::CMenu(const CMenu &rhs) :
	myMax(rhs.myMax), myCounter(rhs.myCounter), myAppName(rhs.myAppName),
		myAppNameClr(rhs.myAppNameClr), error(rhs.error) {

		p_sMenuItem = new MENUITEM[myMax];

		for(int i = 0; i < myCounter; ++i) {
			p_sMenuItem[i].clrNorm		= rhs.p_sMenuItem[i].clrNorm;
			p_sMenuItem[i].clrOver		= rhs.p_sMenuItem[i].clrOver;
			p_sMenuItem[i].menuText		= rhs.p_sMenuItem[i].menuText;
		}

		// std::cout << "CMenu(const CMenu &)-Kopierkonstruktor..." << std::endl;
}

// DESTRUKTOR
// ------------------------------------------------------------

CMenu::~CMenu() {
	delete[ ] p_sMenuItem;
	// Textfarbe wieder auf Weiß setzen
	SetTextcolor(WHITE);

	// std::cout << "~CMenu()-Destruktor..." << std::endl;
}

// ------------------------------------------------------------
// ELEMENTFUNKTIONEN IMPLEMENTIEREN
// ------------------------------------------------------------

// SetMenuItem
// ------------------------------------------------------------
void CMenu::SetMenuItem(const char *menuItemTxt, COLOR_TABLE clrNorm, COLOR_TABLE clrOver) {
	IncreaseCounter();  // Neuer Menupunkt erstellen - myCounter erhöhen
	if(Compare()) { // Vergleicht myCounter mit myMax
		--myCounter;  // Sicherheit!!!! Wenn nicht führt der Aufruf von ShowMenu() zu einem
					  // schweren Fehler (laut VS2005)
		ShowError();
	} else {
		p_sMenuItem[myCounter-1].clrNorm  = clrNorm;
		p_sMenuItem[myCounter-1].clrOver  = clrOver;
		p_sMenuItem[myCounter-1].menuText = menuItemTxt;
	}
}

// ShowMenu
// ------------------------------------------------------------
void CMenu::ShowMenu() const {
	if(!error) {
		SetTextcolor(myAppNameClr);
		std::cout << myAppName << "\n\n";

		SetTextcolor(p_sMenuItem[0].clrOver);
		std::cout << p_sMenuItem[0].menuText << std::endl;

		GetData();
	}
}

// SetTextcolor
// ------------------------------------------------------------
int CMenu::GetUserChoice() {
	// tempörare variablen
	int pos = 1;

	if(!error) {
		do {
			key = ReadKey();
			// Das Herzstück - die Steuerung, das war leider die beste Idee
			// die ich hatte um die Steuerung zu realisieren
			// Habe versucht meinen Weg im Kopf hier so genau wie möglich zu
			// beschreiben
			switch(key) {
				case UP_KEY :	system("cls");
								SetTextcolor(myAppNameClr);
								std::cout << myAppName << "\n\n";

								if(pos == 1) {			// Wenn pos == 1 ist
									pos = myCounter;	// Wird pos auf den letzen Menüpunkt "verschoben"

									for(int i = 0; i < myCounter-1; ++i) {		// Da pos die ID des letzen Menüpunkts besitzt
										SetTextcolor(p_sMenuItem[i].clrNorm);   // können wir alles Menüpunkte vor pos ausgeben
										std::cout << p_sMenuItem[i].menuText << std::endl;
									}
									SetTextcolor(p_sMenuItem[myCounter-1].clrOver);  // und jetzt der letzte Menüpunkt
									std::cout << p_sMenuItem[myCounter-1].menuText << std::endl;
								} else {
									--pos;

									for(int i = 0; i < pos-1; ++i) {			// Alle Menüpunkte vor pos werden in normaler
										SetTextcolor(p_sMenuItem[i].clrNorm);	// Farbe ausgeben
										std::cout << p_sMenuItem[i].menuText << std::endl;
									}

									SetTextcolor(p_sMenuItem[pos-1].clrOver);				// Menüpunkt mit markierter Farbe ausgeben auf das
									std::cout << p_sMenuItem[pos-1].menuText << std::endl;	// pos "zeigt"

									for(int i = pos; i < myCounter; ++i) {		// Alle Menüpunkte nach pos wieder mit
										SetTextcolor(p_sMenuItem[i].clrNorm);	// "normaler" Textfarbe ausgeben
										std::cout << p_sMenuItem[i].menuText << std::endl;
									}
								}
							break;
				case DOWN_KEY : system("cls");
								SetTextcolor(myAppNameClr);
								std::cout << myAppName << "\n\n";

								// Hier wieder genau das selbe, nur in umgedrehter Reihenfolge
								if(pos == myCounter) {
									pos = 1;

									SetTextcolor(p_sMenuItem[pos-1].clrOver);
									std::cout << p_sMenuItem[pos-1].menuText << std::endl;

									for(int i = 1; i < myCounter; ++i) {
										SetTextcolor(p_sMenuItem[i].clrNorm);
										std::cout << p_sMenuItem[i].menuText << std::endl;
									}
								} else {
									++pos;
									for(int i = 0; i < pos-1; ++i) {
										SetTextcolor(p_sMenuItem[i].clrNorm);
										std::cout << p_sMenuItem[i].menuText << std::endl;
									}

									SetTextcolor(p_sMenuItem[pos-1].clrOver);
									std::cout << p_sMenuItem[pos-1].menuText << std::endl;

									for(int i = pos; i < myCounter; ++i) {
										SetTextcolor(p_sMenuItem[i].clrNorm);
										std::cout << p_sMenuItem[i].menuText << std::endl;
									}
								}
							break;
				default:
					break;
			}
		}while(key != ENTER_KEY);

		return(pos);  // pos = automatisch die Wahl des Benutzers
	}
	return(-1);
}

// SetTextcolor
// ------------------------------------------------------------
void CMenu::SetTextcolor(COLOR_TABLE clr) const {
	SetConsoleTextAttribute(::GetStdHandle(STD_OUTPUT_HANDLE), clr);
}

// ReadKey
// ------------------------------------------------------------
int CMenu::ReadKey() {
	return(_getch());
}

// Compare
// ------------------------------------------------------------
bool CMenu::Compare() {
	if(myCounter > myMax) {
		return(error = true);
	}
	return(error);
}

// GetData
// ------------------------------------------------------------
void CMenu::GetData() const {
	for(int i = 1; i < myCounter; ++i) {
		SetTextcolor(p_sMenuItem[i].clrNorm);
		std::cout << p_sMenuItem[i].menuText << std::endl;
	}
}

beispiel.cpp

// ------------------------------------------------------------
// AUTOR	-	Laurent
// DATEI    -	CMenu_Test.cpp
// ------------------------------------------------------------

#include "stdafx.h"
#include "CMenu.h"

int main()
{
	CMenu *pCMenu = new CMenu(3, "\t\t\t\tCMenu Demonstration", DK_GRAY);
	int choice;

	pCMenu->SetMenuItem("\tMenuitem 1", DK_GRAY, GRAY);
	pCMenu->SetMenuItem("\tMenuitem 2", DK_GRAY, GRAY);
	pCMenu->SetMenuItem("\tMenuitem 3", DK_GRAY, GRAY);

	// Fehlerausgabe -  > 3 
	// pCMenu->SetMenuItem("\tMenupunkt 4", DK_GRAY, GRAY);

	pCMenu->ShowMenu();

	choice = pCMenu->GetUserChoice();
	delete pCMenu;  // Speicher freigeben, Textfarbe wird automatisch wieder auf WHITE gesetzt 

	// Benutzerauswahl zur Demonstration ausgeben
	std::cout << "\n\n\tBenutzerwahl : " << choice;

	pCMenu = 0;

	std::cin.get();
	return 0;
}

Vielen dank für eure Tipps und Verbesserungsvorschläge
Die Klasse funktioniert bei mir zu 100%

schönen Tag noch
lg c4f

[Maxi]

sieht doch ganz ordentlich aus

was mir auffällt:

- warum nimmst du keinen std::vector<MenuItem> anstatt deines Arrays? Dann hast du auch nicht den schweren Fehler, wenn du dort immer mit push_back anfügst, er holt sich automatish speicher

- Deine Funktion Compare funktioniert zwar, aber der Leser des Codes weiß überhaupt nicht, was damit gemeint ist. Sie sollte vl eher heißen: NochPlatz() oder so

- ShowError gibt immer nur den einen Fehler aus. Du solltest diese Methode lieber flexibler machen, indem du ihr einen Parameter gibst, der ihr sagt, was sie denn ausgeben soll.

- in deiner Methode GetUserChoice hast du sehr redundanten Code, sprich du wiederholst dich sehr oft. Versuche, den ähnlichen Code in eine Fuunktion auszulagern und diese dann aufzurufen.

for(int i = 0; i < pos-1; ++i) {            // Alle Menüpunkte vor pos werden in normaler
                                        SetTextcolor(p_sMenuItem[i].clrNorm);    // Farbe ausgeben
                                        std::cout << p_sMenuItem[i].menuText << std::endl;
                                    }

                                    SetTextcolor(p_sMenuItem[pos-1].clrOver);                // Menüpunkt mit markierter Farbe ausgeben auf das
                                    std::cout << p_sMenuItem[pos-1].menuText << std::endl;    // pos "zeigt"

                                    for(int i = pos; i < myCounter; ++i) {        // Alle Menüpunkte nach pos wieder mit
                                        SetTextcolor(p_sMenuItem[i].clrNorm);    // "normaler" Textfarbe ausgeben
                                        std::cout << p_sMenuItem[i].menuText << std::endl;
                                    }

könntest du auch so machen (fände ich perönlich besser)

i von 0 bis ende
  wenn punkt i nicht gewählt, 
    settextcolor(normal)
  sonst
    settextcolor(gewählt)

  gibPunktAus()

- key muss nicht in der klasse definiert sein, es reicht doch lokal in GetUserChoice, oder?

- Getuserchoice sollte vor dem return noch die schriftfarbe auf normal setzen. Diese solltest du dir vorher merken, es kann ja sein, dass der nutzer vor mit rot gschrieben hat und jetzt so auch weiter schreiben möchte

zum beispiel: Warum legst du das Menü auf dem Heap an? Reicht nicht eine einfache stack-variable?

[Erhard Henkes]

Ich habe es mit Code::Blocks getestet, es funktioniert. Solltest vielleicht einen größeren Demo-Code liefern.

[c4f]

Maxi schrieb:

zum beispiel: Warum legst du das Menü auf dem Heap an? Reicht nicht eine einfache stack-variable?

Erstmal danke dass du dir zeit genommen hast

Warum auf dem Heap, mir wurde es so beigebracht (laut Buch) dass man größere Objekte von Klassen direkt auf den Heap ablegen soll da das schneller laufn würde im Verglecih zum Stack...und da dacht ich ok nehmen wir halt den Heap...wie gesagt ich bin noch Anfänger und hab keine Erfahrung, vor allem nicht mit Zeiger, und Speicher

Ich habe vector nicht genommen da ich noch nie was mit Vektoren zu tun hatte und die in meinem Buch noch nicht vergekommen sin, ich weiß schon was Vektoren sind aber ich wollte das Wissen einsetzen was ich momentan verfüge.

Nochmals vielen dank für deine Anregungen
Ich werde die Klasse in nächster Zeit überarbeiten um sie "Perfekt" zu machen

lg c4f

[Erhard Henkes]

std::vector hat nichts mit mathematischen Vektoren zu schaffen, sondern ist ein Container, der sehr leicht zu verwenden ist:

http://www.yolinux.com/TUTORIALS/LinuxTutorialC++STL.html#VECTOR
http://www.c-plusplus.net/forum/viewtopic-var-t-is-143816.html

[c4f]

Erhard Henkes schrieb:

std::vector hat nichts mit mathematischen Vektoren zu schaffen, sondern ist ein Container, der sehr leicht zu verwenden ist:

Das hab ich auch gemeint

Danke für die Links

lg c4f