What'ch think of my program? *fixed word wrap*

/* This program is a database program that will store
video tape names, minutes, year released, and price
I want it to be professional so that *YOU* will want
to buy it and use it for a video store, j/k about that
but i want it to be professional */
// note to leor, display works fine, you must have
// pressed update


#include<iostream>
#include<string>
using namespace std;

int totalnodes= 0; // the nodes in this case are tapes


template<class X>
class Lnode{
public:
X value;
Lnode * next;
};

template<class Y>
class linkedlist{
Lnode<Y> *head;
public:
linkedlist();
~linkedlist();
void addNode(Y &); // insert a video
void deleteNode(Y &);
void updateNode(Y &);
void displayNode()const;
void searchNode(const Y &);
};

class VideoTape{

string name;
double time;
int year;
double price;
public:
VideoTape();
VideoTape(string);
~VideoTape();

friend istream& operator >>(istream &,VideoTape &);
friend ostream& operator <<(ostream &,const VideoTape &);

friend bool operator < (const VideoTape &, const VideoTape &);
friend bool operator > (const VideoTape &, const VideoTape &);
friend bool operator != (const VideoTape &, const VideoTape &);
friend bool operator == (const VideoTape &, const VideoTape &);

void changetime(double);
void changeyear(int);
void changeprice(double);
};


template<class Y>
linkedlist<Y>::linkedlist(){
head = NULL;
}

template<class Y>
linkedlist<Y>::~linkedlist(){
Lnode<Y> *nodePtr;
nodePtr = head;

while(nodePtr != NULL)
{
head = nodePtr->next;
delete nodePtr;
nodePtr = head;
}
}

template<class Y>
void linkedlist<Y>::addNode(Y &n){
Lnode<Y> * newnode, * nodePtr, * previousNode;


newnode = new Lnode<Y>;
newnode->value = n;

// Empty linked list

if(!head)
{
head = newnode;
newnode->next = NULL;
totalnodes++;

}
// not empty list
else
{
nodePtr = head;

// If head's value is greater and not equal to n
if(nodePtr->value > n && nodePtr->value != n){
previousNode = head;
head = newnode;
newnode->next = previousNode;
totalnodes++;
}

else{
// cycle through numbers while he pointer's value is less than n,
// and, also while the pointer's value does not equal n

while(nodePtr != NULL && nodePtr->value < n
&& nodePtr->value != n)
{
previousNode = nodePtr;
nodePtr = nodePtr->next;
}
// the while loop ends
// if the number is duplicate and the loop did not end at end of
// list
if(nodePtr != NULL)
if(nodePtr->value == n){
cout << "Tape name already exist" << endl;
cout << "Did not add tape" << endl;
return;
}

// if not then insert the newnode
previousNode->next = newnode;
newnode->next = nodePtr;
totalnodes++;
}
}
}


// Delete a node from the linked list
template<class Y>
void linkedlist<Y>::deleteNode(Y &n)
{
Lnode<Y> *nodePtr, *previousnode;

//empty linked list
if(!head)
{
cout << "empty list" << endl;
return;
}

//delete the first node
if(head->value == n){
nodePtr = head->next;
delete head;
head = nodePtr;
totalnodes--;
}
else
{
nodePtr = head;

while(nodePtr != NULL && nodePtr->value != n){
previousnode = nodePtr;
nodePtr = nodePtr->next;
}
// if there is no number in the list that matches
// then the nodePtr points to NULL

if(nodePtr == NULL)
return;

previousnode->next = nodePtr->next;
delete nodePtr;
totalnodes--;
}
}

template<class Y>
void linkedlist<Y>::updateNode(Y &n){
Lnode<Y> * nodePtr;
char choice;

double tmptime;
int tmpyear;
double tmpprice;

nodePtr = head;

if(!head){
cout << "The list is empty" << endl;
}
else{
while(nodePtr != NULL && nodePtr->value != n){
nodePtr = nodePtr->next;
}

if(nodePtr->value == n){
cout << nodePtr->value << endl;
cout << "Update the running time, year realeased, ";
cout << "or price" <<
endl;

cout << "a. Running time" << endl;
cout << "b. Year released" << endl;
cout << "c. Price" << endl;
cin.clear();
cin >> choice;
while(toupper(choice) != 'A' && toupper(choice) != 'B'
&& toupper(choice) != 'C'){

cout << "Invalid choice" << endl;
cout << "Please try again" << endl;
cout << "Update the running time, year realeased, ";
cout << "or price" <<
endl;

cout << "a. Running time" << endl;
cout << "b. Year released" << endl;
cout << "c. Price" << endl;
cin.clear();
cin >> choice;
}
switch(choice){
case 'A':
cout << "Enter the new running time" << endl;
cin >> tmptime;
nodePtr->value.changetime(tmptime);
break;
case 'B':
cout << "Enter the new Year released" << endl;
cin >> tmpyear;
nodePtr->value.changeyear(tmpyear);
break;
case 'C':
cout << "Enter the new Price" << endl;
cin >> tmpprice;
nodePtr->value.changeprice(tmpprice);
break;
}
}
else if(nodePtr == NULL){
cout << "The video tape does not exist in the list";
cout << endl;
}
}
}



template<class Y>
void linkedlist<Y>::displayNode()const {

Lnode<Y> * nodePtr;

nodePtr = head;

// Empty list

if(!head){
cout << "The list is empty" << endl;
return;
}
else{

cout << "Total of video tapes:" << totalnodes << endl;
cout << "Name\t\tRunning Time\tYear Released\tPrice" << endl;

while(nodePtr != NULL){
cout << nodePtr->value;
nodePtr = nodePtr->next;
}
cout << endl;
}
}

template<class Y>
void linkedlist<Y>::searchNode(const Y &n){ // copyright by john cho
Lnode<Y> *nodePtr;

nodePtr = head;

// Empty list

if(!head){
cout << "The list is empty" << endl;
return;
}
else{
while(nodePtr != NULL && nodePtr->value != n){
nodePtr = nodePtr->next;
}

if(nodePtr->value == n)
cout << nodePtr->value;
else{
cout << "There is no such video tape in the list" << endl;
}
}
}




VideoTape::VideoTape(){
time = 0;
year = 0;
price = 0;
}

VideoTape::VideoTape(string n){
name = n;
time = 0;
year = 0;
price = 0;
}

VideoTape::~VideoTape(){
}


istream& operator >>(istream &in, VideoTape &vtape){
cout << "Enter Video Tape name" << endl;
in >> vtape.name;
cout << "Enter Running time" << endl;
in >> vtape.time;
cout << "Enter release year" << endl;
in >> vtape.year;
cout << "Enter price" << endl;
in >> vtape.price;

return in;
}



ostream& operator <<(ostream &out, const VideoTape &vtape2){
out << vtape2.name;
out << "\t\t";
out << vtape2.time;
out << "\t\t";
out << vtape2.year;
out << "\t\t$";
out << vtape2.price;
out << endl;

return out;
}


bool operator < (const VideoTape &a, const VideoTape &b){
return a.name < b.name;
}

bool operator > (const VideoTape &a, const VideoTape &b){
return a.name > b.name;
}

bool operator != (const VideoTape &a, const VideoTape &b){
return a.name != b.name;
}

bool operator == (const VideoTape &a, const VideoTape &b){
return a.name == b.name;
}


void VideoTape::changetime(double t){
time = t;
}

void VideoTape::changeyear(int y){
year = y;
}

void VideoTape::changeprice(double p){
price = p;
}

int main(){

linkedlist<VideoTape> thelist;

VideoTape avideo;
char choice;
string tapename;


do{
cout << "Videotape SYSTEM" << endl;
cout << "a. Add" << endl;
cout << "b. Delete" << endl;
cout << "c. Update" << endl;
cout << "d. Display" << endl;
cout << "e. Search" << endl;
cout << "x. Exit" << endl;
cin >> choice;

while(toupper(choice) != 'A' && toupper(choice) != 'B' &&
toupper(choice) != 'C' && toupper(choice) != 'D' &&
toupper(choice) != 'E' && toupper(choice) != 'X'){

cout << "Invalid entry, Please try again" << endl << endl;
cout << "Videotape SYSTEM" << endl;
cout << "a. Add" << endl;
cout << "b. Delete" << endl;
cout << "c. Update" << endl;
cout << "d. Display" << endl;
cout << "e. Search" << endl;
cout << "x. Exit" << endl;
cin.clear(); // clear multiple letters in the cin stream
cin >> choice;
cout << endl;
}

if(toupper(choice) == 'A'){
cout << "Enter a video tape" << endl;
cin >> avideo;
thelist.addNode(avideo);
}
else if(toupper(choice) == 'B'){
cout << "Enter a video tape name" << endl;
cin.clear(); // clear the istream
cin >> tapename;
thelist.deleteNode(VideoTape(tapename));
}
else if(toupper(choice) == 'C'){
cout << "Enter a video tape name" << endl;
cin.clear(); // clear the istream
cin >> tapename;
thelist.updateNode(VideoTape(tapename));
}
else if(toupper(choice) == 'D')
thelist.displayNode();
else if(toupper(choice) == 'E'){
cout << "Enter a video tape name" << endl;
cin.clear();
cin >> tapename;
thelist.searchNode(VideoTape(tapename));
}

}while(toupper(choice) != 'X');

return 0;

}

re: What'ch think of my program? *fixed word wrap*

John Cho wrote:[color=blue]
> /* This program is a database program that will store
> video tape names, minutes, year released, and price
> I want it to be professional so that *YOU* will want
> to buy it and use it for a video store, j/k about that
> but i want it to be professional */
> // note to leor, display works fine, you must have
> // pressed update[/color]

You are using linked lists, why not use vectors instead? It would be
more crash safe that way and code would be more simpler (=less bugs) and
it would be faster for you to make (=cheaper).
---
Questions like this would be better if they would tell what kind of
format should be used, and for example am I suppose to give time in
seconds, minutes or hours?

Enter Running time
Enter release year
Enter price
---
When adding new tape, it tells me that name is used after I have given
all information. That makes more extra work for me.
---
Program doesn't save tapes, that also needs to be fixed if this is going
to be used in real life.
---
Program crashed when I inserted one video with name "video1" and then
tried to search for it with a name "vid"

Enter Video Tape name
video1
Enter Running time
1
Enter release year
1
Enter price
1

e
Enter a video tape name
vid
Segmentation fault
---


So suggestions: Rewrite your program using vector or something similar
and fix your user interface.

re: What'ch think of my program? *fixed word wrap*

John Cho wrote:
[color=blue]
> /* This program is a database program that will store
> video tape names, minutes, year released, and price
> I want it to be professional so that *YOU* will want
> to buy it and use it for a video store, j/k about that
> but i want it to be professional */[/color]

My first thought is that if you want it to be professional you should
use the standard library instead of implementing your own linked list.

Second thought was that input operators should not do output.

-Kevin
--
My email address is valid, but changes periodically.
To contact me please use the address from a recent posting.

re: What'ch think of my program? *fixed word wrap*

On Mon, 12 Apr 2004 19:35:44 GMT, John Cho <johncho@johncho.us> wrote:
[color=blue]
>// note to leor, display works fine, you must have
>// pressed update
>[/color]
Whoops, so I must've. OK, so let's say you select "update" and then change
your mind. You still don't provide an "escape" mechanism.

This didn't compile with Comeau, BTW, but the reason is subtle (and
evidently the issue is not even diagnosed under certain other compilers):
when you call deleteNote and updateNode, you pass a temporary VideoTape
object as argument; the corresponding parameters are declared as
reference-to-non-const, and that is a no-no. To make it kosher, just add a
"const" in the definitions and declarations for those two functions. Better
yet, change the design so all you pass in situations such as those is the
tapename directly (by const ref, optimally), avoiding the construction of
that temporary VideoTape object for no good reason.

If I have time to look at more of this I might, but that's all I've looked
at for now.
-leor

--
Leor Zolman --- BD Software --- www.bdsoft.com
On-Site Training in C/C++, Java, Perl and Unix
C++ users: Download BD Software's free STL Error Message Decryptor at:
www.bdsoft.com/tools/stlfilt.html