1

My problem is how to use correctly the function infile.open().

I have a class that, among the others, has the following public properties:

class myclass {
public:
int rows
int columns
const char* array_file
}

All values are given at run-time.

When I call the function that uses a member of the class I have (pt is a pointer to a member of the class)

#include <vector>
#include <fstream>
#include <iostream>
typedef std::vector< std::vector<int> > Matrixint;

void function(myclass* pt) { 
    Matrixint array_name(pt->rows, std::vector<int>(pt->columns));

    std::ifstream infile;

    infile.open("%s", pt->array_file); // my problem: is this correct?
    for (int a = 0; a < pt->rows; a++) {
         for (int b = 0; b < pt->columns; b++) {
               infile >> array_name[a][b] ;
         }
    }
    infile.close();

}

Is this way of opening/reading the file correct?

The data in the file will be formatted as in this question (please note: only the array will be present in the file, no other data)

Community
  • 1
  • 1
Federico
  • 1,092
  • 3
  • 16
  • 36
  • Do not confuse C with C++ (and vice versa). In C++'s standard libraries you do not use formatted printing. You should also use std::string instead of a bare pointer to `char`s. – cschwan Aug 01 '11 at 07:32
  • @cschwan : I am definitely not an expert in neither of the languages, could you expand your comment a bit, please? Thanks – Federico Aug 01 '11 at 07:35
  • @cschwan: "In C++'s standard libraries you do not use formatted printing." Huh? – Lightness Races in Orbit Aug 01 '11 at 14:41
  • Sorry for expressing myself not clearly. What I wanted to say is that in C++ you do not use printing with _format specifier_. For example, in C you would write `printf("foo %d", 42)`, in C++ you use `std::cout << "foo " << 42` instead. Thats the programming behavior "dictated" with C++'s standard libraries, but of course that does not mean you cannot use `printf` and friends in C++. – cschwan Aug 03 '11 at 13:16

2 Answers2

3

infile.open gets as its first parameter the filename:

void open ( const char * filename, ios_base::openmode mode = ios_base::in );

(source)

I don't know what your filename is but maybe something like this (just a guess based on variable types) could do:

infile.open(pt->array_file); 

of course you have to ensure that the filename you pass in is correct at the time of calling that function.

sergio
  • 68,819
  • 11
  • 102
  • 123
  • sorry for bothering you again. I verify that the filename is correct by printing it on the screen and it perfectly matches, but the file fails to open. Do you have any suggestions? I pass the file with an extension (.out), should I use a particular extension or it should not matter? Thanks, Federico – Federico Aug 02 '11 at 08:49
  • nevermind. what is the filename you print on screen? at which path is located the real file in you filesystem? `pt->array_file` will be resolved relative to the running directory, so you might take this into account... – sergio Aug 02 '11 at 09:03
2

Assuming that my psychic abilities for fixing your question's code were right, I'd write it like this:

struct mine {
    int rows
    int columns
    std::string array_file
}

void function(const mine& m) { 
    Matrixint array_name(pt->rows, std::vector<int>(pt->columns));

    std::ifstream infile(m.array_file.c_str());

    for (int a = 0; a < ls->rows && infile.good(); ++a) {
         for (int b = 0; b < ls->columns && infile.good(); ++b) {
               infile >> array_name[a][b] ;
         }
    }

    if(!infile) 
        throw "Uh oh!"; // assume real error handling here
}

Why have I changed all these things?

  • A class with all public data isn't a class, but an aggregation of data. I'd use a struct for that, to not to confuse those who later need to maintain my code. (That might include me a few years down the road, which is a strong incentive to be very helpful.)
  • Unless you know exactly what you're doing (which doesn't seem to be the case), you should be using std::string rather than C-style strings.
  • Why pass the function parameter by pointer, when you can use a const reference?
  • std::ifstream has a constructor with which you can open a file immediately. I rarely ever use (or see used) its open() member function.
  • You need to test whether the file was opened and whether the input operations succeeded. (In this case, I merged the test whether it could be opened with the input operation succeed test, since the for loop is a pre-testing loop.) After the operations, I check whether an error occurred. Alternatively you could break out of the loop with an exception when reading fails.
  • Usually there is no need to close a stream, since its destructor already does this. If you make your variables as local as possible (which is good programming praxis), the file will be closed right away nevertheless.
Community
  • 1
  • 1
sbi
  • 219,715
  • 46
  • 258
  • 445
  • Thanks for the reply. A few comments upon your comments: - yes, I am new to C++ programming - what was not clear of "among the others"? The class has also private data, but not relevant for the question - the original code is not mine, I'm expanding it, using the same structure imposed by the original programmer - I'm pretty sure of how the data will be formatted, so there is no need of testing whether I am reading beyond the end of the line/file. I agree there could be the need of testing if the file opened, what could cause this? Thanks Federico – Federico Aug 01 '11 at 09:15
  • @Federico: I simply overlooked the "among the others". Otherwise it's pretty clear. If you want your app to silently fail just because some jerk edited a file he shouldn't have touched (or for any number of other, equally silly reasons), you don't need to check input operations. Otherwise you should. One common but often overlooked causes for failing to open a file is missing access rights, but many others exist. – sbi Aug 03 '11 at 13:53