qn regarding file closure problem

[unregistered]

ok my lecturer is away and I need some help so here goes:

Q1) in the constructArray() function, why is it that I have to do file.clear() ( after the reading part ) before I can file.close() successfully, what caused the file stream to be in an error state and how do I solve this?

Q2) another thing is that when I try to pass a character constant to an array of a function, I get a compiler warning as below, since this conversion is deprecated, what would be the non-deprecated alternative to it?
lab10.cpp: In function 'int main()':
lab10.cpp:85: warning: deprecated conversion from string constant to 'char*'
lab10.cpp:86: warning: deprecated conversion from string constant to 'char*'
lab10.cpp:93: warning: deprecated conversion from string constant to 'char*'
...

#include<iostream>
#include<iomanip>
#include<fstream>
#include<ctime>
#include<cstdlib>

//global declarations
using namespace std;

//constants
const int MAX = 100;
const int WIDTH = 20;

//user defined types
enum NumType { Odd, Even };

//structure definitions
struct Number
{
    int no;
    NumType type;
    int oddDigits;
    int evenDigits;
    int sumDigits;
    int noDigits;
};

//function prototypes
void constructInfile( fstream&, char [] );
int constructArray( fstream&, char [], Number [] );
void processArray( Number [], int );
void arrayToOutfile( fstream&, char [], Number [], int );
void numProperties( int, NumType&, int&, int&, int&, int& );
void checkFileStream( fstream&, char [], char [] );

//constants


int main()
{
    //local declarations
    fstream file;
    Number array[MAX];
    int size;

    //statements
    srand( time( NULL ) );

    //input
    constructInfile( file, "infile.txt" );
    size = constructArray( file, "infile.txt", array );

    //computations
    processArray( array, size );


    //output
    arrayToOutfile( file, "outfile.txt", array, size );


    return 0;
}

void constructInfile( fstream& file, char fileName[] )
{
    //local declarations
    int noOfEntries = rand() % MAX + 1;

    //statements
    cout << "Attempting to create file: " << fileName << "...";
    cout << endl;

    file.open( fileName, ios::out );

    checkFileStream( file, "create", fileName );

    for ( int idx = 0; idx < noOfEntries; idx++ )
        file << rand() << endl;

    file.close();

    return;
}

int constructArray( fstream& file, char fileName[], Number array[] )
{
    //local declarations
    int size = 0,
        temp;

    cout << "Attempting to open file: " << fileName << "...";
    cout << endl;

    file.open( fileName, ios::in );

    checkFileStream( file, "open", fileName );

    while ( file >> temp )
    {
        array[size].no = temp;
        size++;
    }

    if ( file.eof() )
        cout << "Data read successfully" << endl;
    else
        cerr << "Reading failed" << endl;

    file.clear();
    file.close();

    checkFileStream( file, "close", fileName );

    return size;
}

void processArray( Number array[], int size )
{
    for ( int idx = 0; idx < size; idx++ )
    {
        numProperties( array[idx].no,
                   array[idx].type,
                   array[idx].oddDigits,
                   array[idx].evenDigits,
                   array[idx].sumDigits,
                   array[idx].noDigits
                 );
    }

    return;

}

void numProperties( int num, NumType& oddEven, int& oddCount, int& evenCount, int& sumDigits, int& noDigits )
{
    int temp;

    oddCount = evenCount = sumDigits = noDigits = 0;

    if ( num % 2 )
        oddEven = Odd;
    else
        oddEven = Even;


    while ( num > 0 )
    {
        temp = num % 10;

        sumDigits += temp;
        if ( temp % 2 )
            oddCount++;
        else
            evenCount++;
        noDigits++;

        num /= 10;
    }

    return;
}

void arrayToOutfile( fstream& file, char fileName[], Number array[], int size )
{
    cout << "Attempting to create file: " << fileName << "...";
    cout << endl;

    file.open( fileName, ios::out );

    checkFileStream( file, "create", fileName );

    file << left;

    file << setw(WIDTH) << "No";
    file << setw(WIDTH) << "Type";
    file << setw(WIDTH) << "Odd";
    file << setw(WIDTH) << "Even";
    file << setw(WIDTH) << "Sum";
    file << setw(WIDTH) << "Digit";
    file << endl;

    for ( int idx = 0; idx < size; idx++ )
    {
        file << setw(WIDTH) << array[idx].no;

        switch ( array[idx].type )
        {
            case Odd: file << setw(WIDTH) << "Odd";
                  break;
            case Even: file << setw(WIDTH) << "Even";
                   break;
        }

        file << setw(WIDTH) << array[idx].oddDigits;
        file << setw(WIDTH) << array[idx].evenDigits;
        file << setw(WIDTH) << array[idx].sumDigits;
        file << setw(WIDTH) << array[idx].noDigits;
        file << endl;
    }

    file.close();

    checkFileStream( file, "close", fileName );

    return;

}

void checkFileStream( fstream& file, char operation[], char fileName[] )
{
    if ( file.fail() )
        cerr << "failed to " << operation << " " << fileName << endl;
    else
        cout << operation << " " << fileName << " successful" << endl;


    return;
}

[jsmith]

The loop:

  while ( file >> temp )
    {
        array[size].no = temp;
        size++;
    }

essentially loops until the stream fails to input, correct?

When that happens, the stream is put into an error state.

What do you know about stream operations on a stream in an error state?

[unregistered]

when stream is in an error state, no more input can be input until the state is restored to a working state, I do not know about how this affects file closure though, care to share some light?
so how do I make it such that this won't happen?

[jsmith]

I'm not sure if the stream will refuse to close the file for the same reason or not. My first impression was that since you were asking it did, but on second thought the destructor has to close the file regardless, so I'm not sure, and I don't want to tell you wrong.

In general, in writing robust programs, it is impossible (or at least bad practice) to write code under the assumption that no errors can occur while reading the file. Therefore, I would suggest that you focus not on avoiding the problem, but rather on recovery when the problem occurs.

[unregistered]

Closure of file failed when I didn't include the clear() originally, it was just a hypothesis on my part that the file failed to close if the stream is in an error state that turned out to be right because file closed successfully upon my addition of clear().

So in other words, my current code is alright?
what are the ways I can optimize it and what are the bad habits I need to improve on?
oh and what abt the deprecated string conversions ( see qn 2 ) ?

[helios]

The file had already been closed once without a call to clear() after close(). Maybe that's the problem. Although it is odd that closing would fail instead of opening.


Like OP, I would also like to know about his second question. Only GCC complains when I pass string literals to functions asking for 'char *'s. If that method is "deprecated", what is the new method?

[jsmith]

That's a good question. String literals are de-facto const char* since the characters referred to are typically embedded in a read-only page (ie, the code segment).

I would say the function's prototype needs to change to take an array of const chars.

[helios]

I think I'd prefer to write a macro:
#define STRLITERAL(x) ((char*)(x))

[jsmith]

It could be dangerous. Since the function is declared to take an array of chars, there is no API guarantee that the function will not try to modify the contents of the array which will bomb out (seg fault, gpf, etc).

Otherwise I would have suggested the same thing except I would have suggested using const_cast<char*>( "literal" ). [I use the C++ casts as a form of self-punishment to remind myself to write code that doesn't require casts in the first place.]

[helios]

I know. However, if I'm knowingly passing a literal in the first place it's because it's a function I wrote and I know that particular parameter is not modified.
Why didn't I declare the parameter as const in the first place, you ask? Because const is one of the few things I hate about C++. I hate even more having to write for loops like this one, though:
for (std::map<wchar_t *,NONS_Variable *,wstrCmpCI>::iterator i=this->variables.begin();i!=this->variables.end();i++){/*...*/}
But unlike const, that is inevitable (no, I refuse to write yet another typedef purely on principle).

[jsmith]

Sure, though it is a bit unsafe in that I can cast any type whatsoever to a char* without complaint from the compiler. (Just thinking in a large app with many developers, I know there will be people who try to abuse it).

I think const correctness is too important to ignore, though I agree with you that it is annoying to have to think about it and type it everywhere.

I almost invariably create typedefs for my containers when the scope of the container is greater than a local function. If I've coded everything generically enough, then I can completely change out the underlying container type with little or no code change elsewhere. This reduces a bit the length of the iterator type in the for loop.

Though you could use std::for_each() with a functor or with a lambda expression (from boost, though there is a steep learning curve for even the simplest of lambda expressions).

[unregistered]

How can I make it such that in the following piece of code, file >> array[size].no occurs inside the loop and the loop exits when there are no more data to be read?

while ( file >> temp )
    {
        array[size].no = temp;
        size++;
    }

[firedraco]

Make the while loop while file.good() perhaps (in case other errors occur).

[unregistered]

so it becomes like this?
ok but how can I make it such that the file stream does not go into an error state and go about adjusting the while loop such that it works?

while ( file.good() )
    {
        file >> array[size].no;
        size++;
    }

[jsmith]

I'm not sure you can without knowing a priori how many elements you are going to read from the file.

Your code above does not quite work: if file >> array[size].no fails, then you probably shouldn't increment size since you didn't actually read an element.