First time posting here - need advice

Well, I have been scouring all the tutorials I can find teaching myself C++ lately, and have done a few rather embarassing things in the process....

Now I find that using 'goto's in c++ is a really, really, bad no-no. I have a few in this rather complicated program I am writing at, but am not sure how to eliminate them.....

What I am asking for is any good resources where I can learn how to eliminate things like the following: (Just a sample program I was goofing with to figure out a part of the larger one)

Code:

#include <ncurses.h>
int main (void)
{
  char aa[20];
  int bb = 0;
  initscr();
  cbreak();
  noecho();
  loopud1:
  mvprintw(7,23,"Please enter the number."); //Repeated only on success
  refresh();
  loopud1a:
  aa[bb] = mvgetch(8,38+bb);  //This will repeated on any error
  mvprintw(8,38+bb,"%c",aa[bb]);
  if(   (aa[bb] < 0x30) || (aa[bb] > 0x39))
    {
      mvprintw(7,23,"Please enter only numbers.");
      clrtoeol();
      refresh();
      goto loopud1a;;
    }
  bb++;
  refresh();
  goto loopud1;
}

My biggest problem is getting out of the 'goto' frame of mind, so anything you can point me to that would help is greatly appreciated (Don't post fixed versions of this code, just references please)

Thanks,
David

Instead of using goto, use a loop. Find a place where you use goto to move back up the code. The goto will be replaced with the end of the loop (which is just a closing bracket - }). The label is where the top of the loop goes. In this case you start with a while loop with no conditions - while (true) {. Then just indent everything in between the brackets one level deeper, since that is the code that will repeat.

If your goto is inside of a condition, then use that condition to break out of the loop. For example, your loopud1 label has no condition, it just runs forever, so you can just replace it with while (true). However, your loopud1a only loops if the user enters invalid numbers, so you'd have to work a little harder to use that as the condition for your loop.

In other code, a for loop might be appropriate, but you should learn about these loops first so you can figure out which is the most appropriate for your task.

Thanks for the response. After reading it I went straight to that bit of code and did a modest amount of reworking. It should be easy to tell where my modifications are. Do you think I went about it in the right way? Or should I have considered another alternative. Made the after-compile object 48 bytes bigger, but if it makes for sound coding, its worth it I guess.

Code:

#include <ncurses.h>
int main (void)
{
  char aa[20];
  int bb = 0;
  initscr();
  cbreak();
  noecho();
  mvprintw(7,23,"Please enter the number.");
  while (1) // infinite while
  {
    int cc = 1; // new variable
    refresh();
    aa[bb] = mvgetch(8,38+bb);
    mvprintw(8,38+bb,"%c",aa[bb]);
    if( (aa[bb] < 0x30) || (aa[bb] > 0x39) )
    {
      mvprintw(7,23,"Please enter only numbers.");
      clrtoeol();
      refresh();
      cc = 0;
    }
    if( cc != 0 )
    {
      bb++;
      mvprintw(7,23,"Please enter the number.");
      clrtoeol();
    }
  }
}

I'd say that's fine. It's definitely a move in the right direction. Unless you have some reason to worry about code size, I'd worry more about other design issues that will make it easier to maintain, debug, and extend the code you write.

Welcome, DerelictDream!

Program structure, control, & flow...

1 - FUNCTIONS
Write your code in functions. When you call your function , this is something like 'goto the function'. (Really more like GOSUB, if you know BASIC.) Functions are the key to structuring your program. If you think you need a goto (and it's not a loop), you probably need a function-call.

2 - CONDITIONAL BRANCHING
You use if-statements and switch-case statements to control program flow. Use the if-statement to call a function, NOT to execute a goto!

3 - LOOPS
There are 3 types of loops in C++: for-loops, while-loops, and do-while-loops.

Code:

char aa[20];
int bb = 0;
...
int cc = 1;

You might want to consider using more descriptive names

Code:

if( cc != 0 )

Since you're only using cc as a sort of signal variable, you might as well declare it as a bool and set it to true or false.

Good job converting from goto to loops

I use if .. else break to set this loop:

Code:

.........
while (1)
  {   aa[bb] = mvgetch(8,38+bb);  //This will repeated on any error
      mvprintw(8,38+bb,"%c",aa[bb]);
      if((aa&#091;bb&#093; < 0x30) || (aa&#091;bb&#093; > 0x39))
        {
          mvprintw(7,23,"Please enter only numbers.");
          clrtoeol();
          refresh();
         }
       else break;
   } 
...........

Originally Posted by Hunter2

You might want to consider using more descriptive names

In my larger program I do use descriptive names...

Originally Posted by toysoldier

I use if .. else break to set this loop:

Code:

.........
while (1)
  {   aa[bb] = mvgetch(8,38+bb);  //This will repeated on any error
      mvprintw(8,38+bb,"%c",aa[bb]);
      if((aa[bb] < 0x30) || (aa[bb] > 0x39))
        {
          mvprintw(7,23,"Please enter only numbers.");
          clrtoeol();
          refresh();
         }
       else break;
   } 
...........

If I, and my compiler, are not mistaken, the "else break" will end the while loop on success causing the program to end. Correct me if that is wrong.

Originally Posted by DougDbug

Welcome, DerelictDream!

Program structure, control, & flow...

Thanks for the warm welcome! And thanks for the advice. While I know what Functions, Loops, and Conditional Statements are and how to use them... I guess my biggest problem is learning when to use them. jlou, you have been most helpful with this, and I appreciate it.

it's rather simple...you want to use functions to solve a problem and one problem only...you want to use loops to HELP solve your problem....although there is already a function that resolves exponents, i'll re-invent the wheel to demonstrate how when to use these 3 structures...this function excepts 2 integer parameters and returns one integer...the first parameter is the mantissa and the second parameter is the exponent.

NOTE: if you actually run this 'program' (which i have not ran myself), don't use very large numbers, as you may know, exponential numbers can get large very quickly and variables of type 'int' can only store a value not much larger than 32,000

Code:

int resolveExponent(int mantissa, int exponent)
{
        int solution;                            //the return variable
      
       if(exponent == 0)                    //any number to the power of zero is 1 (i think)
                  return 1;
       if(exponent == 1)                  //any number to the power of 1 is itself (again, i think)
                  return mantissa; 

      //now we use a loop to repeat the multiplication 
     //because a number to the power of x is the number times itself x times
      //i'm sure you know this, but 3 to the power of 3 = 3 * 3 * 3
      //but you don't want to type 3*3*3 everytime you want the number 3 to the 3rd
   
      //this is a for loop... the first part says initialize i to zero..this only happens once 
      //the entire loop. the second part says perform the action within this loop until
      // i meets the condition "i is less than or equal to exponent"...the third part says 
      // increment i (i++ is the same as i = i + 1)...the second and third part occur every pass
     //through this loop
      for(int i = 0; i <= exponent; i++)
     {
             solution = solution * mantissa;
     }
      
     return solution;
}


int main()
{
   //now  instead of doing this (below) everytime when want the exponential value
   //of a number
   /*  int mantissa = 3;
     int exponent = 3;
      for(int i = 0; i <= exponent; i++)
     {
             solution = solution * mantissa;
     }*/

    //we can simply do this
    int x = resolveExponent(3, 3);
  
   //then, maybe we wanted to do this
    x = resolveExponent(x, x); 
  
    return 0;
}

and again without the over commenting

Code:

int resolveExponent(int mantissa, int exponent)
{
        int solution;             
      
       if(exponent == 0)     
                  return 1;
       if(exponent == 1)               
                  return mantissa; 

      for(int i = 0; i <= exponent; i++)
     {
             solution = solution * mantissa;
     }
      
     return solution;
}


int main()
{
    int x = resolveExponent(3, 3);
    x = resolveExponent(x, x); 
  
    return 0;
}

i hope this helps you....
remember though, you only want your functions to solve one problem and one problem only...if the function needs to solve a sub problem then you should probably split that into another function (or use recursion, but don't worry about recursion yet, it's rather confusing)...

Originally Posted by misplaced

remember though, you only want your functions to solve one problem and one problem only...

Well there's something I had assumed but did not know for sure......Thanks for clearing that up!

My larger program has a main which consists of nothing but screen initialization and calls to functions....a couple of which call other functions...

>>Correct me if that is wrong.
You're right.

And, it may be worthwhile to note, many people consider break and continue to be nearly as bad as goto. I personally like them though, and I'll leave it up to you to form your own opinion of them (you can do some board searches on break).