3

I'm trying to do some C++ exercises, but I'm running into a error on build, which doesn't just jump out at me. What am I missing? I'm just getting back to C++ from C# et al after having done it years ago.

[ERROR] syntax error : 'return' [/ERROR]

#include <iostream>
using namespace std;

/* Pre-compiler directives / macros */
#define isValidDrinkChoice(Choice,MaxNumDrinks) ((Choice < MaxNumDrinks) && (Choice > 0))

/* Primary Entry Point for Executable */
int main(const int & argc, char * argv[]){

    const int MaxNumDrinks = 4;
    char ** Drinks;
    Drinks = new char* [MaxNumDrinks];
    Drinks[0] = "Soda";
    Drinks[1] = "Water";
    Drinks[2] = "Coffee";
    Drinks[3] = "Tea";
    Drinks[4] = "Perrier Sparkling Water";

    int Choice = -1;
    do while(!isValidDrinkChoice(Choice, MaxNumDrinks)) {
        cout << "Please select your favorite drink\r\n\r\n" << endl;
        for (int x = 0; x < MaxNumDrinks; x++) cout << "\t" << Drinks[x] << endl;
        cin >> Choice;
        if (isValidDrinkChoice(Choice, MaxNumDrinks)) cout << "\r\n\r\n" << "You chose " << *Drinks[Choice] << endl;
    }
    return 0;
}
bitcycle
  • 7,632
  • 16
  • 70
  • 121
  • 1
    String literals are `const char *` so you shouldn't be assigning them to a `char *` container, and you're leaking the `char **Drinks` memory. Might as well use `vector Drinks; Drinks.push_back("Soda"); // etc.` instead, and this will let you drop the hard-coded `MaxNumDrinks` as well. (Which is buggy, by the way: you have 5 drinks, not 4.) – ephemient Feb 27 '10 at 07:11
  • 1
    Not connected to the compilation error, but do yourself a favour and lose the macro. Reimplement `isValidDrinkChoice` as a simple function - there is no reason it should be a macro an plenty of reasons why it shouldn't. – Timo Geusch Feb 27 '10 at 07:41
  • In addition: `s/(Choice > 0)/(Choice >= 0)/`, `s/& argc/argc/`, `s/*Drinks[Choice]/Drinks[Choice]/` – jfs Feb 27 '10 at 08:37
  • Thanks for the helpful comments. I'll be sure to make those changes. – bitcycle Feb 27 '10 at 16:00

2 Answers2

6

I don't think there's a do while like that in C++. It's do { ... } while (expression);. Or while (expression) { ... }.

Plynx
  • 11,341
  • 3
  • 32
  • 33
3

The Corrected code sample its the while loop that needs replacement

#include <iostream>

namespace {
  bool isValidDrinkChoice(int Choice, int MaxNumDrinks) {
    return ((Choice < MaxNumDrinks) && (Choice >= 0));
  }
}

/* Primary Entry Point for Executable */
int main() {
    using namespace std;   

    const char *Drinks[] = {
      "Soda", "Water", "Coffee", "Tea", "Perrier Sparkling Water" };
    const int MaxNumDrinks = sizeof(Drinks) / sizeof(*Drinks);

    int Choice = -1;
    do  {
        cout << "Please select your favorite drink\r\n\r\n" << endl;
        for (int i = 0; i < MaxNumDrinks; i++) 
          cout << Drinks[i] << endl;

        cin >> Choice;

        if (isValidDrinkChoice(Choice, MaxNumDrinks)) 
          cout << "\r\n\r\n" << "You chose " << Drinks[Choice] << endl;
    } while(!isValidDrinkChoice(Choice, MaxNumDrinks) && cin) ;

    return cin.good() ? 0 : 1;
}
jfs
  • 399,953
  • 195
  • 994
  • 1,670
anijhaw
  • 8,954
  • 7
  • 35
  • 36
  • Hmm.... 3 questions: 1.) why am I returning a value based on whether or not cin.good() returns true or false? 2.) Why the use of the namespace wrapper around the isValidDrinkChoice() function? 3.) Why "&& cin" in the while condition? – bitcycle Feb 27 '10 at 16:03
  • @Sean Ochoa: if `cin` is in invalid state when the loop is infinite without '`&& cin`' condition. `main()` should return `0` on success, nonzero otherwise. The presence of unintentional infinite loops or reporting success (`return 0`) on failure I consider as bugs that should be fixed. – jfs Feb 27 '10 at 17:33
  • @Sean Ochoa: on the namespace wrapper http://stackoverflow.com/questions/154469/unnamed-anonymous-namespaces-vs-static-functions – jfs Feb 27 '10 at 17:36