Critique my code?

This is a discussion on Critique my code? within the C++ Programming forums, part of the General Programming Boards category; Hello everyone, I am trying to get back in programing. I started learning in college, and pretty much forgot everything ...

  1. #1
    Registered User
    Join Date
    Jun 2004
    Posts
    7

    Critique my code?

    Hello everyone, I am trying to get back in programing. I started learning in college, and pretty much forgot everything I learned. I was wondering if anyone wouldn't mind taking a look at my stuff, all I did was make a map of x by y, then spit it out. I am using the free bloodshed compiler.

    Does anyone know of any sites that has any good homework type exercises? I try to think up my own but it gets pretty tough. There was also a function that you could put the text anywere on the screen you want to... but I can't seem to find it anywere.



    Code:
    //attempt at making a map with array's
    
    #include <iostream>
    #include <stdlib.h>
    
    
    
    using namespace std;
    void spitmap();
    void makemapnum(int x, int y);
    int x=0,y=0;
    int xcount=0;
    int place[5000][5000];  // [x],[y]       MAKE GLOBAL
    
    int main(int argc, char *argv[])
    {
      cout<<"Please enter how wide you want the map. :";
      cin>>x;
      cout<<"Please enter how long you want the map. :";
      cin>>y;
      spitmap(); //spits out map
      
      makemapnum(x,y);
      
      system("PAUSE");	
      return 0;
    }
    
    
    
    void spitmap()
    {
      for(int counter=0; counter<y; counter++)
        {
        do
         {
         cout<<"O";
         xcount++;
         }
         while(xcount<x);
        xcount=0;
        cout<<" "<<counter+1<<endl;
        }
      for(int counter=0;counter<x;counter++)
        {
        cout<<counter+1;
        }
    }
    
    void makemapnum(int x,int y)   // counting on the map
    {
    
    int count_x=0, count_y=0;
    int placecount=0;
    
    for(int count_y=0; count_y<y; count_y++)
    
     {
      do
      {
      place[count_x][count_y]=placecount;
      count_x++;
      placecount++;
    
      }while(count_x+1<x);
    
    
     placecount++;
     cout<<endl<<placecount<<" place["<<count_x<<"]["<<count_y<<"]";
     count_x=0;         // resetting count_x for next moving
     }
    
    }

  2. #2
    and the hat of wrongness Salem's Avatar
    Join Date
    Aug 2001
    Location
    The edge of the known universe
    Posts
    32,500
    Well this site - http://acm.uva.es/problemset/ - has lots of challenges
    google for "programming contests" if you want more

    As for your code, I would suggest the following.
    1. Better indentation - its only a few lines long, and it's already looking untidy and hard to follow.

    2. Get rid of all your global variables. Your 100MB array needs fixing as well, to make it use only the memory which is required.

    3. Remove shadowing
    Code:
    int count_x=0, count_y=0;
    int placecount=0;
    for(int count_y=0; count_y<y; count_y++)
    count_y exists at two different scope levels. This can lead to all sorts of confusion at times.

    4. Why are you using while loops when for loops would seem more natural?

    5. #include <stdlib.h>
    Would be
    #include <cstdlib>
    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.
    I support http://www.ukip.org/ as the first necessary step to a free Europe.

  3. #3
    Codebot
    Join Date
    Jun 2004
    Location
    Toronto
    Posts
    195
    -DONT EVER USE GLOBAL VARIABLES!!!
    you can get the same solution by using local variables.

    -Also do you really need a 2.5Mb array???
    You can use dynamic memory allocation to efficiently use your code.

    -Indentation should be 1 TAB. its easier to read.

  4. #4
    ~viaxd() viaxd's Avatar
    Join Date
    Aug 2003
    Posts
    246
    Code:
      system("PAUSE");
    try not to use this as well, to wait for the user to press a key you can use
    Code:
    std::cin.get();

  5. #5
    Toaster Zach L.'s Avatar
    Join Date
    Aug 2001
    Posts
    2,686
    >> DONT EVER USE GLOBAL VARIABLES!!!

    That's a bit of exaggeration, but generally, you can get better results with local variables by passing them by reference, pointer, or simply reassigning them in your code as appropriate. The problem with globals is that it is easy for other functions to modify the value at any time. This can create problems (no way to ensure the data is not improperly modified), and makes it harder to debug since the flow of changes is not particularly intuitive.

  6. #6
    Code Goddess Prelude's Avatar
    Join Date
    Sep 2001
    Posts
    9,796
    >-DONT EVER USE GLOBAL VARIABLES!!!
    Overkill. Perhaps "Use global variables judiciously" would be better.

    >You can use dynamic memory allocation to efficiently use your code.
    Or better yet, the standard library.

    >-Indentation should be 1 TAB. its easier to read.
    Tabs tend to mess up code when read with a different editor. Spaces are often a better option. The most common indent sizes are 2 and 4, with the occasional 1, 3, and 8 popping up.
    My best code is written with the delete key.

  7. #7
    l'Anziano DavidP's Avatar
    Join Date
    Aug 2001
    Location
    Plano, Texas, United States
    Posts
    2,738
    >-DONT EVER USE GLOBAL VARIABLES!!!

    Yeah that is way overkill. I agree with Prelude's saying of "use them judiciously."

    In a program this short they are probably not needed, although there might be a case or two where they would be useful, you never know.

    There are certain project, however, that call for global variable quite a bit. One example is game programming. games use global variables quite a bit.

    as a beginner, you should stay away from globals unless they are needed, and as a more advanced programmer, you should know what situtations would be good to use global variables.
    My Website

    "Circular logic is good because it is."

  8. #8
    Codebot
    Join Date
    Jun 2004
    Location
    Toronto
    Posts
    195

    Cool

    >-DONT EVER USE GLOBAL VARIABLES!!!

    Scare tactic 101.

    The reason I say this is that I avoid using global variables because there are ways of storing things that make golbal variables obsolete. for example, if you use malloc/new to dynamically allocate memory, and then pass around a pointer, that would give you the same result, but agian its quite limited.

    If your going to call a function over and over agian and you need to keep a variable form going out of scope, you can always try making a variable inside the function static. this way the variable will not go out of scope. BUT there are draw backs to every exception.

    I suggest taking a course or reading a book on how pointers work. It will give you insights on how to make your code a bit more efficient and easier to read.

  9. #9
    Registered User truthADjuster's Avatar
    Join Date
    Jun 2004
    Posts
    3
    start adapting a variable naming convention and be consistent with this until the end of time...

  10. #10
    Registered User
    Join Date
    Jun 2004
    Posts
    7
    Ok, thank you everyone I do appreciate it very much. Since I can't take classes right now all of your advice has been extremly valuable to me.
    I am too poor right now to buy a book(gf doesn't see it justified even if the book is 20 bucks).

  11. #11
    Stinking it up. StinkyRyan's Avatar
    Join Date
    Jun 2004
    Posts
    61
    You dont need to buy books just read everything online or print it out and staple it and make your own book.

  12. #12
    Codebot
    Join Date
    Jun 2004
    Location
    Toronto
    Posts
    195
    There are some nice tutorials out there and some nice ebooks. if you can find some then its just a matter of printing it out and sticking to it.

  13. #13
    Toaster Zach L.'s Avatar
    Join Date
    Aug 2001
    Posts
    2,686
    Quote Originally Posted by truthADjuster
    start adapting a variable naming convention and be consistent with this until the end of time...
    But if you use the Hungarian naming convention, I will be forced to hurt you.

  14. #14
    Registered User axon's Avatar
    Join Date
    Feb 2003
    Location
    Mt. Prospect, IL
    Posts
    2,572
    >>gf doesn't see it justified even if the book is 20 bucks)

    HA! thats funny...how old are you?..HAHA

    anywho, there are tons of tutorials online, just google for some...also many schools offer online course webpages which could prove useful as well.

    some entropy with that sink? entropysink.com

    there are two cardinal sins from which all others spring: Impatience and Laziness. - franz kafka

  15. #15
    Registered User
    Join Date
    Jun 2004
    Posts
    7
    Yea, I know that sounds silly, but when money is tight 20 bucks is 20 bucks for gas/food whatever, and since it is just a hobby I'm starting up it's not justified. So I stick for the free online tutorials for now, then when I get better try to find a class.

Page 1 of 2 12 LastLast
Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Proposal: Code colouring
    By Perspective in forum A Brief History of Cprogramming.com
    Replies: 28
    Last Post: 05-14-2007, 07:23 AM
  2. Values changing without reason?
    By subtled in forum C Programming
    Replies: 2
    Last Post: 04-19-2007, 10:20 AM
  3. Updated sound engine code
    By VirtualAce in forum Game Programming
    Replies: 8
    Last Post: 11-18-2004, 11:38 AM
  4. Interface Question
    By smog890 in forum C Programming
    Replies: 11
    Last Post: 06-03-2002, 05:06 PM
  5. Replies: 0
    Last Post: 02-21-2002, 05:05 PM

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21