-2

The error message

I have a file with 9 words and i have to store each word into the char array of 9 pointers but i keep getting an error message. I cannot use vectors!

#include <iostream>
#include <fstream>


using namespace std;


int main()
{
   char *words[9];
   ifstream inStream;
   inStream.open("sentence.txt");
   if (inStream.fail())
   {
       cout << "Input file opening failed.\n";
       exit(1);
   }

   for ( int i = 0; i < 10; i++)
   {
       inStream >> words[i];
   }

       inStream.close();

   return 0;
}
8
  • 1
    using namespace std is bad - also why are you not using std::string? Commented Feb 9, 2017 at 19:26
  • 1
    And the error message is??? Commented Feb 9, 2017 at 19:26
  • Also check if you have opened the file Commented Feb 9, 2017 at 19:27
  • 1
    And why closing the stream is a good idea in the loop? Commented Feb 9, 2017 at 19:27
  • 4
    Enter the error message as text. It's not searchable as an image. In fact, it's barely legible. Commented Feb 9, 2017 at 19:51

3 Answers 3

2

The declaration

char *words[9];

declares a raw array of pointers. This array is not initialized so the pointers have indeterminate values. Using any of them would be Undefined Behavior.

Instead you want

vector<string> words;

where vector is std::vector from the <vector> header, and string is std::string from the <string> header.

Use the push_back member function to add strings to the end of the vector.

Also you need to move the close call out of the loop. Otherwise it will close the file in the first iteration.

This approach gives the code (off the cuff, disclaimer...)

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

int main()
{
   vector<string> words;
   ifstream inStream;
   inStream.open("sentence.txt");

   for ( int i = 0; i < 10; i++)
   {
       string word;
       if( inStream >> word )
          words.push_back( word );
   }
   inStream.close();
}

If you can't use std::string and std::vector then you need to initialize the array of pointers, and make sure that you don't read more into the buffers than there's room for.

The main problem here is that >> is unsafe for reading into a raw array given by a pointer. It doesn't know how large that array is. It can easily lead to a buffer overrun, with dire consequences.

And so this gets a bit complicated, but it can look like this:

#include <ctype.h>          // isspace
#include <fstream>
#include <iostream>
#include <locale.h>         // setlocale, LC_ALL
#include <stdlib.h>         // EXIT_FAILURE
using namespace std;

void fail( char const* const message )
{
    cerr << "! " << message << "\n";
    exit( EXIT_FAILURE );
}

void readWordFrom( istream& stream, char* const p_buffer, int const buffer_size )
{
    int charCode;

    // Skip whitespace:
    while( (charCode = stream.get()) != EOF and isspace( charCode ) ) {}

    int n_read = 0;
    char* p = p_buffer;
    while( n_read < buffer_size - 1 and charCode != EOF and not isspace( charCode ) )
    {
        *p = charCode;  ++p;
        ++n_read;
        charCode = stream.get();
    }
    *p = '\0';      // Terminating null-byte.

    if( charCode != EOF )
    {
        stream.putback( charCode );
        if( not isspace( charCode ) )
        {
            assert( n_read == buffer_size - 1 );    // We exceeded buffer size.
            stream.setstate( ios::failbit );
        }
    }
}

int main()
{
    static int const n_words            = 9;
    static int const max_word_length    = 80;
    static int const buffer_size        = max_word_length + 1;  // For end byte.

    char *words[n_words];
    for( auto& p_word : words ) { p_word = new char[buffer_size]; }

    ifstream inStream{ "sentence.txt" };
    if( inStream.fail() ) { fail( "Input file opening failed." ); }

    setlocale( LC_ALL, "" );            // Pedantically necessary for `isspace`.
    for( auto const p_word : words )
    {
        readWordFrom( inStream, p_word, buffer_size );
        if( inStream.fail() ) { fail( "Reading a word failed." ); }
    }

    for( auto const p_word : words ) { cout << p_word << "\n"; }

    for( auto const p_word : words ) { delete[] p_word; }
}
Sign up to request clarification or add additional context in comments.

5 Comments

Perhaps mention not to close the file in the loop and also check that the file is open. That would add to the answer
@EdHeal: Thanks will do. But first I have some hot potatoes to handle. On the oven. :)
I cannot use vectors for this assignment
how would i initialize the values of the array?
@BrendaGonzalez: I added that to the answer. Enjoy. Or, well, it's really a PITA having to do things at this level. Bjarne Stroustrup, the language creator, wrote a whole article about it. It's available as a free PDF. Unfortunately I don't remember the name.
1

You never allocate any memory for your char* pointers kept in the array.

The idiomatic way to write a c++ code would be:

#include <iostream>
#include <fstream>
#include <vector>

int main() {
   std::vector<std::string> words(9);
   std::ifstream inStream;
   inStream.open("sentence.txt");

   for ( int i = 0; inStream && i < 9; i++) {
       inStream >> words[i];
   }
}

The inStream.close() isn't necessary, and even wrong inside the loop. The std::istream will be closed automatically as soon the variable goes out of scope.

2 Comments

Perhaps check if the file has been opened would help
@EdHeal Added the condition ;-)
1

There are a few problems with your code.

char *words[9]; This allocates space for 9 pointers, not nine strings. Since you don't know how big the strings are you have two choices. You can either "guess" how much you'll need and limit the inputs accordingly, or you can use dynamic memory allocation (malloc or new) to create the space you need to store the strings. Dynamic memory would be my choice.

for ( int i = 0; i < 10; i++) This loop will execute on words[0] through words[9]. However, there is no words[9] (that would be the tenth word) so you'll overwrite memory that you have not allocated

inStream >> words[i]; This will send your input stream to memory that you don't "own". You need to allocate space for the words to live before capturing them from the input stream. To do this correctly, you'll need to know how much space each word will need so you can allocate it.

you could try something like this:

int main()
{
    char *words[9];
    char tempInput[256]; // space to capture the input, up to a maximum size of 256 chars
    ifstream inStream;
    inStream.open("sentence.txt");
    if (inStream.fail())
    {
        cout << "Input file opening failed.\n";
        exit(1);
    }

    for ( int i = 0; i < 9; i++)
    {
        //Clear the input buffer
        memset(tempInput, 0, 256);

        //Capture the next word
        inStream >> tempInput;

        //allocate space to save the word
        words[i] = new char(strlen(tempInput));

        //Copy the word to its final location
        strcpy(words[i], tempInput)
    }

    inStream.close();

    return 0;
}

1 Comment

@BrendaGonzalez memory allocation is performed by the new operator. Read about it in your favorite C++ reference.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.