Thread: How can I improve this working code?

  1. #1
    Banned
    Join Date
    Aug 2017
    Posts
    861

    How can I improve this working code?

    This is my aprox 2nd or 3rd time I have ever dealt with 2d arrays.
    I am taking an unknown amount of data, putting it into a single array. Then spiting it into a 2d array.

    Just knowing their probably is a different more efficient way to do this. Their has to be a better way to do this?

    So that would be a challenge wouldn't it?

    The thing I did get really stuck on is getting "strlen" off the second element of a 2d array. that is why I coded it like I did. Because I could not figure that one out.


    what is a more efficient to take an unknown amount of data and deal with it accordingly to the specs.

    specs
    Code:
    working with x amount of lines, or, unknown amount of data. 
    
    Open file  then add it into a single array, then sort it 
    into a 2d array using only the first line and third line. 
    Then print output to screen. Using the lest amount
    of lines of code needed to do so. 
    
    No using the compilers abilities to read everything on one line.
    Must be formatted for readability.
    I'd just like to know a more efficient way to write this.
    my code.
    Code:
     
     
     #include <stdio.h>
     #include <string.h>
     
     
     #define MAXBUF 1024
     
     
     int main (void)
    {
    
        FILE *fp;
        char music[MAXBUF];
        char check_music[2][MAXBUF];
        fp = fopen("enter_sandman", "r");
        char c = '\n';
        int a = 0;
        
        // fill array with word including end line
        while (( c = fgetc(fp) )!= EOF )
            music[a++] = c;
        
        fclose(fp); 
    
        
        
        // a whole bunch of stuff to keep
        // track of everything. 
        
        int b = 0, count = 0, check = 0, move_arr = 0, x = 0;
        int string1 = 0, string2 = 0, z = 0;
        
        
        //working with x amount of lines
        // need 1st and 3rd
        // ........ can the rest
        
        
        //only need to look at 3 lines
        while ( b != 3 )
        { 
        // reset to go back into inner loop
            check = 0;
            
            while ( check != 1 )
            {    // marks for 2nd line
                if (count == 1)
                {
                    //skips second line
                    // just runs it out to the end
                    while ( music[++x] != '\n');
                    // prevents it from returning inside here
                    count++;
                     
                }
                else
                { 
                    
                    //move_arr keep alignment with roll
                    // z keeps alignment with x 
                    check_music[move_arr][z++] = music[x];
                 
                    x++;
                    
                    // switch off 1st and 3rd line
                    // to keep track of length of
                    // each line
                    switch (count)
                    {
                        case 0:
                            string1++;
                            break;
                        case 2:
                            string2++;
                            break;
                        default:
                            break;
                    }
             
            
            // if end line 
                    if ( music[x] == '\n')
                    {
                        // count is 0
                        // now it is one
                        // marked for 2nd line
                        count++;
                         
                    // moves the 2d 1st element to 
                    // get 3rd line 
                        move_arr++;
                        // kicks it out of this
                        // inner loop
                        check = 1;
                        // resets lenght count
                        //to copy 2nd elements
                        // check_music[1][z = 0 -> length]
                        z = 0;
                         
                 
                    }
                }
                 
            }
            b++;
        }    
         
         // self explanatory? 
         
            int g = 0;
            for ( ; g < string1; g++)
                printf("%c", check_music[0][g]);
                        
        printf("\n");
            
           
            for ( g = 0; g < string2; g++)
                printf("%c", check_music[1][g]);
        
        printf("\n");
        
        
         
    return 0;     
    }
    output
    Code:
    userx@slackwhere:~/bin
    $ ./last_3_words
    Say your prayers, little one
    
    To include everyone
    data file, something everyone might identify with.
    Code:
    Say your prayers, little one
    Don't forget, my son
    To include everyone
    
    Tuck you in, warm within
    Keep you free from sin
    Till the Sandman he comes
    
    Sleep with one eye open
    Gripping your pillow tight
    
    Exit: light
    Enter: night
    Take my hand
    We're off to never never land
    
    Something's wrong, shut the light
    Heavy thoughts tonight
    And they aren't of Snow White
    
    Dreams of war, dreams of liars
    Dreams of dragon's fire
    And of things that will bite
    
    Sleep with one eye open
    Gripping your pillow tight
    Last edited by userxbw; 11-10-2017 at 11:36 PM.

  2. #2
    Registered User
    Join Date
    May 2010
    Posts
    4,633
    If you only are concerned with three lines why are you reading the whole file?

    By the way you should check that the file opened correctly, and you need to insure that you can never overflow the bounds of your array, especially in read loops. You should be checking that you're in bounds as the first condition of your read loop.

    Must be formatted for readability.
    I would say that you failed in this aspect. IMO, you have too many useless multi-line comments that just clutter the program.

    Example:
    Code:
         
        // a whole bunch of stuff to keep
        // track of everything.
    Also if you used meaningful variable names you wouldn't need the comment.
    Code:
         
        //only need to look at 3 lines
        while ( b != 3 )
    IMO, something like "line" instead of "b" would be a much better variable name. Also if you only read three lines you might not need this loop at all.

    Also functions should be your friends, using some would probably make this program much easier to read.

  3. #3
    Registered User
    Join Date
    Feb 2014
    Posts
    30
    Quote Originally Posted by userxbw View Post
    Code:
         
         // self explanatory?
    Don't do this. Either put nothing, or change the two useless words to two meaningful words.

    Code:
              // Print arrays.

  4. #4
    Banned
    Join Date
    Aug 2017
    Posts
    861
    Quote Originally Posted by pjadl View Post
    Don't do this. Either put nothing, or change the two useless words to two meaningful words.

    Code:
              // Print arrays.
    the comments:
    Where placed there especially for in here for them that do not know what anything is used for wen they look at code (they cannot figure it out just by sight).

    so to you they are useless but to others they are not.

    as well it has no interference or optimization to the code itself, so basically your post is more ........
    Last edited by userxbw; 11-11-2017 at 12:19 PM.

  5. #5
    Banned
    Join Date
    Aug 2017
    Posts
    861
    Quote Originally Posted by jimblumberg View Post
    If you only are concerned with three lines why are you reading the whole file?

    By the way you should check that the file opened correctly, and you need to insure that you can never overflow the bounds of your array, especially in read loops. You should be checking that you're in bounds as the first condition of your read loop.


    I would say that you failed in this aspect. IMO, you have too many useless multi-line comments that just clutter the program.

    Example:
    Code:
         
        // a whole bunch of stuff to keep
        // track of everything.
    Also if you used meaningful variable names you wouldn't need the comment.
    Code:
         
        //only need to look at 3 lines
        while ( b != 3 )
    IMO, something like "line" instead of "b" would be a much better variable name. Also if you only read three lines you might not need this loop at all.

    Also functions should be your friends, using some would probably make this program much easier to read.
    why take in more than needed?
    because I was building off of it. say you ran into a situation where you got more than you need and well just take what you need and leave the rest. gee how do I do that? well figure it out.

  6. #6
    Registered User
    Join Date
    May 2015
    Posts
    90
    Hey,

    You could use fgets(), you are as a matter of fact checking for the new line character as far as I can see (did not read it toroughly).

    If you want to stick to fgetc() and read character by character into an array, then you could then build your two dimensional array by checking the newline character as your delimiter, and filling the two dimensional array with strings that represent every line. After that you can use that array for your liking (e.g printing odd lines or even lines, etc.)

    It is also good practice to build this with some custom split() function.

  7. #7
    Registered User
    Join Date
    Feb 2014
    Posts
    30
    Quote Originally Posted by userxbw View Post
    the comments:
    Where placed there especially for in here for them that do not know what anything is used for wen they look at code (they cannot figure it out just by sight).

    so to you they are useless but to others they are not.

    as well it has no interference or optimization to the code itself, so basically your post is more ........
    They're useless, because if it didn't appear, the understanding would be the same. It's also useless, because you typed two words (although one is suffering from poor spelling), when I show you how two (meaningful) words would be better. It's even less typing.

    The fact that you find fault in what I said rather than taking it as it is, shows you are closed minded and not open to understanding. You just want people to pat you on the back and say "Good job!".

  8. #8
    Banned
    Join Date
    Aug 2017
    Posts
    861
    Quote Originally Posted by pjadl View Post
    They're useless, because if it didn't appear, the understanding would be the same. It's also useless, because you typed two words (although one is suffering from poor spelling), when I show you how two (meaningful) words would be better. It's even less typing.
    oh no mom, we got a nit picker in the crowd. so spelling every word "correctly" makes me a better person? NOT!

    less typing, yeah ok, is this code going into production, no, so ....

    The fact that you find fault in what I said rather than taking it as it is, shows you are closed minded and not open to understanding.

    I seen it as you not having a full understanding of why they are even there
    comments? did I ask about comments? I put them there for the reason I said, get over it.

    The post said what?

    How can I improve this working code?

    not how can I spell correcttly because comments do not have spell checker, so what I misspelled a word or two.
    guess what people still got a pay bills, and the sun still came up.

    You just want people to pat you on the back and say "Good job!".
    that is called projecting your self image:

    no I actually believe their is a better way to do that. I just do not know what it is. So I put it up here hoping that someone might have some suggesting on the code, not the comments. nor the indentation use to in my code.

    It seems you're just up set I didn't pat you on the back for you pointing out what you see as important, when I do not. so now you're not feeling so loved.

    it is not my fault you cannot follow instructions, the post said what? nothing about how to write better comments. that is meant as constructive criticism.
    Last edited by userxbw; 11-11-2017 at 07:32 PM.

  9. #9
    and the hat of int overfl Salem's Avatar
    Join Date
    Aug 2001
    Location
    The edge of the known universe
    Posts
    39,660
    I found pjadl's comments to be spot on, and you should take heed.

    Big ball of mud - Wikipedia

    You can start by creating meaningful function names to do specific tasks, rather than one whole mess in main with meaningless variable names.
    If you dance barefoot on the broken glass of undefined behaviour, you've got to expect the occasional cut.
    If at first you don't succeed, try writing your phone number on the exam paper.

  10. #10
    Registered User
    Join Date
    May 2010
    Posts
    4,633
    I also agree with pjadl and Salem, you ask to help you improve the code and then you either belittle, ignore, or say the crap pointed out is a "feature".

    By the way in your original post, before you edited the post to take out some of the requirements you specifically stated that "readability" was a requirement. Your nonsensical comments are making that horrible code even worse.

  11. #11
    Banned
    Join Date
    Aug 2017
    Posts
    861
    Quote Originally Posted by Salem View Post
    I found pjadl's comments to be spot on, and you should take heed.

    Big ball of mud - Wikipedia
    so all of this is not meaningful when you read the code it cannot by any means become apparent to what it is being used for?
    Code:
      int b = 0, count = 0, check = 0, move_arr = 0, x = 0;    
    int string1 = 0, string2 = 0, z = 0;
    Code:
        while ( b != 3 )
    and alike, where just using letters by my experience in reading a lot of other peoples code just a letter is used for keeping count in a loop then used within that loop.

    would you like for me to dig up a bunch of examples of other peoples code in production that do this?
    You can start by creating meaningful function names to do specific tasks,

    2: I do not have any functions in that code at all so that what you said there is invalid NULL and void, it does not apply to this code.

    so how you see what he is saying about my code in that reference to what names I used for my vars I have no idea what you nor he is talking about.

    it sound more like a patriot talking to me.

    rather than one whole mess in main with meaningless variable names.
    that is nit picking for this example of code. in what little bit it is doing within the entire program it does not even need to be pulled apart and have functions created to do what it is doing, in the whole of everything it is doing.
    Last edited by userxbw; 11-12-2017 at 03:34 PM.

  12. #12
    Banned
    Join Date
    Aug 2017
    Posts
    861
    Quote Originally Posted by jimblumberg View Post
    I also agree with pjadl and Salem, you ask to help you improve the code and then you either belittle, ignore, or say the crap pointed out is a "feature".

    By the way in your original post, before you edited the post to take out some of the requirements
    The thing I did get really stuck on is getting "strlen" off the second element of a 2d array. that is why I coded it like I did. Because I could not figure that one out.


    what is a more efficient to take an unknown amount of data and deal with it accordingly to the specs.
    I too pointed out that and even underlined it.
    you specifically stated that "readability" was a requirement. Your nonsensical comments are making that horrible code even worse.
    was is past tense if I even did such a thing that you stated there. as I do not even recall doing that. too, if I did it was edited out way way way before anyone even posted to it. so that argument is invalid.

    you all seem to have been programmed to this one way of thinking and I think it is making it hard for all of you to think out side of that "norm".


    more than one line of comments is a bad thing, DO NOT DO THAT!
    really?

    I Have seen production code that has more then one line of comments in it. I suppose he needs to be spanked for that.

    comment line are as many as you need to try and explain it.

    what is really important for code , first and foremost?
    that it works! the faster and more efficient the better, then you can make it look pretty.

    what good is it to do it backswords? make it look pretty first, but wait it does not even do what it is suppose to.

    yeah, but doesn't look pretty? sooo pretty.

    ( that is not to say it should not have some type of organization to it)
    Last edited by userxbw; 11-12-2017 at 03:46 PM.

  13. #13
    misoturbutc Hodor's Avatar
    Join Date
    Nov 2013
    Posts
    1,791
    Sh_t. You asked for feedback, got it and are all defensive and not listening. Accept that your program is awful and should be scrapped and rewritten. This time, use functions, meaningful variable names, structure, error checking (like checking the file is even open before using it), and fewer useless comments.

    Edit: Your array is wrong

  14. #14
    Banned
    Join Date
    Aug 2017
    Posts
    861
    Quote Originally Posted by Hodor View Post
    Sh_t. You asked for feedback, got it and are all defensive and not listening. Accept that your program is awful and should be scrapped and rewritten. This time, use functions, meaningful variable names, structure, error checking (like checking the file is even open before using it), and fewer useless comments.

    Edit: Your array is wrong
    which one?

    though I do not agree with that meaningful var names that is just spitting out patriot talk . ... they are meaningful. functions I'll think about for that little bit of operation. Just using the function main isn't a bad thing. that in a meaningful function name.

    "awful and should be scrapped" good thing my ego isn't easily bruised.

    scrapped OK.
    Last edited by userxbw; 11-12-2017 at 04:47 PM.

  15. #15
    misoturbutc Hodor's Avatar
    Join Date
    Nov 2013
    Posts
    1,791
    Quote Originally Posted by userxbw View Post
    scrapped OK.
    Excellent. It's not a large program and sometimes scrapping and starting again is the best way to go

    Hmm. I'm not sure now what I meant about the array. Ignore that bit.

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Please Improve my code
    By hariharaan in forum C Programming
    Replies: 0
    Last Post: 05-20-2015, 02:05 AM
  2. Can I improve my code?
    By advancedk in forum C Programming
    Replies: 1
    Last Post: 07-27-2008, 07:47 AM
  3. Help me improve my code!
    By wise_ron in forum C Programming
    Replies: 11
    Last Post: 09-19-2006, 10:04 AM
  4. How to improve code
    By rugby in forum C Programming
    Replies: 3
    Last Post: 04-15-2003, 09:24 AM
  5. help improve my code
    By lambs4 in forum C Programming
    Replies: 3
    Last Post: 11-21-2001, 11:33 AM

Tags for this Thread