1

The current_name is a local char array inside the following loop. I declared it inside the loop so it changes every time I read a new line from a file. But, for some reason the previous data is not removed from the current_name! It prints old data out if it wasn't overridden by new characters from the next line.

ANY IDEAS?

  while (isOpen && !file.eof()) {
    char current_line[LINE];
    char current_name[NAME];

    file.getline(current_line, LINE);

    int i = 0;
    while (current_line[i] != ';') {
      current_name[i] = current_line[i];
      i++;
    }

    cout << current_name << endl;
  }
Cory Kramer
  • 98,167
  • 13
  • 130
  • 181
Mosedince
  • 23
  • 3
  • 1
    Nothing abnormal. Nothing in the standard states that a block local variable **has to** change value at each iteration. It's only that you **cannot assume** that it will keep its value. – A.S.H Nov 11 '15 at 23:03
  • Local variables are not initialized automatically, so accessing it before assigning it results in undefined behavior. – Barmar Nov 11 '15 at 23:04
  • 4
    [why `while(!file.eof())` is always wrong](http://stackoverflow.com/questions/5605125/why-is-iostreameof-inside-a-loop-condition-considered-wrong) – Barmar Nov 11 '15 at 23:05
  • @A.S.H What do you mean? – Mosedince Nov 11 '15 at 23:05
  • 2
    Why does it matter if the previous data is removed? You're overwriting it with the data from the current line. The problem is that you're not adding a null terminator. – Barmar Nov 11 '15 at 23:06
  • 1
    It's like a new variable at each iteration. So as @Barmar said, if you read it before writing to it (within a new iteration), its data is *undefined*. – A.S.H Nov 11 '15 at 23:07
  • Why don't you use `std::string` instead of a `char` array? They are automatically initialized to an empty string. – Barmar Nov 11 '15 at 23:08
  • Would you show what you are talking about by showing some code. I am not getting what you mean. And to answer your question, the previous data needs to be removed because I compare it later with another array to see if there is a match. (THIS IS A SIMPLIFIED SNIPPETS FROM MY ORIGINAL CODE) – Mosedince Nov 11 '15 at 23:11
  • @Barmar I am not accessing anything before assigning!! What are you talking about?!! – Mosedince Nov 11 '15 at 23:14
  • How did you determine that the old contents are repeating if you didn't access them? – Barmar Nov 11 '15 at 23:15
  • @Mosedince You didn't initialize any of the arrays though. Also, it doesn't need to remove data, nowhere in the standard says that. And, as fellow commenters said, accessing uninitialized variables is undefined behavior - which means - anything can happen. – Algirdas Preidžius Nov 11 '15 at 23:16
  • @Barmar I am printing the result each time at the end of the loop!! And the result shows previous data! – Mosedince Nov 11 '15 at 23:16
  • Dude look at your question, we were answering your question: `But, for some reason the previous data is not removed from the current_name! It prints old data out if it wasn't overridden by new characters from the next line.` – A.S.H Nov 11 '15 at 23:18
  • If you're only assigning to elements up to `i=5`, but you print elements past that, then you're accessing elements that you didn't assign. – Barmar Nov 11 '15 at 23:18
  • @A.S.H and that exactly what happened! How do you want me to ask? – Mosedince Nov 11 '15 at 23:30
  • @Mosedince take it easy :). You asked a question very well, and we found that it is nothing abnormal and we tried to explain why we dont find it abnormal. Good to know that your issue was solved. Cheers. – A.S.H Nov 11 '15 at 23:32

2 Answers2

3

You're not terminating current_name after filling it. Add current_name[i] = 0 after the inner loop just before your cout. You're probably seeing this if you read abcdef then read jkl and probably get jkldef for output


UPDATE

You wanted to know if there is a better way. There is--and we'll get to it. But, coming from Java, your question and followup identified some larger issues that I believe you should be aware of. Be careful what you wish for--you may actually get it [and more] :-). All of the following is based on love ...


Attention All Java Programmers! Welcome to "A Brave New World"!


Basic Concepts

Before we even get to C the language, we need to talk about a few concepts first.

Computer Architecture:
https://en.wikipedia.org/wiki/Computer_architecture
https://en.wikipedia.org/wiki/Instruction_set

Memory Layout of Computer Programs:
http://www.geeksforgeeks.org/memory-layout-of-c-program/

Differences between Memory Addresses/Pointers and Java References:
Is Java "pass-by-reference" or "pass-by-value"?
https://softwareengineering.stackexchange.com/questions/141834/how-is-a-java-reference-different-from-a-c-pointer


Concepts Alien to Java Programmers

The C language gives you direct access the underlying computer architecture. It will not do anything that you don't explicitly specify. Herein, I'm mentioning C [for brevity] but what I'm really talking about is a combination of the memory layout and the computer architecture.

  • If you read memory that you didn't initialize, you will see seemingly random data.
  • If you allocate something from the heap, you must explicitly free it. It doesn't magically get marked for deletion by a garbage collector when it "goes out of scope".
  • There is no garbage collector in C
  • C pointers are far more powerful that Java references. You can add and subtract values to pointers. You can subtract two pointers and use the difference as an index value. You can loop through an array without using index variables--you just deference a pointer and increment the pointer.
  • The data of automatic variables in Java are stored in the heap. Each variable requires a separate heap allocation. This is slow and time consuming.
  • In C, the data of automatic variables in stored in the stack frame. The stack frame is a contiguous area of bytes. To allocate space for the stack frame, C simply subtracts the desired size from the stack pointer [hardware register]. The size of the stack frame is the sum of all variables within a given function's scope, regardless of whether they're declared inside a loop inside the function.
  • Its initial value depends upon what previous function used that area for and what byte values it stored there. Thus, if main calls function fnca, it will fill the stack with whatever data. If then main calls fncb it will see fnca's values, which are semi-random as far as fncb is concerned. Both fnca and fncb must initialize stack variables before they are used.
  • Declaration of a C variable without an initializer clause does not initialize the variable. For the bss area, it will be zero. For a stack variable, you must do that explicitly.
  • There is no range checking of array indexes in C [or pointers to arrays or array elements for that matter]. If you write beyond the defined area, you will write into whatever has been mapped/linked into the memory region next. For example, if you have a memory area: int x[10]; int y; and you [inadvertently] write to x[10] [one beyond the end] you will corrupt y
  • This is true regardless of which memory section (e.g. data, bss, heap, or stack) your array is in.
  • C has no concept of a string. When people talk about a "c string" what they're really talking about is a char array that has an "end of string" (aka EOS) sentinel character at the end of the useful data. The "standard" EOS char is almost universally defined as 0x00 [since ~1970]
  • The only intrinsic types supported by an architecture are: char, short, int, long/pointer, long long, and float/double. There may be some others on a given arch, but that's the usual list. Everything else (e.g. a class or struct is "built up" by the compiler as a convenience to the programmer from the arch intrinsic types)

Here are some things that are about C [and C++]:
- C has preprocessor macros. Java has no concept of macros. Preprocessor macros can be thought of as a crude form of metaprogramming.
- C has inline functions. They look just like regular functions, but the compiler will attempt to insert their code directly into any function that calls one. This is handy if the function is cleanly defined but small (e.g. a few lines). It saves the overhead of actually calling the function.


Examples

Here are several versions of your original program as an example:

// myfnc1 -- original
void
myfnc1(void)
{
    istream file;

    while (isOpen && !file.eof()) {
        char current_line[LINE];
        char current_name[NAME];

        file.getline(current_line, LINE);

        int i = 0;

        while (current_line[i] != ';') {
            current_name[i] = current_line[i];
            i++;
        }

        current_name[i] = 0;

        cout << current_name << endl;
    }
}

// myfnc2 -- moved definitions to function scope
void
myfnc2(void)
{
    istream file;
    int i;
    char current_line[LINE];
    char current_name[NAME];

    while (isOpen && !file.eof()) {
        file.getline(current_line, LINE);

        i = 0;

        while (current_line[i] != ';') {
            current_name[i] = current_line[i];
            i++;
        }

        current_name[i] = 0;

        cout << current_name << endl;
    }
}

// myfnc3 -- converted to for loop
void
myfnc(void)
{
    istream file;
    int i;
    char current_line[LINE];
    char current_name[NAME];

    while (isOpen && !file.eof()) {
        file.getline(current_line, LINE);

        for (i = 0;  current_line[i] != ';';  ++i)
            current_name[i] = current_line[i];
        current_name[i] = 0;

        cout << current_name << endl;
    }
}

// myfnc4 -- converted to use pointers
void
myfnc4(void)
{
    istream file;
    const char *line;
    char *name;
    char current_line[LINE];
    char current_name[NAME];

    while (isOpen && !file.eof()) {
        file.getline(current_line, LINE);

        name = current_name;
        for (line = current_line;  *line != ';';  ++line, ++name)
            *name = *line;
        *name = 0;

        cout << current_name << endl;
    }
}

// myfnc5 -- more efficient use of pointers
void
myfnc5(void)
{
    istream file;
    const char *line;
    char *name;
    int chr;
    char current_line[LINE];
    char current_name[NAME];

    while (isOpen && !file.eof()) {
        file.getline(current_line, LINE);

        name = current_name;
        line = current_line;
        for (chr = *line++;  chr != ';';  chr = *line++, ++name)
            *name = chr;
        *name = 0;

        cout << current_name << endl;
    }
}

// myfnc6 -- fixes bug if line has no semicolon
void
myfnc6(void)
{
    istream file;
    const char *line;
    char *name;
    int chr;
    char current_line[LINE];
    char current_name[NAME];

    while (isOpen && !file.eof()) {
        file.getline(current_line, LINE);

        name = current_name;
        line = current_line;
        for (chr = *line++;  chr != 0;  chr = *line++, ++name) {
            if (chr == ';')
                break;
            *name = chr;
        }
        *name = 0;

        cout << current_name << endl;
    }
}

// myfnc7 -- recoded to use "smart" string
void
myfnc7(void)
{
    istream file;
    const char *line;
    char *name;
    int chr;
    char current_line[LINE];
    xstr_t current_name;
    xstr_t *name;

    name = &current_name;
    xstrinit(name);

    while (isOpen && !file.eof()) {
        file.getline(current_line, LINE);

        xstragain(name);

        line = current_line;
        for (chr = *line++;  chr != 0;  chr = *line++) {
            if (chr == ';')
                break;
            xstraddchar(name,chr);
        }

        cout << xstrcstr(name) << endl;
    }

    xstrfree(name);
}

Here is a "smart" string [buffer] class similar to what you're used to:

// xstr -- "smart" string "class" for C

typedef struct {
    size_t xstr_maxlen;                 // maximum space in string buffer
    char *xstr_lhs;                     // pointer to start of string
    char *xstr_rhs;                     // pointer to start of string
} xstr_t;

// xstrinit -- reset string buffer
void
xstrinit(xstr_t *xstr)
{

    memset(xstr,0,sizeof(xstr));
}

// xstragain -- reset string buffer
void
xstragain(xstr_t xstr)
{

    xstr->xstr_rhs = xstr->xstr_lhs;
}

// xstrgrow -- grow string buffer
void
xstrgrow(xstr_t *xstr,size_t needlen)
{
    size_t curlen;
    size_t newlen;
    char *lhs;

    lhs = xstr->xstr_lhs;

    // get amount we're currently using
    curlen = xstr->xstr_rhs - lhs;

    // get amount we'll need after adding the whatever
    newlen = curlen + needlen + 1;

    // allocate more if we need it
    if ((newlen + 1) >= xstr->xstr_maxlen) {
        // allocate what we'll need plus a bit more so we're not called on
        // each add operation
        xstr->xstr_maxlen = newlen + 100;

        // get more memory
        lhs = realloc(lhs,xstr->xstr_maxlen);
        xstr->xstr_lhs = lhs;

        // adjust the append pointer
        xstr->xstr_rhs = lhs + curlen;
    }
}

// xstraddchar -- add character to string
void
xstraddchar(xstr_t *xstr,int chr)
{

    // get more space in string buffer if we need it
    xstrgrow(xstr,1);

    // add the character
    *xstr->xstr_rhs++ = chr;

    // maintain the sentinel/EOS as we go along
    *xstr->xstr_rhs = 0;
}

// xstraddstr -- add string to string
void
xstraddstr(xstr_t *xstr,const char *str)
{
    size_t len;

    len = strlen(str);

    // get more space in string buffer if we need it
    xstrgrow(xstr,len);

    // add the string
    memcpy(xstr->xstr_rhs,str,len);
    *xstr->xstr_rhs += len;

    // maintain the sentinel/EOS as we go along
    *xstr->xstr_rhs = 0;
}

// xstrcstr -- get the "c string" value
char *
xstrcstr(xstr_t *xstr,int chr)
{

    return xstr->xstr_lhs;
}

// xstrfree -- release string buffer data
void
xstrfree(xstr_t *xstr)
{
    char *lhs;

    lhs = xstr->xstr_lhs;
    if (lhs != NULL)
        free(lhs);

    xstrinit(xstr);
}

Recommendations

  • Before you try to "get around" a "c string", embrace it. You'll encounter it in many places. It's unavoidable.
  • Learn how to manipulate pointers as easily as index variables. They're more flexible and [once you get the hang of them] easier to use. I've seen code written by programmers who didn't learn this and their code is always more complex than it needs to be [and usually full of bugs that I've needed to fix].
  • Good commenting is important in any language but, perhaps, more so in C than Java for certain things.
  • Always compile with -Wall -Werror and fix any warnings. You have been warned :-)
  • I'd play around a bit with the myfnc examples I gave you. This can help.
  • Get a firm grasp of the basics before you ...

And now, a word about C++ ...

Most of the above was about architecture, memory layout, and C. All of that still applies to C++.

C++ does do a more limited reclamation of stack variables when the function returns and they go out of scope. This has its pluses and minuses.

C++ has many classes to alleviate the tedium of common functions/idioms/boilerplate. It has the std standard template library. It also has boost. For example, std::string will probably do what you want. But, compare it against my xstr first.

But, once again, I wish to caution you. At your present level, work from the fundamentals, not around them.

Community
  • 1
  • 1
Craig Estey
  • 22,991
  • 3
  • 18
  • 37
  • Exactly! I am going to try that – Mosedince Nov 11 '15 at 23:17
  • That worked perfectly!! Would you explain how would I prevent that from happening in a better way (if there is)? BECAUSE, in Java the one I wrote should work just fine! – Mosedince Nov 11 '15 at 23:26
  • @Mosedince C-style arrays are weird. You could use C++ style arrays instead, or in this case, C++ strings. – M.M Nov 12 '15 at 02:19
-1

Adding current_name[i] = 0; as described did not work for me. Also, I got an error on isOpen as shown in the question. Therefore, I freehanded a revised program beginning with the code presented in the question, and making adjustments until it worked properly given an input file having two rows of text in groups of three alpha characters that were delimited with " ; " without the quotes. That is, the delimiting code was space, semicolon, space. This code works.

Here is my code.

     #define LINE 1000

    int j = 0;
    while (!file1.eof()) {
        j++;
        if( j > 20){break;} // back up escape for testing, in the event of an endless loop

        char current_line[LINE];
        //string current_name = ""; // see redefinition below

        file1.getline(current_line, LINE, '\n');

        stringstream ss(current_line); // stringstream works better in this case

        while (!ss.eof()) {

            string current_name;
            ss >> current_name;
            if (current_name != ";")
            { 
                cout << current_name << endl;
            } // End if(current_name....

            } // End while (!ss.eof...

    } // End while(!file1.eof() ...

    file1.close();

    cout << "Done \n";
K17
  • 668
  • 3
  • 8
  • 25
  • this repeats the `while...eof` mistake (twice) – M.M Nov 12 '15 at 02:18
  • Not a mistake. note one is using eof on a file and the other is using elf on a string stream. Search on that and you should see examples in StackOverflow. Be sure you are correct before marking something a mistake and then marking it down. Suggest you retract that -1. – K17 Nov 12 '15 at 02:20
  • No, it is a mistake. The existence of other posts with the same mistake doesn't make it a non-mistake. [Read this page](http://stackoverflow.com/questions/5605125/why-is-iostreameof-inside-a-loop-condition-considered-wrong) – M.M Nov 12 '15 at 02:36
  • Not a mistake. Check the documentation on file streams vs string streams. You'll get the same answer. And don't overlook that fact that this works perfectly. Remember, string stream vs. file stream. – K17 Nov 12 '15 at 02:38
  • `eof` is a member of `ios`, which is a base class to both of those streams . It is just as much a mistake on the `ifstream` as it is on the `sstream` . – M.M Nov 12 '15 at 02:45
  • No, and I don't intend to . Fix your code if you want downvotes retracted. – M.M Nov 12 '15 at 02:47
  • You have no credibility if you don't even try to test it. You saying it is a mistake doesn't make it a mistake. – K17 Nov 12 '15 at 02:48
  • Have you bothered to read the thread I linked where hundreds of people vote that it is a mistake? – M.M Nov 12 '15 at 02:49
  • Get back to me after you've tested it. – K17 Nov 12 '15 at 02:50
  • You're clearly new to C++. A program appearing to work correctly in one instance is no guarantee that the program is actually correct. Broken code will sometimes coincidentally produce the same output as correct code. I'll say no more here. For "credibility" you could look at site reputation. – M.M Nov 12 '15 at 02:54
  • Like I said, get back to me after you have tested it. – K17 Nov 12 '15 at 02:59