Optimizing my code

[XorioZ]

Introduction:
Hi and thank you in advance. My name is Mikkel and i'm somewhat new to c++.
I actually came here about a month ago to ask how to make a simple cfg parser.
But after reading the rules i decided to try learning on my own and post my results instead.

I'm now done with the most important function in my program and it work's!!!
Fucking nice feeling :)

anyhow in my experience i have probably done a poor job as i carry over some bad habits from my time as a php scriptor.

I was wondering if someone would mind looking over my code and give me some pointers as to how i can optimize it?

what this code does / is supposed to do.
it parses a file line by line checking if certain criteria are met, and then add's the given information to a map called cfgEntries.

again thanks in advance.

void scfgpars::getFileData()
{

    std::string* output; //points to either id or value.
    std::string id, value;

    while(std::getline(file, line)) //go through file line by line.
    {
        output = &id; //set output to point to id.

        if(line.find("=") != std::string::npos && line.at(0) != '=') // if the line has an equals
                                                                     // and it is not the first char
                                                                     // else clear for next line.
        {
            int Ecounter = 0; // equals counter
            for (unsigned i=0; i<line.length(); ++i) //go through all the chars in the line.
            {
                char linechar = line.at(i); // current char.

                if(linechar == '=') // if equals change output pointer to value and add to Ecounter.
                {
                    Ecounter++;
                    output = &value;
                }

                if(Ecounter > 1) // if more then one equals invalidate line and clear for next line.
                {
                    id.clear();
                    value.clear();
                    break;
                }

                if(linechar != ' ' && linechar != '=')  // if it is not an equals or a space.
                    *output += linechar; // add char to output 
            }

            if(Ecounter == 1) //add value to map with id as id.
            {
                cfgEntries [id] = value;
            }
            id.clear();
            value.clear();

        } else {
            id.clear();
            value.clear();
        }

    }
}

EDIT:
Moved to General C++

[XorioZ]

so i made a few changes.
added another map that represents each line of the file as i figured out i could not edit a file directly using fstreams.
instead a make a pair in the cfgEntris map that holds the entry / value and the corresponding line number from the file.
the new map cfgFile then holds each line in order with the linenumber as the id.
This way i can add changes to both cfgFile and cfgEntries by reference and then rewrite the cfg file by iterating through the cfgFile map.
I would like to post all the code here but it has suddenly become quite large :)
I will add the getFileData here and post a pastebin to the whole thing.

On a side-note: when am i supposed to use constant's. I'm having difficulty figuring out when exactly to use it.

void scfgPars::getFileData()
{

    std::string* buffer;
    std::string id, value;
    int lineNumber = 1;

    while(std::getline(iFile, line))
    {

        cfgFile [lineNumber] = line;

        if(line.find("=") != std::string::npos && line.at(0) != '=')
        {
            buffer = &id;
            int Ecounter = 0; // equals counter
            id.clear();
            value.clear();

            for (unsigned i=0; i<line.length(); ++i)
            {
                char lineChar = line.at(i);

                if(lineChar == '=')
                {
                    Ecounter++;
                    buffer = &value;
                }

                if(Ecounter > 1) break;

                if(lineChar != ' ' && lineChar != '=')
                    *buffer += lineChar;
            }

            if(Ecounter == 1)
                cfgEntries [id] = std::make_pair(value, lineNumber);
        }
        lineNumber++;
    }
}

the entire code is here http://pastebin.com/vJcXuxBK

[JLBorges]

> looking over my code and give me some pointers as to how i can optimize it?

Some optimisations, primarily for programmer efficiency:

#include <iostream>
#include <sstream>
#include <map>
#include <string>
#include <cctype>
#include <algorithm>

std::string trim( std::string str ) // remove spaces
{
    std::string trimmed ;
    
    // http://www.stroustrup.com/C++11FAQ.html#for 
    for( char c : str ) // for each char in the string
    {
        // http://en.cppreference.com/w/cpp/string/byte/isspace
        if( !std::isspace(c) ) trimmed += c ;
    }

    return trimmed ;

    // note: the erase-remove idiom could have been gainfully used here
    // https://en.wikipedia.org/wiki/Erase-remove_idiom
}

std::map <int, std::string> cfgFile;
std::map <std::string, std::pair<std::string, int> > cfgEntries;

void getFileData( std::istream& iFile )
{
    int lineNumber = 1;
    std::string line ;

    while( std::getline(iFile, line) ) // go through file line by line.
    {
        cfgFile[lineNumber] = line;

        line = trim(line) ; // sanitise

        // if the line has an equals and it is not the first char, else ignore this line.
        // if more then one equals ignore this line
        /*constexpr*/ const char EQUALS = '=' ;

        // http://en.cppreference.com/w/cpp/algorithm/count
        const int Ecounter = std::count( line.begin(), line.end(), EQUALS ) ;
        if( Ecounter == 1 && line[0] != EQUALS )
        {
            // split the line into two parts: before_equals and after equals
            const std::size_t equals_pos = line.find(EQUALS) ;

            // http://en.cppreference.com/w/cpp/string/basic_string/substr
            const std::string id = line.substr( 0, equals_pos ) ;
            const std::string value = line.substr( equals_pos+1 ) ;

            // add to map
            // http://www.stroustrup.com/C++11FAQ.html#uniform-init
            cfgEntries[id] = { value, lineNumber } ; // std::pair<std::string, int>{ value, lineNumber } ;
        }

        lineNumber++;
    }
}

int main()
{
    std::istringstream test( "abc  = def \n" // id: abc value def
                             "jklmn opqrs \n" // invalid: no =
                             " =wxyz \n" // invalid: = as first non-ws character
                             "ab = cd = ef \n" // // invalid: more than one =
                             " ghi = \n" // fine; = as last character id:ghi value: nothing
                             "jklmn=opqrst\n" // fine, id: jklmn, value opqrst
                             " programmer = XorioZ\n"
                             " num_posts = 3\n"
                             "board=General C++\n"
                            );

    getFileData(test) ;

    std::cout << "sanitised lines\n----------------\n" ;
    // http://www.stroustrup.com/C++11FAQ.html#auto
    for( const auto& pair : cfgFile ) std::cout << pair.first << ". " << pair.second << '\n' ;

    std::cout << "\nids and values\n----------------\n" ;
    for( const auto& pair : cfgEntries )
        std::cout << pair.first << " = " << pair.second.first << " (line:" << pair.second.second << ")\n" ;
}

http://rextester.com/RMONA20236

[XorioZ]

Damn that's some nice code.
Thanks a lot JLBorges. Super nice with the http references.

I might have to leave out the c++11 stuff as i'm using code::blocks with gcc version 4.7.1.
i would like to use v4.8.1 with all the new 11 features but i'm working with SFML and encountering a lot of compiler errors when using the new compiler.

I even bought the c++ 11 primer book to learn from and i'm stuck in c++ 03. at least i think that's the standard i'm using.

anyway thanks a lot.

ps. you removed my pointer stuff :'( *sniff*
std::string* buffer;
std::string id, value;
That pointer was defining for me. I learned how pointers work with that. or i think i did :)

[JLBorges]

#include <iostream>
#include <string>
#include <sstream>

bool is_decimal_int( const std::string& str )
{
    std::istringstream stm(str) ;
    int v ;
    char c ;
    return stm >> v && !( stm >> c ) && stm.eof() ;

    // C++11: http://en.cppreference.com/w/cpp/string/basic_string/stol
}

int main()
{
    for( std::string str : { " 123 ", "-123", "+123", "12a", "0xff", "- 123", "12+3", "++123", "12345678901234", "12345678" } )
        std::cout << "'" << str << "'\t" << std::boolalpha << is_decimal_int(str) << '\n' ;
}

http://rextester.com/JHB51559