Thread: probs with coding

I have removed my query.

Get rid of the globals and the gotos; they're not necessary. Maybe then I'll help. Also format your code so that it's readable. What is TRUE? What is conio.h? What is ch == 93 supposed to mean? What is character 91? fgetc() returns an int, not a char. It's all nonsense. Seriously, don't write code like this; there's no need for it and it's unreadable. Get a book that explains structured programming.

Edit: I'd just like to add that this is the worst code I've seen on this forum in a long time. It's gibberish. Yeah, you won't like me for saying that, but it's TRUE. Maybe go back to quickbasic or whatever it is you're used to. Hate me, I don't care.

Originally Posted by ivory348

i have removed my query.

lol

What is conio.h?

What is ch == 93 supposed to mean?

oof.

I wish to extract those [ .... ] strings and print them.

Code:

#include <stdio.h>

int main(int args, char *argv[]) {

    FILE *fp = fopen("text.txt", "r");
    int readChar = 0, bp = 0;
    char buffer[30], ch;
    
    while ((ch = fgetc(fp)) != EOF) {
        if (ch == ']') {
            buffer[bp++] = '\0';
            readChar = 0; bp = 0;
            printf("%s\n", buffer);
        };
        if (readChar == 1) {
            buffer[bp++] = ch;
        };
        if (ch == '[') readChar = 1;
    };

    fclose(fp);
    return 0;
};

Originally Posted by Ivory348

Or is it gut feeling after endless experience?

"Structure" has no experience. He's a permanent amateur with no desire to learn.

Originally Posted by Hodor

Edit: I'd just like to add that this is the worst code I've seen on this forum in a long time. It's gibberish. Yeah, you won't like me for saying that, but it's TRUE. Maybe go back to quickbasic or whatever it is you're used to. Hate me, I don't care.

Criticising code is one thing; an ad hominem attack on its author by telling them they should stop programming in C is another. You might be really irritated at the code, but refrain from doing this.

Originally Posted by Ivory348

I wish to extract those [ .... ] strings and print them.

One thing to consider is whether the format allows for nested delimiters, and what does that mean for the output. For example, if the input contains "[[hello]]", should the output be "hello" or "[hello]" or "[hello", or should the program report an input format error? What about "[[hello]world]"?

Originally Posted by Ivory348

So I rewrote the code (below) and need some help to get it running.

Your code certainly looks better than what was previously described, but I think your problem is that you were nesting loops while reading in the inner loops: sometimes this works well, but it can be easier to deal with one character at a time in the loop body rather than try to do too much at a time. Also, note that fgetc returns an int, not a char. This is important for comparing with EOF (you compared with 0 instead, which is wrong).

Originally Posted by Ivory348

I flow charted your solution and see that you start with xyz] rather than with [xyz . I could never have come up with your solution, and wonder is there a principle behind how you did it? Or is it gut feeling after endless experience?

Structure's solution is basically what it looked like you were trying to do, except that it deals with one character at a time. What it looked like you were trying to do is write a state machine: one state is the default state or when a closing bracket has been detected; the other state is when an opening bracket has been detected. If you're in the first state but not transitioning to the second state, you save the current character in a buffer for printing later. If you're in the second state and transitioning to the first state, you print the buffer. Since there are only two states, state transitions can be recorded with a boolean flag (but note that readChar probably isn't a good name since a char is always read; perhaps writeChar might be a more apt name). This is a common technique and can appear more explicitly with a switch and enums when there are more than two states.

Having just two states like this in a boolean flag caters for the case where "[[hello]]" should result in the output of "[hello". If that isn't desired, you may need a somewhat more complex solution, e.g., an opening bracket counter that is decremented each time a closing bracket is encountered and the counter is greater than 0: in such a case the first state corresponds to when the counter is 0, the second state corresponds to when the counter is positive.

As mentioned earlier, the return value of fgets is an int, so ch in Structure's code should have been an int. The reason is that the character read will be converted to an unsigned char and then returned as an int, so that you can differentiate between a valid value (i.e., a non-negative value in the range of unsigned char) and EOF (which is a negative integer). If you use char instead of int, and char happens to be unsigned, then you won't be able to compare with EOF correctly (since EOF will be converted to (unsigned) char, and the result of that conversion will then be promoted to int to compare to EOF).

Note that both your attempted solution and Structure's solution are vulnerable to buffer overflow: if you have a fixed size buffer of 30 char, then you can only store 29 chars + null char in it at most, so the code that writes to the buffer must check that it will not write to the buffer out of bounds.

Remember to check the return value of fopen before using the file pointer. Observe that Structure's code correctly closes the file if it is open whereas yours did not do so; while you can get away with not doing so since the program will terminate soon after, it is good practice to do so.

You should also be aware that Structure's use of semi-colons and therefore null statements after closing braces constitutes poor style.

Originally Posted by Ivory348

Re Hodor. I have been attempting to respond to your texts but Cboard is blocking me. What a culture!

I have no idea why anything would be blocked. If your responses are being blocked then... I don't know what to say. I find it hard to believe but if they are being blocked it has nothing to do with me. If you're talking about me not responding to some posts in this thread where you addressed me directly then that's a different story. You were not blocked and I saw your questions but I decided that given the mood I was in last night and this morning it would be better that I didn't respond. For the record, I did not mean to insult you personally but the original code you posted was horrible (not you, the code). @laserlight asked me to refrain from ad hominem responses and although I did not intend my original response to be an ad hominem attack I can see how it turned out that way. So I chose to not respond at all. There was enough discussion in this thread without my further input anyway.

Edit: If you want me to apologise for attacking you (perceived or otherwise) I'll do that. If you want me to say that your original code wasn't that bad after all then... well, you're out of luck. My reason for not replying to this thread was to hopefully defuse an ugly situation that I had caused, and since lots of people had responded I couldn't see the point of responding anyway