Hello
I had the seg fault last night, after making some changes, I don't have it anymore, the program compiles but it doesn't do anything !
" the program should read in data from the file in this following form :
2
Dell inspiron 299
Hp pavilion 499
"
simply read in and the display ! thanks in advance

#include<iostream>
#include<fstream>

using namespace std;

   struct computer{
   char * brand ;
   char * model;
   double  price;
   };

    void readin(computer * , int );
    void printf(computer * , int );
    int main ()
  {
    computer * pc ;
    computer * temp;
    int size;
    char filename [30];
    ifstream fin;
    
    cout<< " enter the file name : "<<endl;
    cin>>filename;
    fin.open(filename);

    
    readin(pc, size);// call the read in function 

    for(int i = 0; i<size; i++)
    {
    printf(pc, size);// call to the print function to display the results 
    pc++;
    }
   // dealllocate the memory 
    delete [] pc;
    delete [] temp;
    
    system("pause");
    return 0;
}

// functions implementation 
    // reading function 
    void readin(computer * pc  , int size)
    {

    computer * temp;
    ifstream fin;
    
    fin>>size;
    
    pc = new computer[size];
    temp = pc;
    for (int i =0; i<size; i++)
    {
    (*temp).brand = new char [15];
    (*temp).model = new char [15];
    
    fin>>(*temp).brand>>(*temp).model>>(*temp).price;
    temp++;
    
    }
    fin.close();
    // set the pointer back to base 
    temp = pc;
    // deallocate the memory 
    for ( int i = 0; i<size; i++)
    {
    delete [] (*temp).brand;
    delete [] (*temp).model;
    temp++;
    
    }
    delete [] pc;
    delete [] temp;
    }
 // printing function 
    void printf(computer * pc, int size )
    {

    computer * temp;

    for(int i= 0; i<size; i++)
    {
    cout<<"brand:"<<(*temp).brand<<"model:"<<(*temp).model<<"price:"<<(*temp).price <<endl;
    temp++;
    }
    temp = pc;
    //decallocate the memory 
    for ( int i = 0; i<size; i++)
    {

    delete [] (*temp).brand;
    delete [] (*temp).model;
    temp++;
    
    }
    //set it back to base ;
    temp = pc;
    // decallocate the memory 
    delete [] pc;
    delete [] temp;


    }

2 things im seeing off the top is in your readin function after reading in the file data you delete the data in temp and than you delete the data in pc so you do all the work and then delete it all. second your printing function is named printf which is a function in C/C++ for printing data. does your compiler complain about that at all?

First thing, when you call reading function, you pass size variable without initializing. This will not create a problem but bad practice.

Second, in the function readin(), you are using ifstream fin but you are not opening any file to read contents.

void readin(computer * pc , int size)
{
        computer * temp;
        ifstream fin;
        fin>>size;
        pc = new computer[size];

First thing, when you call reading function, you pass size variable without initializing. This will not create a problem but bad practice.

Second, in the function readin(), you are using ifstream fin but you are not opening any file to read contents.

void readin(computer * pc , int size)
{
        computer * temp;
        ifstream fin;
        fin>>size;
        pc = new computer[size];

Hey thanks !
I opened the file in main () do you think I should open it up in the function ?

2 things im seeing off the top is in your readin function after reading in the file data you delete the data in temp and than you delete the data in pc so you do all the work and then delete it all. second your printing function is named printf which is a function in C/C++ for printing data. does your compiler complain about that at all?

Helllo
thanks for responding !
I changed it ( not to delete the data ) and still not working, also I changed the function name ...the program compiles and ask about the file name... that's all !

the ifstream fin on line 48 inside readin() is NOT the same as the stream of the same name that is declared on line 20 and opened on line 24 in main().

You either need to open the file in readin() or pass the stream from main into readin().

If your data file really looks like:

2
Dell inspiron 299
Hp pavilion 499

You will not be able to use a standard stream read for the name. The standard string read will stop at the first space. You will need to perform something like a line read (see getline()) and then parse the string you read to find the number at the end of the line.

If your data file really looks like:

You will not be able to use a standard stream read for the name. The standard string read will stop at the first space. You will need to perform something like a line read (see getline()) and then parse the string you read to find the number at the end of the line.

Hey thank you !
I did make the change as far as declaring the ifstream fin in the file, the program runs but its stops when it displays brand : !


I got the same program to do it all from main (), using just fin ! the problem is when I tried to break it down to functions !
any thoughts would be appreciate it

any chance you can post what you have now (in code tags) so I can look at it?

there you go ! :)

#include<iostream>
#include<fstream>

using namespace std;

   struct computer{
   char * brand ;
   char * model;
   double  price;
   };

    void readin(computer *& , int );
    void printF(computer *& , int );
    int main ()
  {
    computer * pc ;
    computer * temp;
    int size;
    char filename [30];
 
    
    cout<< " enter the file name : "<<endl;
    cin>>filename;
  
    readin(pc, size);// call the read in function 

    for(int i = 0; i<size; i++)
  {
    printF(pc, size);// call to the print function to display the results 
   pc++;
    }
   // dealllocate the memory 
    delete [] pc;
    delete [] temp;
    
    system("pause");
    return 0;
}

// functions implementation 
    // reading function 
    void readin(computer * &pc  , int size)
    {

    computer * temp;
    ifstream fin;
    char filename [30];
    fin.open(filename);
    
    fin>>size;
    
    pc = new computer[size];
    temp = pc;
    for (int i =0; i<size; i++)
    {
    (*temp).brand = new char [15];
    (*temp).model = new char [15];
    
    fin>>(*temp).brand>>(*temp).model>>(*temp).price;
    temp++;
    
    }
    fin.close();
    // set the pointer back to base 

    temp = pc;
    // deallocate the memory 
    for ( int i = 0; i<size; i++)
    {
    delete [] (*temp).brand;
    delete [] (*temp).model;
    temp++;
    }
    
    delete [] pc;
    delete [] temp;


    }
 // printing function 
    void printF(computer * &pc, int size )
    {

    computer * temp;

    for(int i= 0; i<size; i++)
    {
    cout<<"brand:"<<(*temp).brand<<"model:"<<(*temp).model<<"price:"<<(*temp).price <<endl;
    temp++;
    }
    temp = pc;
    //decallocate the memory 
    for ( int i = 0; i<size; i++)
    {

    delete [] (*temp).brand;
    delete [] (*temp).model;
    temp++;
    
    }
    //set it back to base ;
    temp = pc;
    // decallocate the memory 
    delete [] pc;
    delete [] temp;


    }

You appear to be having some trouble with scoping.

No function can see the local variables of another function unless that other function passes them as arguments.

The filename declared on line 19 is the one that main puts the name the user enters into. The filename declared on line 47 and used on line 48 will NOT contain the filename. So to start with, you aren't opening the right file in readin().

You could easily add another parameter for the filename readin(char * filename, computer * & pc, int size); Also, readin() if it reads the right file, reads in the size and main and printF expect to be able to use the size, but the size is local to readin(). If you want readin() to set the size and for main to see it, you have to pass the size by reference (like you did the computer array.) readin(char * filename, computer * & pc, int & size); To simplify the rest of the code, you could skip allocating character strings for the brand and the model by just declaring them with size in the structure:

struct computer{
   char brand[15];
   char model[15];
   double  price;
   };

(As a matter of personal preference, I like declared types to start with a capital letter and I like indenting so my declaration would look more like this:

struct Computer {
      char brand[15];
      char model[15];
      double  price;
   };

but it is your code.)

readin() just allocated the array of information and it should not delete anything...the data still needs to be around for main() to call the print function which should not be named printf.

(Yours is named printF and while that is in fact a different name to the compiler, I would generally frown upon two variables or functions with different purposes and/or arguments that differ in name only by changing the case of one of the letters. I would suggest something like printComputer(...) or printComputers(...) depending on whether it was supposed to print one or all of them.)

Speaking of the one or all of them point, main seems to be looping through all of the computers and calling printF() for each one, but printF() also appears to iterate through the list. Pick one of them to iterate and get rid of the loop in the other.

On line 17, main declares but does not initialize computer * temp on line 34 it is deleted without ever being initialized. (Do you have warnings on in your compiler? Using an uninitialized value is usually something it complains about.)

Try working on those and see what you end up with.

If you have questions, post them. If you get the code changed, post it again along with what it appears to be doing and what it still needs to do or do differently.

I made some changes and still can't get it to display :

#include<iostream>
#include<fstream>

using namespace std;

struct computer{
    char * brand;
    char * model;
    double  price;
};

void readin(computer *& , int &, char * );
void printF(computer * , int );

// main function

int main ()
{
    computer * pc ;
    computer * temp;
    int size;
	char * filename = new char[30];
    ifstream fin;
    
	// get the filename from user
    cout<< " enter the file name : "<<endl;
    cin>>filename;
  
    readin(pc, size, filename);// call the read in function 

    printF(pc, size);// call to the print function to display the results 
    
	temp = pc;
    // deallocate the memory 
    for ( int i = 0; i<size; i++)
    {
        delete [] (*temp).brand;
        delete [] (*temp).model;
        temp++;
    }

    delete [] pc;
   
    // deallocate the memory for filename string
	delete [] filename;
    // 
system("pause");
    return 0;
}

// functions implementation 
   
// reading function 
void readin(computer *& pc  , int& size, char * filename )
{

    computer * temp;
    ifstream fin;

    fin.open(filename);
    fin>>size;
    
    pc = new computer[size];
    temp = pc;
    for (int i =0; i<size; i++)
    {
        (*temp).brand = new char [15];
        (*temp).model = new char [15];
    
        fin>>(*temp).brand>>(*temp).model>>(*temp).price;

        temp++;
    
    }
    fin.close();
}

// printing function 
void printF(computer * pc, int size )
{

    computer * temp;

	// set temporary pointer
	temp = pc;
    for(int i = 0; i<size; i++)
    {
        cout<<"brand:"<<(*temp).brand<<"   model:"<<(*temp).model<<"   price:$"<<(*temp).price <<endl;
        temp++;
    }

}

At first glance, it looks pretty close to me.

So I loaded it up in Visual Studio, cobbled together a data file named "data." using notepad and it seems to run for me.

The data file contains:

2
Dell Inspiron 299
Hp Pavilion 499

When I run the program I get:

enter the file name :
data
brand:Dell   model:Inspiron   price:$299
brand:Hp   model:Pavilion   price:$499
Press any key to continue . . .

That looks right to me.

What are you seeing?

NOTE: I did explicitly over-ride the default working directory to the directory where the data file was created. Either do that or make sure your data file is in the correct directory or the program won't be able to find it. Alternatively, you could enter the full path to the data file, but you only allowed 30 characters so don't overflow it.

Edited 6 Years Ago by Murtan: notes about working directory

At first glance, it looks pretty close to me.

So I loaded it up in Visual Studio, cobbled together a data file named "data." using notepad and it seems to run for me.

The data file contains:

2
Dell Inspiron 299
Hp Pavilion 499

When I run the program I get:

enter the file name :
data
brand:Dell   model:Inspiron   price:$299
brand:Hp   model:Pavilion   price:$499
Press any key to continue . . .

That looks right to me.

What are you seeing?

NOTE: I did explicitly over-ride the default working directory to the directory where the data file was created. Either do that or make sure your data file is in the correct directory or the program won't be able to find it. Alternatively, you could enter the full path to the data file, but you only allowed 30 characters so don't overflow it.

Yea thanks man !
for some reasons when I run it on DevC++ it won't display the results ....but i ran it on linux and its worked perfectly !
so looking at the program, do you think I have any memory leaks or something that I need to work on ?
thanks again :)

I don't see anywhere that you are not releasing memory you have allocated.

I however, would not have used quite so much allocation. For example the filename is declared and used as follows. The buffer is explicitly allocated, filled, passed to another function and explicitly deallocated.

char * filename = new char[30];
   cin >> filename;
   readin(pc, size, filename);
   delete [] filename;

Using a stack-based local variable is simpler and does not require actual allocation and deallocation. The buffer is declared, filled, passed to another function and released when the function exits.

char filename[255];
   cin >> filename;
   readin(pc, size, filename);

You may have noticed that I 'sized up' the filename buffer. When you're working with user input, you should have more room available than the user is ever likely to provide. (Some input functions don't check to make sure you have enough.)

I would still argue that the brand and model character buffers should be explicitly sized as well. Your sample program only has 2 instances of the computer structure, but each time you use a computer structure, you have to perform 3 allocations. One for the structure and two more for the character buffers. If the character buffers are always going to be allocated at 15 characters in size, just go ahead and declare them that size. The first allocation would have to go get more memory, but you would not have to make the two additional allocations.

I would have declared it as:

struct computer{
    char brand[15];
    char model[15];
    double  price;
};

Though if I might have made the brand and model buffers a little bigger as well. Unless memory (or disk space as appropriate) is at a premium, I like to make sure that character buffers in a 'record' are larger than anything in the sample set. I would suspect a brand of size 15-20 is probably fine, but I might allow 20-25 for the model.

For long-term use, I might also make some changes to the data file. Presently, all of the data is "space delimited", meaning that the data fields are seperated by spaces. The system breaks down if the brand name or model name needs to contain a space. You could replace spaces with punctuation (like a hyphen '-' or underscore '_') but could not support an actual space in either name.

This article has been dead for over six months. Start a new discussion instead.