Line not being well read

[Ivitoh]

Hello, I have a problem when reading lines in a text, passing them in to a list<string> (I must use this) and then evaluate and do the actions the text says. Let me clarify, lets say i have:
INT a
INT b
a = 4
b = 1
IF ( b = 5 )
The program should recognize the INT a and save it into a node, same with b, then when it reads the next line, it should store for a the value of 4 and for b the 1, and then the IF () should evaluate if the sentence between brackets is true or not. My problem is that the IF is not being well read. I will post the code downside so you guys can see it.

  class Nodo {
protected:
	string s;
	int e;
	char c;
	Nodo* next;
public:
	Nodo() {
		next = NULL;
	};
	Nodo(string a) {
		s = a;
		next = NULL;
	};
	Nodo(int a) {
		e = a;
		next = NULL;
	};
	Nodo(char a) {
		c = a;
		next = NULL;
	};

	void set_s(string a) {
		s = a;
	};
	void set_c(char a) {
		c = a;
	};
	void set_i(int a) {
		e = a;
	};
	void set_next(Nodo* n) {
		next = n;
	};

	string get_s() {
		return s;
	};
	int get_i() {
		return e;
	};
	char get_c() {
		return c;
	};

	Nodo* get_next() {
		return next;
	};

	bool es_vacio() {
		return (next == NULL) ? true : false;
	}
};

class AYED2019 {
private:
	Pila* ye;
	list<string> Original;
	list<string>::iterator it;
	list<Nodo> declaraciones;
	list<Nodo>::iterator conteo;
	Lista* lista;
	Pila* invertirPila(Pila* original);
	Lista* invertirLista(Lista* original);
	Nodo* asignaciones;
	bool ok;
	bool isnum(string valor);
public:
	AYED2019() {
	};
	AYED2019(list<string> OG) {
		ok = checkParentesis(OG);
	};
	int error = 0;
	char d, p1;
	int o1, o2;
	bool readFile();
	bool checkParentesis(list<string> Original);
	bool posfijo(Pila* p, list<string> Original);
	bool isnumS(char valor);
	//bool expresionesbooleanas();
	void operar(Pila* p, int a);
};

bool AYED2019::readFile() {
	string fichero = "trabajo1.txt";
	ifstream inputFile;
	inputFile.open(fichero.c_str());
	string cad;
	string word;
	Pila* head = new Pila();
	Lista* list = new Lista();
	
	if (!inputFile) {
		cerr << "Open Faiulre" << endl;
		exit(1);
		return false;
	}
	while (getline(inputFile, word)) {
		it = Original.begin();
		Original.push_back(word);
		head->adds(word);
		list->adds(word);
		it++;
	}
	lista = invertirLista(list);
	ye = invertirPila(head);
	checkParentesis(Original);
}

bool AYED2019::posfijo(Pila * p, list<string> Original){
	bool cond;
	int aux = 0;
	int d;
	stringstream str;
	string ss;
	for (it = Original.begin(); it != Original.end(); it++) {
		string& cadena = *it;
		if (cadena.find("INT") == 0) {
			declaraciones.push_back(cadena.at(4));
		}

		if (cadena.at(2) == '=') {
			for (conteo = declaraciones.begin(); conteo != declaraciones.end(); conteo++) {
				if (conteo->get_c() == cadena.at(0)) {
					if (isnumS(cadena.at(4))) {
						d = atoi(&cadena.at(4));
						conteo->set_i(d);
					}
				}
			}
		}

		if (cadena.at(0)=='I' && cadena.at(1) =='F') {
			for (conteo = declaraciones.begin(); conteo != declaraciones.end(); conteo++) {
				if (cadena.at(3) = conteo->get_c()) {
					o1 = conteo->get_i();
					cout << "Este es el valor del primer caracter que le sigue al IF: " << o1 << endl;
					if (cadena.at(5) = conteo->get_c()) {
						o2 = conteo->get_i();
					}
					else {
						o2 = 0;
					}
					cout << "Este deberia ser del segundo, o sea 5: " << cadena.at(5) << endl;
					cout << "Y este el 5 pasado a numero: " << atoi(&cadena.at(5)) << endl;
					
				switch (cadena.at(4)) {
					case '=': if (o1 == o2) {
						cond = true;
					}
						 else {
						cond = false;
					} break;
					case '<':if (o1 < o2) {
						cond = true;
					}
							 else {
						cond = false;
					} break;
					case '>':if (o1 > o2) {
						cond = true;
					}
							 else {
						cond = false;
					} break;
					default: if (o1 != o2) {
						cond = true;
					}
							 else {
						cond = false;
					} break; // != es un char[2], no se puede usar en un switch 
					
					}
					
				}
			}
			//cout << "o2: " << o2 << "o1:" << o1 << endl;
		}
		/*if (cond == true) {
			cout << "Hay una igualdad" << endl;
		}
		else {
			cout << "Nada por aqui..." << endl;
		}
		*/

I hope I made my self clear.

[Duthomhas]

I am only glancing over your code, so comments will be minimal.
Also, I will make multiple posting comments.

First: naming.

You are struggling with naming things well. For example, it is not clear what the parts of your Node class are for. You have to look at the code to see that c is the variable name, s and e are unused, and the names of their accessors do not match.

Life is much easier when you are explicit about what the node pieces are, and leave out everything that is not strictly necessary.

struct Elemento
{
  typedef char Nombre;
  typedef int  Valor;

protected:
  Nombre    nombre;
  Valor     valor;
  Elemento* siguiente;

  Elemento( Nombre nombre, Elemento* siguiente = nullptr ): nombre(nombre), valor(), siguiente(siguiente) { }
  
  friend class Entorno;
};

struct Entorno
{
protected:
  Elemento* primer;
  
  Elemento* buscar( char nombre ) const
  {
    Elemento* v = primer;
    while (v)
    {
      if (v->nombre == nombre)
        return v;
      v = v->siguiente;
    }
    return nullptr;
  }
  
public:
  Entorno(): primer(nullptr) { }
  
  bool existe( char nombre ) const
  {
    return buscar( nombre ) != nullptr;
  }
  
  Elemento::Valor& operator [] ( char nombre )
  {
    Elemento* v = buscar( nombre );
    if (!v)   v = primer = new Elemento( nombre, primer );
    return    v->valor;
  }
  
  void borrar( char nombre )
  {
    // Esta función no es necesaria (actualmente)
  }
};

This makes maintaining your environment simple:

Entorno entorno;
entorno['a'] = 12;
if (entorno.existe('a')) std::cout << "a = " << entorno['a'] << "\n";

You could totally skip all the linked list stuff, though, by simply using a std::unordered_map:

typedef std::unordered_map <char, int> Entorno;

Which gives you a very similar interface:

Entorno entorno;
entorno['a'] = 12;
if (entorno.contains('a')) std::cout << "a = " << entorno['a'] << "\n";

Being a standard container gives you other super powers, too.

More to come. (And an actual answer to your stated question.)

[edit]
Fixed typo.

[Duthomhas]

Your it appears to be the iterator through the list of the current line. Name it something better. The Original could be better named something better too.

  list<string> script; // or guión or something like that
  list<string>::iterator linea_actual;

Now when you use them below it is abundantly clear what they are.

Do not separate variable names (“declarations”) from the names and values (what I have called the “environment”). It only produces useless duplication of information. You only need the environment key→value lookup.

So, for example, the following lines turn into the following code:

INT a

entorno['a']; // Create the variable named 'a' (and assign it a default value of zero)

a = 12

entorno['a'] = 12; // Assign the value 12 to the variable named 'a'

I do not know what some of your objects are. You seem to have a pile/heap/stack/? (ye) and a list (lista). Both names are uninformative, and are likely unnecessary.

You can certainly use a stack to process fancy arithmetic operations, but...



You should typically declare variables as close to their use as possible, and let them not exist when they are not needed. All those things like d, p1, o1, etc should not be persistent objects.

Declare them when they are needed, and let them not exist elsewhere.

if (cadena.at(2) == '=') {
			for (conteo = declaraciones.begin(); conteo != declaraciones.end(); conteo++) {
				if (conteo->get_c() == cadena.at(0)) {
					if (isnumS(cadena.at(4))) {
						int d = atoi(&cadena.at(4));
						conteo->set_i(d);

You can completely eliminate the name too:

						conteo->set_i(atoi(&cadena.at(4));

However, as I described in your other post (www.cplusplus.com/forum/beginner/253468/), tokenizing the line is exceedingly useful:

  for (auto linea_actual = script.begin(); linea_actual != script.end(); ++linea_actual)
  {
    auto tokenes = tokenizar( *linea_actual );

    if (tokenes.size() == 2) and (tokenes[0] == "INT"))
    {
      if (!es_identificador( tokenes[1] )) fooey();
      entorno[tokenes[1][0]];
    }

    if ((tokenes.size() == 3) and (tokenes[1] == "="))
    {
      if (!es_identificador( tokenes[0] ) or !es_cifra( tokenes[2] ))
        fooey();
      entorno[tokenes[0][0]] = stoi( tokenes[2] );
    }

    if ((tokenes.size() == 2) and (tokenes[0] == "JUMP"))
    {
      if (!es_cifra( tokenes[1] )) fooey();
      auto nueva_linea = stoi( tokenes[1] );
      if ((nueva_linea < 1) or (nueva_linea >= script.size())) fooey();
      linea_actual = std::next( script.begin(), nueva_linea - 1 );
      --linea_actual;
      continue;
    }

    if ((tokenes.size() = 6) and (tokenes[0] == "IF"))
    {
      …
    }

    …
  }

Anyway, hope this helps.

[Duthomhas]

Ah, for the IF.

"IF ( a = 5 )"

This is 6 tokens:
  IF required exactly
  ( required exactly
  a left hand argument
  = operator
  5 right hand argument
  ) required exactly

This makes handling it easier.

if ((tokens.size() > 3) and (tokens[0] == "IF"))
{
  // Now validate the other stuff. First, parentheses:
  if ((tokens[1] != "(") or (tokens.back() != ")")) fooey();

  // Next, currently acceptable things: "name|value operator name|value"
  if (tokens.size() != 6) fooey();
  if (!es_identificador( tokens[2] ) and !es_cifra( tokens[2] )) error();
  if (!es_identificador( tokens[4] ) and !es_cifra( tokens[4] )) error();
  if (!es_comparador( tokens[3] )) error();

  // Get the values to compare
  int mi;
  if (es_identificador( tokens[2] ))
  {
    if (!entorno.existe( tokens[2][0] )) fooey();
    else mi = entorno[tokens[2][0]];
  }
  else mi = stoi( tokens[2] );

  int md;
  …

  if (comparar( mi, tokens[3], md )) continue;  // Execute the next line
  else ++linea_actual;  // Skip the next line
}

Notice that this is already getting a little messy. But it should be enough to move you forward.

Hope this helps.