Solved

global string array looses content when calling a new function

Posted on 2011-02-27
32
267 Views
Last Modified: 2012-05-11
I have the following global definitions:

const int MAXCOUNT = 12;

string SysBuf[MAXCOUNT];


When I call the function OpenAndReadFile the SysBuf is filled with content from a disk file.
This works OK.

When I return to the calling function I'm able to print out the content of SysBuf. But when I do a new call to another function (SetupStatus) and then return to same function I'm no longer able to print out the content of SysBuf. (?????).  The function SetupStatus does not alter any value of SysBuf, it is not even referenced in that function.

Anyone ?

const int MAXCOUNT = 12;
string SysBuf[MAXCOUNT];
..
..
..
void OpenAndReadFile(char *cp)
int counter;
string line;
line = cp;
const char *cpp = line.c_str();

ifstream myfile(ccp);

if (myfile.is_open()
{
 for (counter=0;(counter<MAXCOUNT);counter++)
  {
   SysBuf[counter] = line;
   getline(myfile,line);
  }
   myfile.close();
}
 else cout << "Unable to open file";
return;
}

..
..
void CallingFunction()
{
 char *mystr;
 
 mystr = "Path and filename";

 OpenAndReadFile(mystr);    

// Writing out the content of SysBuf. //
// The output seems OK. //


 SetUpStatus();

// Writing out the content of SysBuf. //  
// No output!!! //

}

Open in new window

0
Comment
Question by:toyboy61
  • 13
  • 11
  • 7
32 Comments
 
LVL 53

Expert Comment

by:Infinity08
Comment Utility
>> // Writing out the content of SysBuf. //

How do you write out the content of SysBuf ?


>> void OpenAndReadFile(char *cp)

I assume that the missing { here is a copy-paste error ?


>>  SetUpStatus();

What's the implementation of this function ?
0
 
LVL 16

Expert Comment

by:sjklein42
Comment Utility
You are doing some crazy stuff.  :)  Follow this:

You set "mystr", a character string pointer, to the read-only string "Path and Filename".  (OK)
You pass "mystr" into OpenAndReadFile (OK)
Inside OpenAndReadFile, "cp" get mystr, which is a pointer to that read-only string. (OK)
You set "line" equal to cp, so "line" now points to that read-only string (starting to get suspicious)
You then use "line" as the target buffer for getline (whoops! it isn't a buffer, is it?  It is a pointer to our read-only string!)

So you are using the (supposed-to-be) read-only memory the compiler allocated for the "Path and Filename" literal string as your input buffer!


Fix that confusion first and then we can see if there are more issues.
0
 
LVL 53

Expert Comment

by:Infinity08
Comment Utility
>> You set "line" equal to cp, so "line" now points to that read-only string (starting to get suspicious)

line doesn't point to the read-only string, it contains a copy of the read-only string (remember that line is a std::string, not a char*).
0
 
LVL 16

Expert Comment

by:sjklein42
Comment Utility
Good point.

Still scrutinizing.
0
 
LVL 16

Assisted Solution

by:sjklein42
sjklein42 earned 100 total points
Comment Utility
Your SetUpStatus routine must be trashing the global variable memory pool.  Can we take a look at that?

Even though it isn't supposed to be touching the SysBuf array, SetUpStatus must be overstepping the bounds of one of the other global arrays in your program that happens to be allocated adjacent to SysBuf.
0
 

Author Comment

by:toyboy61
Comment Utility
Infinity08:  I'm a newbie when it comes to C++-programming, so I might have done some silly errors...
Of course there should be a "{" after the "void OpenAndReadFile(char *cp)" definition...

This is how I write out the content of SysBuf:

 for (counter=0;(counter<MAXCOUNT);counter++)
   cout << SysBuf[counter] << "\n";

sjklein42: Well... It might not be a very good C++ -code, but it does work....
(And it's mainly copy-and-paste from a tutorial I found on the Internet..).
I'm able to fill SysBuf with the content of the file I'm reading from.. :-)

void SetupStatus()
{
  int counter, Choice1;

// Checking to see if SysBuf survived the call.. :-)
 
  for (counter=0;(counter<MAXCOUNT);counter++)
   cout << SysBuf[counter] << "\n";
 
cout << "Menu" << "\n";
cout << " 1. Choice 1. " << "\n";
cout << " 2. Choice 2. " << "\n";
cout << " 3. Choice 3. " << "\n";

cout << "Make your choice: "
cin >> Choice1;

// Some logic to select which menu choice has been chosen. //

}

But the problem is still: Why does not the content of SysBuf survives after the second function call ?









 




0
 
LVL 53

Expert Comment

by:Infinity08
Comment Utility
Ok, that doesn't shed any extra light on the situation. Would it be possible to post the complete code here (ie. everything, so we can try to reproduce your problem) ?
0
 

Author Comment

by:toyboy61
Comment Utility
Infinity08: My code is written with a lot of Norwegian comments, variable names and so on. It might not be very easy to understand for English-speaking people, and I do not have the time to translate it to English. But I'll investigate further on using the advice from sjklein42..
0
 
LVL 53

Expert Comment

by:Infinity08
Comment Utility
I see you accepted http:#34992991 as the solution ... what was the problem then ?
0
 
LVL 53

Expert Comment

by:Infinity08
Comment Utility
>> It might not be very easy to understand for English-speaking people, and I do not have the time to translate it to English.

Heh ... I don't need comments. The code speaks the truth ... comments can lie ;)


>> But I'll investigate further on using the advice from sjklein42..

Well, you asked this question to get our help, and we can still provide it.

In any case, you shouldn't accept a post as the solution, unless you have verified that it is actually the solution.

If you no longer wish to continue with this question, you can delete it, or otherwise, you can re-open it, and we can continue working on this.
0
 

Author Comment

by:toyboy61
Comment Utility
I've asked Moderator to delete the whole question. If I do not find out a solution on my own I will post the whole program-code and ask for help..  :-)
(Right now the program code is uncomplete and not suitable for anyone other than myself to read.. :-) ).
0
 
LVL 16

Expert Comment

by:sjklein42
Comment Utility
I see no reason to delete the question.  I gave a good answer.  Now he knows what to look for.  What's the big deal?
0
 
LVL 53

Expert Comment

by:Infinity08
Comment Utility
>> I gave a good answer.

What you posted, is only one out of many possible causes for this problem. As long as it hasn't been verified, and as long as toyboy61's problem isn't solved, this question isn't answered :)

toyboy61 can re-open this question, and keep it open until he has either found the issue, or wants to post all the code (I really don't mind that the code is not all English or is a work in progress). Alternatively, deleting the question is an option, although I'd rather prefer continuing to provide assistance.
0
 

Author Comment

by:toyboy61
Comment Utility
Infinity08 and sjklein42:  I have rewritten parts of the code, and I'm posting the whole program-code as attachment.

The primary problem is now solved, but I have got another in function SettOppStamme:
   
   if ((Dekning == "HUB") && (Hele == 0))
   
   What I'm trying to do is:
      If "Dekning" has the value "HUB" and "Hele" has the value 0 (zero) then the test should be true.

   if ((Dekning == "UB" || "HU" || "HB") && (Halve == 0))
      If "Dekning" has the value "UB" or "HU" or "HB" and "Halve" has the value 0 (zero) the test    should be true.  

   if ((Dekning == "H" || "U" || "B") && (Sikre == 0))
     If "Dekning" has the value "H" or "U" or "B" and "Sikre" has the value 0 (zero) the test should be true.

Anyone who can help a newbie with this?
The code does not work as intended by now... :-(


What I'm trying to is:
   If the variable Dekning contains












#include <cstdlib>
#include <stdio.h>
#include <iostream>
#include <string>
#include <dirent.h>
#include <cstdio>
#include <fstream>
#include <ios>
#include <cstring>

using namespace std;

// ==================================== 
// == Definisjon av diverse konstanter og globale variable. == 
// ==================================== 

const char *PROGVER = "0.2beta";
const int MAXSYS = 10;
const char *SYSTEMDIR = "/home/tomaurlund/Programutvikling/systemer/";
const int MAXKAMPER = 12;

char * buffer;

int Hele, Halve, Sikre, Rekker;

string Kamp[MAXKAMPER+1];
string SystemNavn[MAXSYS];
string SysBuf[MAXKAMPER]; 
string SysNavn;    

int Flagg;

// ====================
// == Functions and subroutines ==
// ====================

// ===============
void NullstillStamme()
// ===============

/*
  This routine resets the content of Kamp-array.
*/  

{
  int teller;
  
  for (teller=1;teller<13;teller++)
   { 
     Kamp[teller] = " ";
    } 
}

// ========================
void OpenAndReadFile(char *cp)
// ========================
{
/*
  This routine opens the file chosen in routine VelgSystem (ChooseSystem),
  read its contents and put the main content in the SysBuf-buffer.
*/  

int Lengde, kamper, teller;
string line, Buffer1;
line = cp;
const char *ccp = line.c_str();

Flagg = 0;

/* Programutførelse starter her. */

ifstream myfile (ccp);

 if (myfile.is_open())
 {
//   while (!myfile.eof())
//    {
/*  ====================    
     Reading initialization parameters
     ====================
*/              
  
          if (Flagg == 0) 
           {
                
              getline(myfile,line); 
              Hele = atoi(ccp);
           
              getline(myfile,line);
              Halve = atoi(ccp);
        
              getline(myfile,line);
              Sikre = atoi(ccp);
        
              getline(myfile,line);
              Rekker = atoi(ccp);
                
/* ============== 
    Reading comment lines 
    ==============
*/                   
              line = "//";
              
              while(line.find("//") != string::npos)
               getline(myfile,line);
              
/* ================= */
/* Reading system information  */
/* ================= */
              
               for (teller=0;(teller<(Hele+Halve));teller++)
                {
                  SysBuf[teller] = line;
                  getline(myfile,line);
                 }       
                Flagg = 1;
           }         
              myfile.close();
   }   

          else cout << "Unable to open file";  
          
return;
}

/* ======================================= */ 
int ListKatalogFiler()
/* ======================================= */
/*
  This routine lists the files under the directory given by the constant SYSTEMDIR.
*/  

{
   DIR *dirp;
   int counter;
   struct dirent *entry;
   
   counter = 0;
   
   if(dirp = opendir(SYSTEMDIR))

   {

    while(entry = readdir(dirp))

      if (entry->d_type == 8)       // file namespace
       {
         SystemNavn[counter] = entry->d_name;
         counter++;
        }

    closedir(dirp);

    }     

    return counter;

}
  
/* =====================================  */
void VelgSystem()
/* =====================================  */
/*
  This routine takes the result from ListKatalogFiler (ListCatalogueFiles)
  displays it on the screen and let the user choose which file to use.
*/  

{

 int SysValg, status, counter;
 char *cp;

 status = ListKatalogFiler(); 

   counter =  0;
   cout << "\nSYSTEMFILER TILGJENGELIGE:\n";
   
   while (counter < status) 
    {   
      cout << "Fil nr. " << counter+1 << ":" << SystemNavn[counter] << "\n";
      counter++;
     }  
      cout << "\n\n";
      
     cout << "Velg system:";
     cin >> SysValg;
     
   SysNavn = SystemNavn[SysValg-1];
     
  return; 
}
/* ======================================= */
void KampMeny()
/* ======================================= */
/*
  This routine displays the KampMeny on the screen with the content
  of the Kamp-array for each of the matches.  (Kamp = match/competiton).
*/  
{
   cout << "\n";
   cout << "======================" << "\n";
   cout << "Kamp nr. 01: " << Kamp[1] << "\n";
   cout << "Kamp nr. 02: " << Kamp[2] << "\n";
   cout << "Kamp nr. 03: " << Kamp[3] << "\n";
   cout << "======================" << "\n";
   cout << "Kamp nr. 04: " << Kamp[4] << "\n";
   cout << "Kamp nr. 05: " << Kamp[5] << "\n";
   cout << "Kamp nr. 06: " << Kamp[6] << "\n";
   cout << "======================" << "\n";
   cout << "Kamp nr. 07: " << Kamp[7] << "\n";
   cout << "Kamp nr. 08: " << Kamp[8] << "\n";
   cout << "Kamp nr. 09: " << Kamp[9] << "\n";
   cout << "======================" << "\n";
   cout << "Kamp nr. 10: " << Kamp[10] << "\n";
   cout << "Kamp nr. 11: " << Kamp[11] << "\n";
   cout << "Kamp nr. 12: " << Kamp[12] << "\n";
   cout << "======================" << "\n";
   cout << "\n";
}   
   

/* =========================================== */
void SettOppStamme()
/* =========================================== */ 
/*
 This routine filles the Kamp-array with the correct values.
*/ 

{

 char Dekning[3];
 int Kampnr, teller;

#ifdef SYSTEMBUFFER 
 cout << "Utskrift av SysBuf etter at SettOppStamme er valgt:\n"; 
 
  for (teller=0;(teller<(Hele+Halve));teller++)
    cout << SysBuf[teller] << "\n"; 
#endif 
 
 cout << "\n";
 cout << "Stamme settes opp for ";
 cout <<  SysNavn;
 cout << "\n";
 cout << " Antall helgarderte/varierte:   " << Hele << "\n";
 cout << " Antall halvgarderte/varierte: " << Halve << "\n";
 cout << " Antall sikre:" << Sikre << "\n";
 cout << " Antall rekker: " << Rekker << "\n";
 cout << "\n";

#ifdef SYSTEMBUFFER
 cout << "Utskrift av SysBuf etter utskrift av systeminfo:\n";
  for (teller=0;(teller<(Hele+Halve));teller++)
    cout << SysBuf[teller] << "\n"; 
#endif

 KampMeny();
 
#ifdef SYSTEMBUFFER 
 cout << "Utskrift av SysBuf etter retur fra rutine KampMeny():\n";  
  for (teller=0;(teller<(Hele+Halve));teller++)
    cout << SysBuf[teller] << "\n"; 
#endif 
 
  for (Kampnr=1;(Kampnr < 13);Kampnr++)
  {
    start:
     cout << "Kampnr = " << Kampnr << "\n";
     cout << "\n Stammevalg:\n"; 
     cout << "HUB, HU, UB, HB, H, U, B ->";
     cin >> Dekning;
    
     cout << "\nDekning = " << Dekning << "\n";
     cout << "\nHele = " << Hele << "\n";
    
    if ((Dekning == "HUB") && (Hele == 0))
      {
        cout << "\nIkke flere helgarderte/varierte tilgjengelig!!";
        cout << "\nPrøv igjen!";
        goto start;
       }
      
    if ((Dekning == "UB" || "HU" || "HB") && (Halve == 0))
      {
        cout << "\nIkke flere halvgarderte/varierte tilgjengelig!!";
        cout << "\nPrøv igjen!";
        goto start;
       }
     if ((Dekning == "H" || "U" || "B") && (Sikre == 0))
       {
        cout << "\nIkke flere sikre tilgjengelig!!";
        cout << "\nPrøv igjen!";
        goto start;    
        }
    
    string Stm( Dekning );
    Kamp[Kampnr] = Stm;
 
    if (Stm == "HUB") Hele--;
    if (Stm == "HU") Halve--;
    if (Stm == "UB") Halve--;
    if (Stm == "HB") Halve--;
    if (Stm == "H") Sikre--;
    if (Stm == "U") Sikre--;
    if (Stm == "B") Sikre--;
    
    cout << "\n";
    cout << "Antall hele: " << Hele << "\n";
    cout << "Antall halve:" << Halve << "\n";
    cout << "Antall sikre:" << Sikre << "\n";
   } 
   
   KampMeny();
        
     if (Kamp[Kampnr].find_first_of ("HUB") !=  string::npos)
      {
        if (Kamp[Kampnr] == "HUB") Hele++;
        if (Kamp[Kampnr] == "HU") Halve++;
        if (Kamp[Kampnr] == "UB") Halve++;
        if (Kamp[Kampnr] == "HB") Halve++;
        if (Kamp[Kampnr] == "H") Sikre++;
        if (Kamp[Kampnr] == "U") Sikre++;
        if (Kamp[Kampnr] == "B") Sikre++;
       }
  cout << "\n";    
}

/* =========================================== */
void GenererSystem()
/* =========================================== */
/*
   This routine transforms the information in the Kamp-array based on the
   information read from file and generates a new content which should begins
   written back to file.

   Routine has to be completed later on.
*/   
{
 
 int teller;
 
 cout << "\n\n\n";
 cout << "\n";
 cout << "Systemgenerering pågår for ";
 cout << SysNavn;
 cout << "\n";
 KampMeny();
 cout << "\n";
 
 cout << "\n" << "Skriver ut innholdet av SysBuf: " << "\n"; 
 
  for (teller=0;(teller<(Hele+Halve));teller++)
    cout << SysBuf[teller] << "\n";
  
   cout << "\n";
   
// ============================================== 
// == Her skal vi sette opp systemet basert på nøytral nøkkel og stammen. ==
// ============================================== 
 
// More code to be added later // 
 
} 

/* =========================================== */
void screenmenu()
/* =========================================== */

{
  cout << "\n";
  cout << "=================================\n";
  cout << "== SYSTEMTIPP versjon " << PROGVER << " ==\n";
  cout << "=================================\n";
  cout << " 1. Velg system.\n";
  cout << " 2. Sett opp stamme.\n";
  cout << " 3. Generer system.\n";
  cout << " 4. Nullstill stamme.\n";
  cout << " 0. Avslutt program. \n";
  cout << "=================================\n";
  cout << "Menyvalg:";
 }
 
/* =========================================== */ 
void Valg(int valg)
/* =========================================== */
{

 const char *cp = SysNavn.c_str();
 
 char *mystr;
 int i, teller;
 string Line;

  if (valg ==1) 
  {
   VelgSystem();
   cout << "\n\nValgt system er ";
   cout << SysNavn;
   cout << "\n\n";
   screenmenu();
  }
  else if (valg ==2)
  {

// Størrelsen på char-variablen bestemmes her.   
   mystr = new char[strlen(SYSTEMDIR)+strlen(cp)+1];
   
// Deretter må variabelen fylles med innhold.. :-)
   strcpy(mystr,SYSTEMDIR);   // Overskriver eventuelt gammelt innhold i mystr.
   strcat(mystr,cp);
   
// Neste steg er å kalle funksjonen OpenAndReadFile med parameter mystr.
// SysBuf inneholder systeminformasjon.
     
   OpenAndReadFile(mystr);
   
#ifdef SYSTEMBUFFER   
  cout << "\n Utskrift av SysBuf etter utførelse av rutine OpenAndReadFile():\n"; 
   
  for (teller=0;(teller<(Hele+Halve));teller++)
    cout << SysBuf[teller] << "\n";
#endif
   
   SettOppStamme();

#ifdef SYSTEMBUFFER   
  cout << "\n Utskrift av SysBuf etter utførelse av rutine SettOppStamme():\n"; 
   
  for (teller=0;(teller<(Hele+Halve));teller++)
    cout << SysBuf[teller] << "\n"; 
#endif    
   
   screenmenu();
   }
  else if (valg == 3)
   {
   GenererSystem();
   screenmenu();
   }
  else if (valg == 4)
   {
   NullstillStamme();
   screenmenu();
   } 
  else
   { 
   cout << "Ugyldig menyvalg!!\n\n";
   screenmenu();
   }
   return;
 }  

 // -------------------------------------
 //  Main function begins here.
 // -------------------------------------

 
 int main (){

 int menyvalg;
 
 screenmenu();
 cin >> menyvalg;
 
 while (menyvalg > 0)
 {
    Valg(menyvalg);
    cin >> menyvalg;
 }
 cout << "\n";
 return 0;
 }

Open in new window

0
 
LVL 16

Assisted Solution

by:sjklein42
sjklein42 earned 100 total points
Comment Utility
Hi.

if ((Dekning == "UB" || "HU" || "HB") && (Halve == 0))

should be:

 
  if (((Dekning == "UB") || (Dekning == "HU") || (Dekning == "HB")) && (Halve == 0))

Open in new window



Do the same fix to this line:

     if ((Dekning == "H" || "U" || "B") && (Sikre == 0))

0
Why You Should Analyze Threat Actor TTPs

After years of analyzing threat actor behavior, it’s become clear that at any given time there are specific tactics, techniques, and procedures (TTPs) that are particularly prevalent. By analyzing and understanding these TTPs, you can dramatically enhance your security program.

 

Author Comment

by:toyboy61
Comment Utility
sjklein42: In my program I have three variables called Hele, Halve and Sikre (Full, Half and Secure).
Even if the program counts down each of these (according to the input of "HUB", "HU" and so on) the tests never ends up as true, and the code following the tests are never run. I can't figure out why.

for (Kampnr=1;(Kampnr < 13);Kampnr++)
  {
    start:
     cout << "Kampnr = " << Kampnr << "\n";
     cout << "\n Stammevalg:\n"; 
     cout << "HUB, HU, UB, HB, H, U, B ->";
     cin >> Dekning;
    
     cout << "\nDekning = " << Dekning << "\n";
     cout << "\nHele = " << Hele << "\n";
    
    if ((Dekning == "HUB") && (Hele == 0))
      {
        cout << "\nIkke flere helgarderte/varierte tilgjengelig!!";
        cout << "\nPrøv igjen!";
        goto start;
       }
      
   if (((Dekning == "UB") || (Dekning == "HU") || (Dekning == "HB")) && (Halve == 0))
      {
        cout << "\nIkke flere halvgarderte/varierte tilgjengelig!!";
        cout << "\nPrøv igjen!";
        goto start;
       }
     if (((Dekning == "H") || (Dekning == "U") || (Dekning == "B")) && (Sikre == 0))
       {
        cout << "\nIkke flere sikre tilgjengelig!!";
        cout << "\nPrøv igjen!";
        goto start;    
        }
    
    string Stm( Dekning );
    Kamp[Kampnr] = Stm;
 
    if (Stm == "HUB") Hele--;
    if (Stm == "HU") Halve--;
    if (Stm == "UB") Halve--;
    if (Stm == "HB") Halve--;
    if (Stm == "H") Sikre--;
    if (Stm == "U") Sikre--;
    if (Stm == "B") Sikre--;
    
    cout << "\n";
    cout << "Antall hele: " << Hele << "\n";
    cout << "Antall halve:" << Halve << "\n";
    cout << "Antall sikre:" << Sikre << "\n";
   }

Open in new window

0
 
LVL 53

Expert Comment

by:Infinity08
Comment Utility
>>    if ((Dekning == "UB" || "HU" || "HB") && (Halve == 0))

You cannot compare a char* with the == operator.

There are two ways :

(a) make Dekning a std::string, so you CAN use == to compare them

(b) use strcpy (or strncpy) to compare char*'s :

        http://www.cplusplus.com/reference/clibrary/cstring/strcpy/
0
 
LVL 16

Expert Comment

by:sjklein42
Comment Utility
Looking.

Have you confirmed that these variables are being initialized with values as you expect?

              getline(myfile,line); 
              Hele = atoi(ccp);
           
              getline(myfile,line);
              Halve = atoi(ccp);
        
              getline(myfile,line);
              Sikre = atoi(ccp);
        
              getline(myfile,line);
              Rekker = atoi(ccp);

Open in new window


What output are you seeing from these lines?

    cout << "Antall hele: " << Hele << "\n";
    cout << "Antall halve:" << Halve << "\n";
    cout << "Antall sikre:" << Sikre << "\n";

Open in new window

0
 

Author Comment

by:toyboy61
Comment Utility
sjklein42: Yes, I have confirmed that the variables Hele, Halve and Sikre have got the correct values when being read from the file.

Infinity08: Ah - I knew there was something wrong here.. :-).
So I did the following changes in the code:

local definitions:
 char Dekning1[3];
 string Dekning;

...
   cout << "Kampnr = " << Kampnr << "\n";
     cout << "\n Stammevalg:\n";
     cout << "HUB, HU, UB, HB, H, U, B ->";
     cin >> Dekning1;
   
     cout << "\nDekning = " << Dekning1 << "\n";
     cout << "\nHele = " << Hele << "\n";
   
    Dekning.assign( Dekning1 );

...

and then the tests works as planned.. :-)

0
 

Author Comment

by:toyboy61
Comment Utility
Are there any other improvements or changes I should do in the code ?
I.e are there any other way to do the test  if (((Dekning == "UB") || (Dekning == "HU") || (Dekning == "HB")) && (Halve == 0)) ?

I'm trying to learn C++, and I really appreciate any good advices or suggestions... :-)
0
 
LVL 53

Expert Comment

by:Infinity08
Comment Utility
>> and then the tests works as planned.. :-)

Ok.

Btw, why not read directly into the std::string ? It's safer. Note especially that  char Dekning1[3] only allows space for TWO characters (not three), so if you try to write more than two characters in there, you'll cause some memory corruption - and that can cause all kinds of nasty things.
0
 

Author Comment

by:toyboy61
Comment Utility
Infinity08: I should have known. Character arrays are null-terminated, right ?

I redefined Dekning from char[3] to String, and dropped the use of Dekning1.
It made my code much more simple, and it works!!

Any other improvements I should do with my code ?

 

for (Kampnr=1;(Kampnr < 13);Kampnr++)
  {
    start:
     cout << "\nKampnr = " << Kampnr << "\n";
     cout << "\nStammevalg:\n"; 
     cout << "HUB, HU, UB, HB, H, U, B ->";
     cin >> Dekning;

    if ((Dekning == "HUB") && (Hele == 0))
      {
        cout << "\nIkke flere helgarderte/varierte tilgjengelig!!";
        cout << "\nPrøv igjen!";
        goto start;
       }
      
   if (((Dekning == "UB") || (Dekning == "HU") || (Dekning == "HB")) && (Halve == 0))
      {
        cout << "\nIkke flere halvgarderte/varierte tilgjengelig!!";
        cout << "\nPrøv igjen!";
        goto start;
       }
     if (((Dekning == "H") || (Dekning == "U") || (Dekning == "B")) && (Sikre == 0))
       {
        cout << "\nIkke flere sikre tilgjengelig!!";
        cout << "\nPrøv igjen!";
        goto start;    
        }
     
      Kamp[Kampnr] += Dekning; 
 
    if (Dekning == "HUB") Hele--;
    if (Dekning == "HU") Halve--;
    if (Dekning == "UB") Halve--;
    if (Dekning == "HB") Halve--;
    if (Dekning == "H") Sikre--;
    if (Dekning == "U") Sikre--;
    if (Dekning == "B") Sikre--;
    
    cout << "\n";
    cout << "Antall hele: " << Hele << "\n";
    cout << "Antall halve:" << Halve << "\n";
    cout << "Antall sikre:" << Sikre << "\n";
   }

Open in new window

0
 

Author Comment

by:toyboy61
Comment Utility
My original problem is back again: Suddenly I lost the contents of SysBuf (a global variable) inside the function SettOppStamme. The position where this happens is marked with a comment block with the following content:
// ============================
// == Here I have lost the contents of SysBuf  ==
// ============================.

Can anyone tell my why I loose the content of SysBuf at this positiion ?

/* =========================================== */
void SettOppStamme()
/* =========================================== */ 
/*
 This routine filles the Kamp-array with the correct values.
*/ 

{

 int Kampnr, teller;
 string Dekning;

#ifdef SYSTEMBUFFER 
 cout << "Utskrift av SysBuf etter at SettOppStamme er valgt:\n"; 
 
  for (teller=0;(teller<(Hele+Halve));teller++)
    cout << SysBuf[teller] << "\n"; 
#endif 
 
 cout << "\n";
 cout << "Stamme settes opp for ";
 cout <<  SysNavn;
 cout << "\n";
 cout << " Antall helgarderte/varierte:   " << Hele << "\n";
 cout << " Antall halvgarderte/varierte: " << Halve << "\n";
 cout << " Antall sikre:" << Sikre << "\n";
 cout << " Antall rekker: " << Rekker << "\n";
 cout << "\n";

#ifdef SYSTEMBUFFER
 cout << "Utskrift av SysBuf etter utskrift av systeminfo:\n";
  for (teller=0;(teller<(Hele+Halve));teller++)
    cout << SysBuf[teller] << "\n"; 
#endif

 KampMeny();
 
#ifdef SYSTEMBUFFER 
 cout << "Utskrift av SysBuf etter retur fra rutine KampMeny():\n";  
  for (teller=0;(teller<(Hele+Halve));teller++)
    cout << SysBuf[teller] << "\n"; 
#endif 
 
  for (Kampnr=1;(Kampnr < 13);Kampnr++)
  {
    start:
     cout << "\nKampnr = " << Kampnr << "\n";
     cout << "\nStammevalg:\n"; 
     cout << "HUB, HU, UB, HB, H, U, B ->";
     cin >> Dekning;

    if ((Dekning == "HUB") && (Hele == 0))
      {
        cout << "\nIkke flere helgarderte/varierte tilgjengelig!!";
        cout << "\nPrøv igjen!";
        goto start;
       }
      
   if (((Dekning == "UB") || (Dekning == "HU") || (Dekning == "HB")) && (Halve == 0))
      {
        cout << "\nIkke flere halvgarderte/varierte tilgjengelig!!";
        cout << "\nPrøv igjen!";
        goto start;
       }
     if (((Dekning == "H") || (Dekning == "U") || (Dekning == "B")) && (Sikre == 0))
       {
        cout << "\nIkke flere sikre tilgjengelig!!";
        cout << "\nPrøv igjen!";
        goto start;    
        }
     
      Kamp[Kampnr] += Dekning; 
 
    if (Dekning == "HUB") Hele--;
    if (Dekning == "HU") Halve--;
    if (Dekning == "UB") Halve--;
    if (Dekning == "HB") Halve--;
    if (Dekning == "H") Sikre--;
    if (Dekning == "U") Sikre--;
    if (Dekning == "B") Sikre--;
      
    cout << "\n";
    cout << "Antall hele: " << Hele << "\n";
    cout << "Antall halve:" << Halve << "\n";
    cout << "Antall sikre:" << Sikre << "\n";
   } 
 
// ============================
// == Here I have lost the contents of SysBuf  == 
// ============================
   
#ifdef SYSTEMBUFFER 
 cout << "Utskrift av SysBuf etter at stammen er satt opp (før utskrift av KampMeny():\n";  
  for (teller=0;(teller<(Hele+Halve));teller++)
    cout << SysBuf[teller] << "\n"; 
#endif 
      
   KampMeny();
   
#ifdef SYSTEMBUFFER 
 cout << "Utskrift av SysBuf etter utskrift av KampMeny():\n";  
  for (teller=0;(teller<(Hele+Halve));teller++)
    cout << SysBuf[teller] << "\n"; 
#endif 
   
   
       
#ifdef SYSCHANGES        
     if (Kamp[Kampnr].find_first_of ("HUB") !=  string::npos)
      {
        if (Kamp[Kampnr] == "HUB") Hele++;
        if (Kamp[Kampnr] == "HU") Halve++;
        if (Kamp[Kampnr] == "UB") Halve++;
        if (Kamp[Kampnr] == "HB") Halve++;
        if (Kamp[Kampnr] == "H") Sikre++;
        if (Kamp[Kampnr] == "U") Sikre++;
        if (Kamp[Kampnr] == "B") Sikre++;
       }
 #endif
       
  cout << "\n";    
}

Open in new window

0
 
LVL 16

Expert Comment

by:sjklein42
Comment Utility
You are indexing Kamp by Kampnr, but Kampnr goes from 1 to 12 in your loop instead of 0 to 11.  Do you mean to index by Kmpnr-1 ?

      Kamp[Kampnr] += Dekning;
0
 

Author Comment

by:toyboy61
Comment Utility
sjklein42: The definition of Kamp as global variable is :

const int MAXKAMPER = 12;
string Kamp[MAXKAMPER+1];

So it is intended that the index should run from 1 to 12, not from 0 to 11.
Yes, I'm loosing the first element but that doesn't matter for me.. :-)
0
 
LVL 53

Assisted Solution

by:Infinity08
Infinity08 earned 150 total points
Comment Utility
>> Infinity08: I should have known. Character arrays are null-terminated, right ?

Correct.


>> It made my code much more simple, and it works!!

That's what I like to hear :)


>> Can anyone tell my why I loose the content of SysBuf at this positiion ?

The content is not lost - you're just not showing it. You are using (Hele+Halve) as the upper limit for your loop, but those variables have been decremented a few times in the for loop right before where you say you lost the contents of SysBuf. So (Hele + Halve) is unlikely to still have the same value as before the for loop.
0
 

Author Comment

by:toyboy61
Comment Utility
Infinity08: You're right - a silly mistake from a newbie like me. I will correct it. Thanx anyway!
0
 

Author Comment

by:toyboy61
Comment Utility
Any other improvements I should do ?
0
 
LVL 53

Assisted Solution

by:Infinity08
Infinity08 earned 150 total points
Comment Utility
There's a lot of things that could be improved, but then that's the case for pretty much all code. So, I won't delve too deep, as it's practice that makes perfect, and making mistakes is the best way not to forget the solution heh.

Here's a few hints to take to heart though :

1) don't use goto. It can make code difficult to read and maintain, and can cause some hard to track down bugs. Instead use loops and conditional expressions effectively.

2) try to avoid global variables as much as possible. Keep the scope (and lifetime) of a variable as small as possible, and pass it around to other functions as parameter if needed.

3) get used to 0 as the first index of an array, and actually use the first item in the array (ie. don't start your loop at 1, but at 0). If you want to display the value 1 to the user for the first item, you can always just add 1 to the index.

4) use std::endl instead of "\n", so line breaks are processed correctly for different platforms.

5) get to know the STL, and try to use it as much as possible. It has many containers, algorithms, and other useful stuff, that come in very handy.

        http://www.cplusplus.com/reference/
0
 

Author Comment

by:toyboy61
Comment Utility
Infinity08:
1) From other programming languages I have learned that you should not use 'goto'. I will try to rewrite my code not using goto in the future. It's not an optimal solution, but it was a "quick and dirty" way to solve my problem.

2) Global variables could be handy in some cases, but mostly I agree with you.
But how do I reference the content of a string array using pointers ?

main()
{
string * strp;
string SysBuf[MAXKAMPER+1];

*strp = &SysBuf;

 Myfunction(strp);
}

// ------------------------------------------------------------------------------------------------------------------
 void Myfunction(string *strp);
 {
 // How do I reference the content of i.e. SysBuf[2] using this pointer in the Myfunction function ?
 }  

or are there other errors in this code snippet ?
0
 
LVL 53

Accepted Solution

by:
Infinity08 earned 150 total points
Comment Utility
>> But how do I reference the content of a string array using pointers ?

Arrays can be passed as they are :

void myfunction(std::string buf[]) {
    std::cout << buf[2];
}

int main() {
    std::string sysBuf[MAXKAMPER+1];
    myfunction(sysBuf);
}

Open in new window


Or better yet, you could opt for a vector instead of an array :

void myfunction(std::vector<std::string>& buf) {
    std::cout << buf[2];
}

int main() {
    std::vector<std::string> sysBuf;
    // fill the vector ...
    myfunction(sysBuf);
}

Open in new window

0

Featured Post

How to improve team productivity

Quip adds documents, spreadsheets, and tasklists to your Slack experience
- Elevate ideas to Quip docs
- Share Quip docs in Slack
- Get notified of changes to your docs
- Available on iOS/Android/Desktop/Web
- Online/Offline

Join & Write a Comment

Often, when implementing a feature, you won't know how certain events should be handled at the point where they occur and you'd rather defer to the user of your function or class. For example, a XML parser will extract a tag from the source code, wh…
What is C++ STL?: STL stands for Standard Template Library and is a part of standard C++ libraries. It contains many useful data structures (containers) and algorithms, which can spare you a lot of the time. Today we will look at the STL Vector. …
The viewer will learn how to pass data into a function in C++. This is one step further in using functions. Instead of only printing text onto the console, the function will be able to perform calculations with argumentents given by the user.
The viewer will learn how to clear a vector as well as how to detect empty vectors in C++.

772 members asked questions and received personalized solutions in the past 7 days.

Join the community of 500,000 technology professionals and ask your questions.

Join & Ask a Question

Need Help in Real-Time?

Connect with top rated Experts

11 Experts available now in Live!

Get 1:1 Help Now