views:

144

answers:

7

my assignment requires me to write a function that reads in a title and return the corresponding fee to the calling function. if the title is not in the list, return -1.0.

as per my previous question, im a noob on c++, and this is what i have got at the moment:

struct eventType 
{ 
    string title;
    double fees;
}; 

eventType eventTable[10];

int findFees (string newTitle, string newFees)
{
    int Index = 0;
    int flag;
    while (Index < 9) && (eventTable[Index].title != newTitle))
     Index = Index + 1;

    if (eventTable[Index].title == newTitle)
    {
     eventTable[Index].fees = newFees;
     flag = 0;
    }
    else
     flag = -1;

    return flag;
}

is anything missing?

update

after looking at the advice u guys have given, i have adopted and changed the codes to:

double findFees (string title)
{
    for (int Index = 0 ; Index < 10 ; Index++)
    {
        if (eventTable[Index].title == title)
        {
            return eventTable[Index].fees;
        }
    }
    return -1.0;
}

i'm not sure if this is correct either but i do not need a new title or new fees since these values are to be found within eventTable, and return it.

corrections??

A: 

It should be:

while (Index < 10)

And you said it should return the fee, but it returns 0 when found. (This is ok, since you are passing in the fee, why return it too?)

I would also change the signature of the function to be:

int findFees (const string &newTitle, const string &newFees)

and while you are at it, have it return a "bool" instead of a flag to denote success since:

if(findFees(blahTitle, blahFees))

sounds a lot better than:

if(findFees(blahTitle, blahFees) == 0)

when checking for whether the title is found.

Jim Buck
< 10 will cause it to loop off the end of the array (unless the very last one is a match). Of course, it appears wrong because this is not idiomatic C++.
Cade Roux
Ahh, true, I'm not used to seeing a loop written in such an odd way, so I immediately thought (i < 10) as it typical in a loop (and which is in his updated code).
Jim Buck
A: 

It seems to me your function does not return the fees, as you described. It looks like it updates the eventTable, changing the fee stored there, and returns a flag if the update was done successfully.

Please clarify. Do you want to find the fee stored in the eventTable and return it? Or do you want to update the eventTable with new data? Or a hybrid of both.

Still, for a noob, your code is well structured and reasonably well written.

abelenky
i need to find the fees from eventTable and return it.seems like i have included pretty much unnecessaries in my code.
RealiX
+1  A: 

I don't want to give away the answer for you, but here are two things you should keep in mind:

When you have a conditional or a loop, you need to surround statements in { and } so that the compiler knows which statements go inside the loop.

Second, C++ is a type-safe language, meaning that if you are assigning variables to a value of a different type, your program won't compile, look through your code and see if you can find anything wrong on that front

LorenVS
A: 

The only error I see is

while (Index < 9) && (eventTable[Index].title != newTitle))

should probably be:

while ((Index < 10) && (eventTable[Index].title != newTitle))

Note the missing '('. Otherwise you miss matching the last entry in the array.

I would probably write the function something like:

double findFees (const string& title, double newFee)
{
    for (int i = 0; i < 10; i++)
    {
        if (eventTable[i].title == title)
        {
            eventTable[i].fees = newFee;
            return newFee;
        }
    }

    return -1.0;
}

This way you will not iterate through the entire array if you already found the item you where searching for.

bjarkef
A: 

You could simplfy it as so.

int flag = -1;
int Index = 0;

while(Index <= 9)
{
     if(eventTable[Index].title == newTitle)
     {
          eventTable[Index].fees = newFees;
          flag = 0
          break;
     }
}

return flag;
Jaimal Chohan
A: 
eventTable[Index].fees = newFees;

This won't work because fees is a double and newFees is a string. Also you didn't say anything about changing the fees, so I'm confused by that line.

If you want to return the fees of the item you found, you should just put return eventTable[Index].fees; at that point and change the return value of the function to float.

sepp2k
A: 

Your description (return the fee, or -1.0 if not found) does not match your code:

struct eventType 
{ 
    string title;
    double fees;
}; 

eventType eventTable[10];

double findFees (string newTitle)
{
    for (int Index = 0 ; Index < 10 ; Index++)
    {
        if (eventTable[Index].title == newTitle)
        {
            return eventTable[Index].fees;
        }
    }
    return -1.0;
}
Cade Roux