views:

108

answers:

6

Hi,

I am working on sorting several large files in C++. I have a text file containing the names of all the input files, one on each line. I would like to read the file names in one at a time, store them in an array, and then create a file with each of those names. Right now, I am using fopen and fread, which require character arrays (I am trying to optimize for speed), so my filenames are read into an array of character arrays. Those arrays, however, need to have a maximum size fixed in advance, so if the filename is smaller than the maximum, the rest is full of garbage. Then, when I try to use that array as the filename in fopen(), it doesn't recognize the file because it has garbage at the end of the string. How can I solve this problem? Here is my code:

 #include <iostream>
#include <fstream>
#include <string>
#include "stdafx.h"
#define NUM_INPUT_FILES 4

using namespace std;



FILE *fp;
unsigned char *buff;
FILE *inputFiles[NUM_INPUT_FILES];


int _tmain(int argc, _TCHAR* argv[])
{


    buff = (unsigned char *) malloc(2048);
    char j[8];
    char outputstring[] = "Feelings are not supposed to be logical. Dangerous is the man who has rationalized his emotions. (David Borenstein)";

    fp = fopen("hello.txt", "r");

    string tempfname[NUM_INPUT_FILES];
    //fp = fopen("hello.txt", "r");
    for(int i=0;i<NUM_INPUT_FILES;i++)
    {
        fgets(tempfname[i], 20, fp);
        cout << tempfname[i];
    }
    fclose(fp);

    for(int i=0; i<NUM_INPUT_FILES;i++)
    {
        fp = fopen(tempfname[i], "w");
        //fwrite(outputstring, sizeof(char), sizeof outputstring/sizeof(char), fp);
        if(fp)
        {
            fclose(fp);}
        else
            cout << "sorry" << endl;
    }


    return 0;
}

Also, how do I find the size of a buffer to write it out with fwrite()?

Thank you very much, bsg

+5  A: 

As Don Knuth said, premature optimization is the root of all evil.

Your filenames are definitely not the bottleneck! Just use std::string for them.

You'd need to replace fp = fopen(tempfname[i], "w"); with fp = fopen(tempfname[i].c_str(), "w"); however.

Vlad
+1  A: 

you are using C type idioms, it would be better if you go google file handling in C++. which is a bit strange to start with if you're a C programmer, but its definitely worth the effort to work out how to do thing the C++ way

Keith Nicholas
+2  A: 

Forget optomizing at this stage.
Use std::vector<std::string> and get your program working. Once it is working, if speed is really that crucial then you can go back and change it

hamishmcn
A: 

If you read the files one line at a time, you can then allocate only the amount of space for each line that is needed and build your array of lines up in that way.

I can understand that this may not be fast enough for you, so as an alternative. may I suggest

  1. get the size of the file
  2. allocate a buffer of that size
  3. read the entire file into the buffer.
  4. scan the buffer replacing \r and \n with \0 and storing the start of each line in a vector of type char*
John Knoeller
+1  A: 

You need to add a null byte and strip the new line, so write a for loop within your first for loop that searches for the newline and replaces it with a null byte.

Although the others are right that you are seriously misguided in your optimization attempts.

And make sure you're freeing what you malloc. Another good reason why you should be using the STL.

FranticPedantic
A: 

I'm with everyone else here, this is premature optimization.

I don't see how fgets(tempfname[i], 20, fp); could compile, much less work, since tempfname[i] is a string& and fgets requires a char*.

Probably you want

typedef char file_name[20]; // way too short
file_name tempfnames[NUM_INPUT_FILES];

Although, among many other changes I would make here, you could entirely handle a file on each loop iteration and avoid having an array of names entirely.

Potatoswatter