Vector of pointers to class objects dynamic allocation and dellocation

[SeppJ]

euklid schrieb:

[*] Actually, I want to read files with at least one million SNP( rows) with at least 4 columns. That is why, as far as I learnt, allocation and deallocation is assumed to be very good for fast computing.

You are mistaken. Twice:
1. Dynamic memory allocation is waaaaay slower than static variables. But you need it here, because many operating system enforce a very small stack size (Windwos for example ~ 1 MB), there is no way to have a static array this size. And you don't know how many values will be in the file, thus you need dynamic allocation anyway.
2. Dynamic memory management in C++ does not mean using new and delete. You very very rarely need new and delete and never for the purpose of dynamic memory management. The vector does the dynamic stuff for you. And it is good at what it does. If it helps you can imagine that the vector uses new and delete internally (it does not, but that is another story).

[*] I have put all the variables in public because I have to use them all over the program and I don't know yet if it will be okay to put them in private. I would try to use them as private and then a friend function or something like that.

That is not an argument. That's either lazyness or bad design. If you need friend a lot, your classes are not well designed. If your functions need a lot of private data, maybe they should be methods of your class? Or maybe your class has data-members that don't belong in this class.

I would be thankful to you if you could write me the solution or give me some hints .

Here you are:

#include <iostream>
#include <fstream>
#include <vector>
#include <string>
#include <sstream>

using namespace std;

class CSNP
{
private:
  int nchr;
  string snpName;
  string allele1;
  string allele2;
  friend ostream& operator<<(ostream &out, const CSNP &csnp)
  {
    // This was your display-function. Look up "operator overloading" if you
    // don't understand what is going on here.
    return out << "chr No:"<< csnp.nchr<< ", SNP ID: "<< csnp.snpName 
               << ", allele1:" << csnp.allele1<< " and alelle2: "<< csnp.allele2;
  }
  friend istream& operator>>(istream& in, CSNP &csnp)
  {
    // This function was your lines 43-65.

    // Ignore the empty lines and lines starting with '#'
    string line;
    while (getline(in, line) and (!line.size() or line[0] == '#'));

    // Parse the line. Yes, it's this easy.
    stringstream line_parser(line);
    line_parser >> csnp.nchr >> csnp.snpName >> csnp.allele1 >> csnp.allele2;

    // If parsing failed set the error flags
    if (!line_parser) in.setstate(ios::failbit);

    return in;
  }
}; 

int main()
{
  // No need for any pointers or globals
  vector<CSNP> genInfo;
  {
    // This was the rest of your readFile function that was not covered above.
    // I won't even make a separate function for these four lines.
    CSNP value;
    ifstream file("test.map");
    while (file >> value)
      genInfo.push_back(value);
  }
  // Note the use of scoping. value and file do no longer exist at this point.
  // The ifstream was closed automatically upon its destruction

  // This has not really changed. Using unsigned to get rid of the compiler
  // warning, use the overloaded operator instead of the display function
  // and use '\n' instead of endl as endl would be '\n' << flush and we don't
  // need the flush.
  for (unsigned int i=0; i<genInfo.size(); ++i)
    cout << genInfo[i] << '\n';
}

This program does about the same things your old program did. See how it's only about 40 lines of code? And it is much more robust at the same time and should even be faster. Try it: Your program will often Segfault when there are illegal lines in the input file. Or sometimes even on comment lines.

Use the power of the standard library! And maybe you should get a good book. Germans can always say that there is no good German literature on C++ but there are so many good English titles: The C++ Primer, Thinking in C++ (free ebook), the books by Stroustrup (the inventor of C++) himself, the (a tad more advanced) books by Myers.

[euklid]

Thank you very much ,,SeppJ" for your kind efforts. One more question which I wanted to ask you is : Will it be a problem if I read a large file with at least (1000000) rows?
After your comments, I searched articles in Dynamic memory and found the following text.

Frequent allocation and deallocation of dynamic memory leads to heap fragmentation, especially if the application is running for long periods and allocates small memory chunks. Learn what the dangers of heap fragmentation are and how you can avoid this problem.A highly fragmented heap may have a large number of free memory blocks but they are small and non-contiguous.

Here was something more:

To avoid heap fragmentation, first of all, use dynamic memory as little as possible. In most cases, you can use static or automatic storage instead of allocating objects dynamically. Secondly, try to allocate large chunks rather than small ones. For example, instead of allocating a single object, allocate an array of objects at once, and use these objects when they are needed. If all these tips don't solve the fragmentation problem, you should consider building a custom memory pool.

I am also looking on the litrature you mentioned. Hopefully, it will help me a a lot for further progress.
Thank you once again and have a nice evening.
best wishes
Euklid

[SeppJ]

1000000 is a rather small number nowadays. If it were 1000000000 rows (or more generally: several Gigabytes of data) I would start thinking about something else. vector should not fragment the heap very much as it just allocates one large chunk of memory. It's your old scheme of allocating single objects and saving pointers to them that may cause heap fragmentation.

edit: You might also want to think about your CSNP-data-members: Is string the correct model for the kind of data you need for snpName and allele1? From your example file it looks more like an enum, char[4] or int. You should consider using a custom datatype for these.

[Skym0sh0]

i havent followed the thread yet, but about your question to read in 10...00 rows of data. consider the possibility to read only some less rows you really need isntead of the whole file...

[euklid]

Skym0sh0 schrieb:

i havent followed the thread yet, but about your question to read in 10...00 rows of data. consider the possibility to read only some less rows you really need isntead of the whole file...

What you mean by ,, consider the possibility to read only less rows?" I have to write a program to read nearly 1000,000 rows. I have read a file with nearly 960,000 using the code corrected by SeppJ.It took 14 seconds in my computer windows vista.
By the way, I have a question to SeppJ. You never closed the file ,,test.map" Isn't it necessary to close the file after reading? If not , how it works? Would you please mind explaining about it?
thank you and have a nice weekend.
Euklid

[cooky451]

euklid schrieb:

By the way, I have a question to SeppJ. You never closed the file ,,test.map" Isn't it necessary to close the file after reading? If not , how it works? Would you please mind explaining about it?

He mentioned that in line 55, the file is closed by the destructor.

[euklid]

Sorry "SeppJ", but I have to bother you once again.
Actually, my ,,test.map" file has two options. In the first case, test.map file looks like
**
#nchr #snpName #allele1 # allele2
1 snp1 0 1
3 snp2 0 2
2 snp3 1 1
2 snp4 2 2
3 snp5 1 2
**

In the second case: it looks like
**

#snpName #allele1 # allele2
snp1 0 1
snp2 0 2
snp3 1 1
snp4 2 2
snp5 1 2
**
Then I was trying to check this case on your code like following but it didn't work. Would you please mind looking at this code. Line 41 is not working. it gives an error message
**
Error: can't call member function "bool CSNP::checkCorrectMapfile(std::string )" without object.
**

#include <iostream>
#include <fstream>
#include <vector>
#include <string>
#include <sstream>
#include<cstdlib>

using namespace std;

class CSNP{
public:
	bool checkCorrectMapfile(string line);
private:
	static bool threeColumns;
	int nchr;
	string snpName;
	string allele1;
	string allele2;
	friend ostream& operator<<(ostream &out, const CSNP &csnp);
	friend istream& operator>>(istream& in, CSNP &csnp);
};

int main(){
  vector<CSNP> genInfo;{
    CSNP value;
    ifstream file("test.map");
    while (file >> value)
      genInfo.push_back(value);
  }
  for (unsigned int i=0; i<genInfo.size(); ++i)
    cout << genInfo[i] << '\n';
return 0;} 

/Defining class objects:
bool CSNP::threeColumns=false;

istream& operator>>(istream& in, CSNP &csnp){
    string line;
    while (getline(in, line) and (!line.size() or line[0] == '#'));
    stringstream line_parser(line);
    if(CSNP::checkCorrectMapfile(line)) // this line is not working 
	line_parser >> csnp.nchr >> csnp.snpName >> csnp.allele1 >> csnp.allele2;
	else 
		line_parser >> csnp.snpName >> csnp.allele1 >> csnp.allele2;
    if (!line_parser) in.setstate(ios::failbit);
    return in;}

bool CSNP::checkCorrectMapfile(string line){
	bool tfValue=false;
		stringstream ss(line);	  
	 string buff;
	  vector<string> tokens;
	 	while(ss>>buff)
     tokens.push_back(buff);
	if(threeColumns && tokens.size()!=3){
    	 cerr<<"your map file was expected to have 3 columns but this is not the case.";
		 exit(1);
	}
	if(!threeColumns &&tokens.size()!=4){
    		cerr<<"your map file was expected to have 4 columns but this is not the case";
			exit(1);
	}		
	if(threeColumns)tfValue=true;
	 return tfValue;}

 ostream& operator<<(ostream &out, const CSNP &csnp){
    return out << "chr No:"<< csnp.nchr<< ", SNP ID: "<< csnp.snpName
               << ", allele1:" << csnp.allele1<< " and alelle2: "<< csnp.allele2;
  }

[Gast]

class CSNP{
public:
    static bool checkCorrectMapfile(string line);
// ...
};

[euklid]

• Gugelmoser schrieb:

class CSNP{
public:
    static bool checkCorrectMapfile(string line);
// ...
};

Hi Gugelmoser, thank you so much! would you please explain what kind of miracle this static in the code

static bool checkCorrectMapfile(string line);

did and it worked.why it didn't work without static?

best wishes
euklid

[Gast]

euklid schrieb:

• Gugelmoser schrieb:

class CSNP{
public:
    static bool checkCorrectMapfile(string line);
// ...
};

Hi Gugelmoser, thank you so much! would you please explain what kind of miracle this static in the code

static bool checkCorrectMapfile(string line);

did and it worked.why it didn't work without static?

• I tried with static but checkCorrectMapfile is still not doing what I want this function to do. Even though I have four columns in the map file, it is still saying that:
  "your map file was expected to have 4 columns but this is not the case"
  I do not know why.

best wishes
euklid

class Person
{
    private:
        double weight;

    public:
        Person(double w) : weight(w) {}
        double whats_my_weight() { return weight; };    
};

int main()
{
    /*
        In general, you'll always have to greate an object.
        Look, what sense would it make if you do Person::whats_me_weight()... if you have no person, you can't have any weight.
    */
    Person p(88.5);
    cout << "I weight " << p.whats_my_weight();

    /*
        But sometimes you want a function to be independent from a object... then static comes into play, like in your case.
        You want to call a function with its class name using the scope operator: you want to do CSNP::checkCorrectMapfile(line)
        --> thus you want a function being independent from an object.
    */
}

I hope that helps a bit.

[euklid]

class Person
{
    private:
        double weight;
    
    public:
        Person(double w) : weight(w) {}
        double whats_my_weight() { return weight; };    
};

int main()
{
    /*
        In general, you'll always have to greate an object.
        Look, what sense would it make if you do Person::whats_me_weight()... if you have no person, you can't have any weight.
    */
    Person p(88.5);
    cout << "I weight " << p.whats_my_weight();
    
    
    /*
        But sometimes you want a function to be independent from a object... then static comes into play, like in your case.
        You want to call a function with its class name using the scope operator: you want to do CSNP::checkCorrectMapfile(line)
        --> thus you want a function being independent from an object.
    */
}

I hope that helps a bit.

Thank you so much! I appreciate your explanation!

• I tried with static but checkCorrectMapfile is still not doing what I want this function to do. Even though I have four columns in the map file, it is still saying that:
  "your map file was expected to have 4 columns but this is not the case".
• I found that vector<string> tokens has 0 size. This means there is no stream any more in when I call this function. But I still do not know what to do. I think after line 40 of the code

stringstream line_parser(line);

I should not write

if(CSNP::checkCorrectMapfile(line))

Probably the line has no value any more. However, I still do not know how to solve this problem.

[Gast]

edit: Murks

[euklid]

thank you so much for kind response. it works fine but in this way, I have to open and read the file two times: once when I want for checkCorrectMapfile and once for the function

ostream& operator<<(ostream &out, const CSNP &csnp)

since my file is very large, I am interested to read the file only once to check if the correctness of test.map and then save it as an object of class type.
using the following function.

stream& operator>>(istream& in, CSNP &csnp){
    string line;
    while (getline(in, line) and (!line.size() or line[0] == '#'));
    stringstream line_parser(line);
    if(CSNP::checkCorrectMapfile(line)) // this line is not working
    line_parser >> csnp.nchr >> csnp.snpName >> csnp.allele1 >> csnp.allele2;
    else
        line_parser >> csnp.snpName >> csnp.allele1 >> csnp.allele2;
    if (!line_parser) in.setstate(ios::failbit);
    return in;}

However, it is not working. I have written this problem as a simple example. If anyone could find the solution of the following problem, I would be thankful.
The problem is is written as comments in lines 22, 23 and 27 of the following program

#include<iostream>
#include<sstream>
#include<string>
#include<vector>
#include<cstdlib>
using namespace std;
int main(){
	string line;
	string name, famName, buff;
	vector<string> tokens;
	cout << " enter your name: ";
	while(cin){
		getline(cin, line);
		if(line=="0")exit(1);
	cout << "your name is: "<< line <<".\n"; 
	stringstream ss(line);
		while (ss>> buff)
		tokens.push_back(buff);
		for(int i=0; i<tokens.size();i++)
			cout <<" "<<tokens[i];
			cout <<endl;
		/* I want to use ss again but it is not working 
		 Is the life time of ss expired now?
		 */
		ss>>name >> famName;
	cout <<"name: " <<name << " and "<<"famName: "<<famName <<".\n";
        // name and famName have  no values assigned. why?       

	}	
return 0;
}

ps: There is no compiling error. Only that name and famName have no values assigned at the end.

[Gast]

Just to clarify:

1. There is a mapfile given just like that:

1 snp1 0 1
3 snp2 0 2
2 snp3 1 1
2 snp4 2 2
3 snp5 1 2

2. You want to check wheather the mapfile is built correctly.

3. You want to read the mapfile and store its content into CSNP-Objects.

4. You want to output all the CSNP-Objects.

Is this what you're trying to do or are we tearing past each other?

[euklid]

Gugelmoser schrieb:

Just to clarify:
1. There is a mapfile given just like that:

1 snp1 0 1
3 snp2 0 2
2 snp3 1 1
2 snp4 2 2
3 snp5 1 2

2. You want to check wheather the mapfile is built correctly.

3. You want to read the mapfile and store its content into CSNP-Objects.

4. You want to output all the CSNP-Objects.

Is this what you're trying to do or are we tearing past each other?

Yes Gugelmoser, you are absolutely right.
[list]
[1] I have a bool parameter with default value false say

bool threeColumns =false

if threeColumns=true, the map file looks like

snp1 0 1
 snp2 0 2
 snp3 1 1
 snp4 2 2
 snp5 1 2

In this case I want that the default value of #nchr is for example 0.

[2]If threeColumns=false, then

1 snp1 0 1
3 snp2 0 2
2 snp3 1 1
2 snp4 2 2
3 snp5 1 2

[3] I want first check for the correct mapfile like I did before and then I want to save them as class a vector of class objects.
[4] The map file is very large at least 10000,000 rows.

int main()
{
  vector<CSNP> genInfo;
  {
    CSNP value;
    ifstream file("test.map");
    while (file >> value)
      genInfo.push_back(value);
  }

where each object CSPP looks like

class CSNP
{
private:
  int nchr;
  string snpName;
  string allele1;
  string allele2;
  friend istream& operator>>(istream& in, CSNP &csnp);
// this I was trying to define like before. 

 };

[Gast]

Well i was bored and that's the result (you're looking for)

#include <iostream>
#include <string>
#include <fstream>
#include <sstream>
#include <vector>
using namespace std;

/*
    mapfile:
        1 snp1 0 1
        3 snp2 0 2
        2 snp3 1 1
        2 snp4 2 2
        3 snp5 1 2
*/

class CSNP
{
    private:
        static bool three_columns; 
        int nchr;
        string snpName;
        string allele1;
        string allele2;

    public:
        static bool checkCorrectMapfile(const string&);
        CSNP() : nchr(0) {}
        friend istream& operator>>(istream& in, CSNP& csnp) 
        {
            if(!three_columns)
            {
                in>>csnp.nchr;
            }
            getline(in,csnp.snpName,' ');
            getline(in,csnp.allele1,' ');
            return getline(in,csnp.allele2);
        }
        friend ostream& operator<<(ostream &out, const CSNP &csnp)
        {
            return out << csnp.nchr << ' ' << csnp.snpName << ' ' << csnp.allele1 << ' ' << csnp.allele2;
        }      
};

bool CSNP::three_columns = false;
bool CSNP::checkCorrectMapfile(const string& mapfile)
{

    ifstream in( mapfile.c_str() );

    if(!in)
    {
        cerr << "could not open mapfile";
        return false;
    }
    else
    {
        vector<string> lines;
        string buffer;

        while( getline(in,buffer) )
        {
            lines.push_back(buffer);
        }

        for(vector<string>::iterator it=lines.begin(); it!=lines.end(); ++it)
        {
            istringstream iss(*it);
            vector<string> tokens;

            while( getline(iss,buffer,' ') )
            {
                tokens.push_back(buffer);
            }

            if(tokens.size() != 3 && tokens.size() != 4)
            {
                cerr << "your mapfile was expected to have either 3 or 4 columns!";
                return false;
            }

            if( !three_columns && tokens.size() == 3)
            {
                three_columns = true;
            }
        }
    }
    cout << "your mapfile is correct!";
    return true;
}

int main()
{
    string mapfile = "...";

    if( CSNP::checkCorrectMapfile(mapfile) )
    {
        ifstream in( mapfile.c_str() );
        vector<CSNP> objects;

        for(CSNP csnp; in >> csnp;)
        {
            objects.push_back(csnp);
            cout << '\n' << csnp;
        }

        // now, every line is stored into a CSNP-Object.
    }

    return 0;
}

To be honest, i don't know wheater the design is or is not good.
Anyhow, reading the file only once... i didn't manage to get any good design for that :(.

[euklid]

thank you so much for your help. I have posted new code which is error free but not efficient.