Reading input from keyboard

Question

I am required to read the following text from a keyboard (stdin). Please note that it will be entered by the user from the keyboard in this format only.

    #the total size of physical memory (units are B, KB, MB, GB)
    512MB   2       #the following are memory allocations
    {
            abc = alloc(1KB);
             {
                y_ = alloc(128MB);
                x1= alloc(128MB);
                y_ = alloc(32MB);
               for (i = 0; i < 256; i++) abc[i] =alloc(512kB);
                  x1 = alloc(32MB); x2 = alloc(32MB); x3 = alloc(32MB);
               x1.next = x2, x2.next = x3, x3.next = x1;
    }
    abc = alloc(256MB);
    }

• A line beginning with the # sign is considered a comment and is ignored.

• The first two allocations are physical memory size and number of generations.

• A global bracket will be opened and it may be followed by a line called

  abc = alloc(1KB);
  

  where abc is the object name and 1KB is the memory size allocated.

• x1.next = x2, where x1 points to x2.

• The for loop is entered in this format and it can have a same-line command or can have nested for loops.

  for (i = 0; i < 256; i++) abc[i] =alloc(512kB);
  

I have the following code that somewhat takes care of this. I want to know how to improve on it.

#include <iostream>
#include <algorithm>
#include <string>
#include <iomanip>
#include <limits>
#include <stdio.h>
#include <sstream>



using namespace std;
using std::stringstream;

string pMem,sGen, comment,val,input,input_for,id_size,id,init_str1, init_str2, inc_str, id_dummy,s_out,sss, id_dummy1;
int gen=0, pMem_int=0,i=0, gBrckt =0,cBrckt=0, oBrckt=0, id_size_int,v1,v2, for_oBrckt=0,for_cBrckt=0,y=0, y1=0, g=0;
unsigned long pMem_ulong =0, id_size_ulong;
char t[20], m[256], init1[10],init2[10],inc[10];
unsigned pos_start, pos,pos_strt=0,pos_end=0;

string extract(string pMem_extract);
unsigned long toByte(int pMem_int_func, string val);
void commentIgnore(string& input);
void func_insert();
void func_insert_for();
stringstream out;
void commentIgnore_for(string& input_for);

int main() 
{
  /* Reading the input main memory and num of generations */
  /* Ignoring comment line */
  cin >> pMem;
  if(pMem == "#") {
    cin.clear();
    pMem.clear();
    getline(cin,comment);
    cin >> pMem;
  }
  if(pMem == "#") {
    cin.clear();
    pMem.clear();
    getline(cin,comment);
    cin >> pMem;
  }
  if(pMem == "#") {
    cin.clear();
    pMem.clear();
    getline(cin,comment);
    cin >> pMem;
    }
  /* Reading input generations */
  cin>> sGen;
  if(sGen == "#") {
    cin.clear();
    sGen.clear();
    getline(cin,comment);
    cin >> sGen;
  }
  if(sGen == "#") {
    cin.clear();
    sGen.clear();
    getline(cin,comment);
    cin >> sGen;
  }
  if(sGen == "#") {
    cin.clear();
    sGen.clear();
    getline(cin,comment);
    cin >> sGen;
  }
  /* Convert sGen and physical memory to int and report error if not a number */
  gen = atoi(sGen.c_str());
  if(gen ==0) {
    cerr << "Generation must be a number"<<endl;
      exit(0);
  }
  pMem_int = atoi(pMem.c_str());
  //  cout<< gen<<" "<<pMem_int<<endl;

  /* Now that the number from pMem is removed, get its unit B,MB,KB */
  extract(pMem); /* returns val(string) */

  /* convert the given physical memory to Byte. input: pMem_int*/
  toByte(pMem_int, val); /*  return(pMem_ulong)*/
  // move pMem_ulond to another location to keep address intact
  /* read rest of the inputs  */
  /* Ignore comment lines before the global bracket */
   cin >> input;
  if(input == "#"){
    cin.clear();
    input.clear();
    getline(cin,comment);
    cin >> input;
  }
  if(input == "#"){
    cin.clear();
    input.clear();
    getline(cin,comment);
    cin >> input;
  }
  if(input == "#"){
    cin.clear();
    input.clear();
    getline(cin,comment);
    cin >> input;
  }

  if(input.compare("{") ==0)
    gBrckt=1;

  else {
    cerr<< "Syntax error\n";
    exit(0);
  }

  /* Clearing the input stream for next input */
  cin.ignore(numeric_limits<streamsize>::max(), '\n');
  cin.clear();
  input.clear();
  //cout<<"input: "<<input<<endl;
  while( getline(cin,input)) {

    if(input == "CTRL-D")
      break;

    commentIgnore(input);
    //cout<<"inputloop: "<<input<<endl;

    /* If input = '{' or '}'*/
    if(input.compare("{") ==0)
      oBrckt = oBrckt + 1;

     if (input.compare("}") ==0)
      cBrckt = cBrckt + 1;

     if (((input.find("alloc"))!= string::npos) && (input.find("alloc") < input.find("for"))) {
       func_insert();
       //call the allocate function here with name: id, size: id_size_ulong
     }

     if ((input.find("for")) != string::npos) {
        sscanf(input.c_str(), "for (%s = %d; %s < %d; %[^)])", init1, &v1, init2, &v2, inc);
    init_str1 = init1, init_str2 = init2, inc_str = inc;

    cout<<init1<<" ="<< v1<<" "<<init_str1<<" < " << v2<< " "<< inc_str<<endl;
    cout << input <<endl;

    if(init_str1 != init_str2) {
      cerr << "Error!\n";
      exit(0);
    }

    if ((input.find("alloc"))!= string::npos) {
      // unsigned pos = (input.find("alloc"));

      if((input.find(";")) != string::npos) {

         pos_start = (input.find(")")+1);
        string alloc_substr  = input.substr(pos_start);
        cout<<"Substring alloc: "<< alloc_substr<<endl;

        func_insert();
        //call the allocate function here with name: id, size: id_size_ulong
      }
      else {
        cerr << "ERROR: SYNTAX\n";
        exit(0);
      }


    }
    //  cin.ignore();
    while(getline(cin,input_for)) {
      commentIgnore_for(input_for);

      if ((input_for.find("{") != string::npos)) {
        pos = input_for.find("{");
        for_oBrckt = for_oBrckt+1;
        string for_brckt = input_for.substr(pos,pos);
        cout<< "Found: " << for_oBrckt<<endl;
      }


      if ((input_for.find("}") != string::npos)) {
         pos = input_for.find("}");
        for_cBrckt = for_cBrckt+1;
        string for_brckt = input_for.substr(pos,pos);
        cout<< "Found: " << for_cBrckt<<endl;
      }

      if (((input_for.find("alloc"))!= string::npos) && (input_for.find("alloc") < input_for.find("for"))) {
        func_insert_for();
        //call the allocate function here with name: id, size: id_size_ulong
      }




      if(for_oBrckt == for_cBrckt)
        break;


    }
    cout<<"out of break"<<endl;
     }

      if (((input.find(".next"))!= string::npos) && (input.find(".next") < input.find("for"))) {
       func_insert();
       //call the allocate function here with name: id, size: id_size_ulong
     }

      if(((cBrckt-oBrckt)) == gBrckt)
       break;
  }

}

/*---------------------- Function definitions --------------------------------*/
/* Function to extract the string part of physical memory */
string extract(string pMem_extract) {
  i=0;
  const char *p = pMem_extract.c_str();
  for(i=0; i<=(pMem_extract.length()); i++) {
    if (*p=='0'|| *p=='1'|| *p=='2'|| *p=='3'|| *p =='4'|| *p=='5'|| *p=='6'|| *p=='7'|| *p=='8'|| *p=='9')
      *p++;
    else {
      val = pMem_extract.substr(i);
      return(val);
    }
  }
}

/* Convert the physical memory to bytes. return(pMem_ulong);*/
unsigned long toByte(int pMem_int_func, string val)
{
  if (val == "KB")
    pMem_ulong =  (unsigned long) pMem_int_func * 1024;
  else if (val == "B")
    pMem_ulong = (unsigned long) pMem_int_func;
  else if (val == "GB")
    pMem_ulong = (unsigned long) pMem_int_func * 1073741824;
  else if (val == "MB")
    pMem_ulong = (unsigned long) pMem_int_func * 1048576;
  else {
    cerr<<"Missing the value in memory, B, KB, MB, GB\n";
    exit(0);
  }

  return(pMem_ulong);
}


/*Ignoring comment line*/
void commentIgnore(string& input)
{
  unsigned found = input.find('#');

  if (found!=std::string::npos)
   input= input.erase(found);

  else
    return;
  return;
}


void func_insert() {
 sscanf(input.c_str(), "%s = alloc(%[^)]);", t, m);
       id =t;
       id_size =m;
       cout<<"Tag: "<<id <<"  Memory: "<<id_size<<endl;
       extract(id_size); /* Separates B,MB,KB and GB of input, returns val*/
       id_size_int = atoi(id_size.c_str());
       /* Convert object size to B */
       toByte(id_size_int, val); /* return(pMem_ulong) */
       id_size_ulong = pMem_ulong;

}

void func_insert_for() {
  sscanf(input_for.c_str(), "%s = alloc(%[^)]);", t, m);
  id =t;
  id_size =m;
  if(!((id.find("[")) && (id.find("]")) != string::npos)) {
    cout<<"Tag: "<<id <<"  Memory: "<<id_size<<endl;
    extract(id_size); /* Separates B,MB,KB and GB of input, returns val*/
    id_size_int = atoi(id_size.c_str());
    /* Convert object size to B */
    toByte(id_size_int, val); /* return(pMem_ulong) */
    id_size_ulong = pMem_ulong;
    // allocate here
    return;
  }
  else {
    if(inc_str.find("++"))
      y1 =1;
    if(inc_str.find("="))
      {
    sss = inc_str.substr(inc_str.find("+") +1);
    y1 = atoi(sss.c_str());
    cout<<"y1:"<<y1<<endl;

      }
    pos_strt = id.find("[");
    pos_end = id.find("]") -1;
    cout<<"Positions start and ebd: " << pos_strt<<pos_end<<endl;
    id_dummy = id.substr(0,pos_strt);
    id = id_dummy;
    cout<<"Tag: "<<id_dummy <<"  Memory: "<<id_size<<endl;
    extract(id_size); /* Separates B,MB,KB and GB of input, returns val*/
    id_size_int = atoi(id_size.c_str());
    /* Convert object size to B */
    toByte(id_size_int, val); /* return(pMem_ulong) */
    id_size_ulong = pMem_ulong;
    //allocate here
    cout<<"v1: " << v1 << " " << v2<<endl;
    // g = 0;
    for(y = v1; y < v2; y= y+y1) {
      // allocate here
    }
  }
  return;
}

void commentIgnore_for(string& input_for)
{
  unsigned found = input_for.find('#');

  if (found!=std::string::npos)
   input_for= input_for.erase(found);

  else
    return;
  return;
}

Answer 1

• With this much code, you definitely should avoid using using namespace std. More info about that can be found here.

• You have a lot of global variables, and you shouldn't have any. They're generally discouraged as they can be modified anywhere within the program. This can introduce bugs, and it'll just make maintenance more painful. Only use them when you have no other choice.

  Otherwise, in this procedural code, just pass to variables to functions as needed. That'll limit the variables scopes to which entity needs them, and it'll vastly improve your code. Based on the rest of the code, it looks like you've declared most of the variables in global. This is either lack of awareness of global variables, or some kind of laziness with handling variables.

• Keep your use of indentation and whitespace consistent. Four spaces is customary for indentation, and there should be whitespace between operators and after commas.

• Don't leave commented-out code present; it just clutters your code. If there's a specific reason for doing that in certain places, specify that; otherwise, leave it out.

• Instead of having all those function prototypes, have main() defined below each of the functions. That way, they'll already be recognized by main() when you need to call them from there.

• void functions don't need an explicit return at the end; they'll always return there. However, you'll still need it if you need to exit from the function prematurely.

• For the input blocks in main(), have each different one in a different loop, rather than typing out each one. Don't Repeat Yourself (DRY).

  Each of those different blocks are done three times, so have a for loop for each one (using the first as an example):

  for (int i = 0; i < 3; i++)
  {
      if(pMem == "#") {
          cin.clear();
          pMem.clear();
          getline(cin,comment);
          cin >> pMem;
      }
  }
  

  Also, since you have std::cin and std::getline together, you should have std::ignore between them here, and in other similar instances.

• In toByte, there's no need for C-style casts. Use C++-style casts here, specifically with static_cast<>:You don't need a C-style cast here:

  pMem_ulong = static_cast<unsigned long>(pMem_int_func);
  

• It doesn't make sense to call exit(0) after reporting an error. The 0 argument indicates successful termination, whereas 1 indicates failure.

  Moreover, since this is done in main(), just use return 1.

Answer 2

I don't understand what your program is doing: you've defined what the input is, but haven't said what the output must be.

Re. making it "whitespace compatible", your program is parsing input. Parsing is often done in two stages:

• A "lexer" splits the input stream into 'tokens'
• A second-stage "parser" combines the tokens with a "grammar"

Examples of the things (different types of thing) which I think could be 'one token' in the input stream are:

• Whitespace
• Comment
• Puntuation (e.g. '=', ';', '{', '++', etc.)
• Identifier, i.e. alphabetic first character followed by one or more alphanumerics (e.g. "abc", "alloc", "x2"
• Numbers (e.g. "2", "256")
• Memory (e.g. "1KB", "512MB")

I think your code would be clearer if you did it in two stages:

• Inspect the stream of input characters and turned it into a list of "tokens"
• Parse the input list of tokens

A "token" could be represented by a class and enum for example:

enum TokenType
{
    Comment,
    Whitespace,
    Punctuation,
    ... etc ...
}

struct Token
{
    TokenType tokenType; // e.g. Memory
    std::string value; // e.g. "512MB"
};