if else statement not working?

[BlackStar]

from the pointer problem thread.
I got this,

void is_valid (char& s)
{
    return s=='1' || s=='2';
}

void is_submenu(char& m)
{
   return m=='x' || m=='y';
}

// then i'm using it in my main.

char choice;
cout<<"enter your choice";
cin>> choice;

if( is_valid(choice)
 {
  if(letter == '1')
  {
     // code for link list is here
        if( one of the pointers is null
         { 
                // pointers points to link list. 
           }
          else
           {     //does something else             }
   }
     else if(letter == '2')
   {
     cout<<"what would you like from the submenu?";
         if(is_submenu(choice))
         {
           if(choice=='x')
          {
              //does something
           }
            else if(choice =='y')
        {
              //does something
          }
             }
                  }

Everything works until, the submenu part, after I hit 2 it works.
Then I put x, or y. Nothing happens.

[William Hemsworth]

I'm surprised it even compiles.

void is_valid (char& s)
{
    return s=='1' || s=='2';
}

void is_submenu(char& m)
{
   return m=='x' || m=='y';
}

Both these functions should be returning bool , not void and there isn't really any need to reference the parameters either.

[mvmalderen]

bool , not void and there isn't really any need to reference the parameters either.

And if you're referencing parameters and you aren't changing the parameters passed by reference it's better to pass them as a constant reference :) ...

To the OP: You forgot several brackets in your code (e.g: if( is_valid(choice) ), did you even try to compile this ?

[ArkM]

Remember: if statement works as ever ;)
It's impossible that is_submenu(choice) is true if is_valid(choice) was true. The choice variable didn't changed between these two condition tests. Probably you fotgot to input a new value of choice...

[BlackStar]

Yes, it compiles, yes, I know I'm missing some of the ) and the }
That's because I didn't copy and paste.
and the void should be bool.
But besides syntax.

Can you guys see what's wrong? Other than the ) and }

[mvmalderen]

To the OP: There are much expression errors (brackets you forget) in your code, try compiling your code and you're compiler will indicate you where you've to add one :) ...

[BlackStar]

Trust me, the program I have in my compiler right now.
Is correct, I already changed the errors already.

But when I hit 2, and then x.
Nothing happens.

[mvmalderen]

Trust me, the program I have in my compiler right now.
Is correct, I already changed the errors already.

But when I hit 2, and then x.
Nothing happens.

Can you please post us a copy of your fixed code (without typos this time) so we can test it and give a suggestion about how to fix this ...

[William Hemsworth]

Trust me, the program I have in my compiler right now. Is correct

Trust me, it's not. Otherwise it would be working.

Post your revised code so we can see what's wrong.

[BlackStar]

#include <iostream>
#include <cstdlib>
#include <string>

using namespace std;

struct node{
       int value;
       node *curr; 
       };
bool is_valid (char& s)
{
    return s=='1' || s=='2';
}

bool is_submenu(char& m)
{
   return m=='x' || m=='y';
}

// then i'm using it in my main.

int main()
{
char choice;
cout<<"enter your choice";
cin>> choice;

if( is_valid(choice) )
 {
  if(choice == '1')
  {
         node *ptr = new node();
         node *ptr2;
         ptr2 = NULL;
         ptr -> value = 1;
         
        if( ptr2 == NULL)
         { 
                ptr2= ptr; 
           }
          else
           {     //does something else             }
           }  
           }
           }
     else if(choice == '2')
   {
     cout<<"what would you like from the submenu?";
         if(is_submenu(choice))
         {
           if(choice=='x')
          {
              cout<<"Hello";//does something
           }
            else if(choice =='y')
        {
              cout<<"what";//does something
          }
             }
                  }
                  
 return 0;
 system("PAUSE");
}

[mvmalderen]

Does the following work ?

bool is_valid (char& s)
{
    if('1' <= s || '2'>= s) return true;
    return false;
}

Edit:: No it doesn't change something :)

[ArkM]

Once again:

{
     cout<<"what would you like from the submenu?";
         if(is_submenu(choice))

Where is an input choice statement after the prompt there???
;)

[ArkM]

Does the following work ?

bool is_valid (char& s)
{
    if('1' <= s || '2'>= s) return true;
    return false;
}

???!
I have no comments...

[mvmalderen]

Please avoid using system("PAUSE"); (here are the reasons), use cin.get(); instead ...

[mvmalderen]

???!
I have no comments...

It's just easier to understand :P

[BlackStar]

Once again:

{
     cout<<"what would you like from the submenu?";
         if(is_submenu(choice))

Where is an input choice statement after the prompt there???
;)

So basically, all I need is an other input variable?

[ArkM]

I don't know what you need. I see evidently wrong code - that's all.
It seems you need another input statement. If not, what for "what would you like from the submenu?" without input?

[BlackStar]

You saying the way I'm going about my program is wrong?

[ArkM]

Better try to run your program with debugger. Watch choice variable value...

[BlackStar]

Programm called debugger? never heard of it, what is it?

[ArkM]

Programm called debugger? never heard of it, what is it?

Congratulations (you and me). I have never seen a programmer who never heard of debuggers.
Well, print choice variable just before if statements...

[BlackStar]

I been doing that :/

[William Hemsworth]

First of all, the order in which you execute your statements are wrong.

return 0;
system("PAUSE");

That second line is never executed as the main function returns before it has a chance to.

Secondly, you shouldn't be using system("PAUSE") in the first place, use cin.get() . But as the input stream still has characters in it, you will have to call it twice to freeze the program before it closes, like this:

cin.get();
cin.get();
return 0;

Then you also have a whole block of code in the wrong place I think, all this can be moved inside the first if code block.

else if ( choice == '2' ) {

  cout<<"what would you like from the submenu?";

  if ( is_submenu(choice) )
  {
    if ( choice == 'x' )
    {
      cout << "Hello"; //does something
    }
    else if ( choice == 'y' )
    {
      cout << "what"; //does something
    }
  }
}

And you have to let the user input the choice variable again.

Here is the corrected code:

#include <iostream>
#include <cstdlib>
#include <string>

using namespace std;

struct node{
  int value;
  node *curr; 
};

bool is_valid (char& s)
{
  return s == '1' || s == '2';
}

bool is_submenu(char& m)
{
  return m == 'x' || m == 'y';
}


int main()
{
  char choice;
  cout << "enter your choice\n";
  cin >> choice;

  if ( is_valid(choice) )
  {
    if ( choice == '1' )
    {
      node *ptr = new node();
      node *ptr2;
      ptr2 = NULL;
      ptr->value = 1;

      if( ptr2 == NULL)
      { 
        ptr2= ptr; 
      }
      else
      {
        //does something else
      }  
    } 
    else if ( choice == '2' ) {

      cout << "what would you like from the submenu?\n";
      cin >> choice;

      if ( is_submenu(choice) )
      {
        if ( choice == 'x' )
        {
          cout << "Hello"; //does something
        }
        else if ( choice == 'y' )
        {
          cout << "what"; //does something
        }
      }
    }
  }

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

Hope this helps.

edit: don't forget to release the memory allocated in ptr,

[BlackStar]

#include <iostream>
#include <cstdlib>
#include <string>

using namespace std;

struct node{
  int value;
  node *curr; 
};

bool is_valid (char& s)
{
  return s == '1' || s == '2';
}

bool is_submenu(char& m)
{
  return m == 'x' || m == 'y';
}


int main()
{
  char choice;
  cout << "enter your choice\n";
  cin >> choice;

  if ( is_valid(choice) )
  {
    if ( choice == '1' )
    {
      node *ptr = new node();
      node *ptr2;
      ptr2 = NULL;
      ptr->value = 1;

      if( ptr2 == NULL)
      { 
        ptr2= ptr; 
        if(ptr2 != NULL)
            {
               string a;
                cout<<endl;
                cin>>a;
                    if (a =="exit")
                    {
                              delete a; 
                               cout<<"hello";
                                   }
      }
      else
      {
        //does something else
      }  
    } 
    else if ( choice == '2' ) {

      cout << "what would you like from the submenu?\n";
      cin >> choice;

      if ( is_submenu(choice) )
      {
        if ( choice == 'x' )
        {
          cout << "Hello"; //does something
        }
        else if ( choice == 'y' )
        {
          cout << "what"; //does something
        }
      }
    }
  }

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

the color coded portion of the code isn't working.

When I hit 1, i should be able to type in exit.
But it closes.

Actually no, when I run it on Dev compiler it works.
But when I run it on another compiler, it doesn't work.
The other compiler is g++

[BlackStar]

Does anyone know why this code would work in one compiler and not in the other?

[mvmalderen]

But as the input stream still has characters in it, you will have to call it twice to freeze the program before it closes, like this:

cin.get();
cin.get();

Or you could use cin.ignore to clean the input buffer, but cin.get is less typing :P !

[BlackStar]

Umm about my compiler problem?

[ArkM]

>the color coded portion of the code isn't working
You can't COMPLILE this portion! There is a terrible error in this portion:

string s;
...
delete s;

1. You are trying to delete automatic variable. It's impossible. You didn't allocate it with operator new!
2. No conversion from std::string to a pointer in C++ library so delete s operation is impossible (fortunately)...

[BlackStar]

I meant to type.

delete ptr.
Would that be a sufficient way to free up memory?

[ArkM]

>Would that be a sufficient way to free up memory?
What's a memory?
Better look at this strange logic:

node *ptr2;
      ptr2 = NULL;
      ptr->value = 1; //*** ptr2 still NULL!!!

      if( ptr2 == NULL) //*** Of course, ptr2 is NULL
        ...
      else //*** Never happened!!!
      {
        //does something else
      }

Try to rewrite this untidy code with a good indentation then post it with correct code tag:
[code=cplusplus] source

[/code]