Thread Rating:
  • 0 Vote(s) - 0 Average
  • 1
  • 2
  • 3
  • 4
  • 5
Asking for Tips: Writing readable, beautiful code
#11
like this ?????

Code:
#include<iostream>
#include<sstream>
#include<conio.h>
#include<windows.h>

using namespace std;

int GEN;

/************************************CLASS*OF*CELL*FOR*GETTING*AND*SETTING*ALIVE*CELL***********************************/

class Cell
{
      private:
              bool Cell_Check;
      public:
            
        bool Cell_Get()const
        {
            return(Cell_Check);
        }
        
        void Cell_Set(bool Stats)
        {
            Cell_Check = Stats;
        }    
};
      
int main()
{
    system("color 0A");
    
    int cell_x, cell_y;
    
    char choice;
    
    /***************************************BASIC*GRID*INPUT*****************************************************/
    
    cout << "Enter dimensions of your cell \20\20\20" << endl;
    cin >> cell_x >> cell_y;
    
    Cell Grid[cell_y][cell_x], Dead[cell_y + 2][cell_x + 2], Next_Grid[cell_y][cell_x];
    
    for(int i = 0; i < cell_y + 2; i++)
    {
            for(int j = 0; j < cell_x + 2; j++)
            {
                    Dead[i][j].Cell_Set(false);
            }
    }
    for(int i = 0; i < cell_y; i++)
    {
            for(int j = 0; j < cell_x; j++)
            {
                    Grid[i][j].Cell_Set(false);
                    Next_Grid[i][j].Cell_Set(false);
            }
    }
    
    cout << "Do you want to give input through coordinates(c) or you want to try navigation input system(n) \20\20\20" << endl;
    cin >> choice;
    
    fflush(stdin);
    
    /***********************************************************************************************************************8/
    
    /************************************************INPUT*THROUGH*COORDINATES********************************************************************/
        if(choice == 'c')
        {
                  int co_x, co_y;
                  
                  char ch_a;
                  
                    for(int i = 0; ; i++)
                    {
                            cout << "Enter the coordinates \20\20\20 " << endl;
                            cin >> co_x >> co_y;
                            
                            Grid[co_y][co_x].Cell_Set(true);
                            
                            cout << "Do you want to add more \20\20\20" << endl;
                            cin >> ch_a;
                            
                            if(ch_a == 'n')
                            break;
                    }
    /**********************************************************************************************************************/
                    
    /****************************************OUTPUT*OF*USER'S*INPUT************************************************/                
      cout << "Your input is \20\20\20" << endl;
    
     for(int y = 0; y < cell_y; y++)
            {
                    for(int x = 0; x < cell_x; x++)
                    {
                    
                            if(Grid[y][x].Cell_Get() == false)
                                cout << " . ";
                        
                            else if(Grid[y][x].Cell_Get() == true)
                                cout << " * ";
                        
                    }
            
                    cout << endl << endl;
            }
        }                
    /*****************************************************************************************************************/
    
    
    /*****************************************NAVIGATOR SYSTEM*******************************************************/
    
    else if(choice == 'n')
    {
    
    cout << "Use W,A,S,D keys to navigate and press K to make the cell alive" << endl;
    
    char key_Arrow;
    
    int move_A = 0, move_B = 0;
    
    for(int y = 0; y < cell_y; y++)
    {
            for(int x = 0; x < cell_x; x++)
            {
                    if((x == move_A) && (y == move_B))
                    cout << " <> ";
                    
                    else
                    {
                        if(Grid[y][x].Cell_Get() == false)
                        cout << " . ";
                        
                        if(Grid[y][x].Cell_Get() == true)
                        cout << " * ";
                    }
            }
            
            cout << endl << endl;
    }
    /*************************************BASIC*KEY*NAVIGATION*********************************************************/
    for(int inf = 0; ; inf++)
    {
            key_Arrow = getch();

                                       if(key_Arrow == 'w') //UP
                                              move_B--;
                                      
                                       if(key_Arrow == 's')    //DOWN  
                                              move_B++;
                                      
                                              
                                        if(key_Arrow == 'a') //LEFT
                                              move_A--;
                                              
                                              
                                        if(key_Arrow == 'd')   //RIGHT      
                                              move_A++;
                                      
                                        
                                         if(key_Arrow == 'k')   //ALIVE
                                              Grid[move_B][move_A].Cell_Set(true);
                                        
                                              
                                        if(key_Arrow == 13)
                                             break;      

                        
            
            
            system("cls");
            
            for(int y = 0; y < cell_y; y++)
            {
                    for(int x = 0; x < cell_x; x++)
                    {
                            if((x == move_A) && (y == move_B))
                            cout << " <> ";
                    
                            else
                            {
                                if(Grid[y][x].Cell_Get() == false)
                                cout << " . ";
                        
                                if(Grid[y][x].Cell_Get() == true)
                                cout << " * ";
                            }
                    }
            
                    cout << endl << endl;
            }
    }
    /******************************************************************************************************/
    
    /*******************************OUTPUT*OF*USER'S*INPUT***********************************************/  
    
    cout << "Your input is \20\20\20" << endl;
    
     for(int y = 0; y < cell_y; y++)
            {
                    for(int x = 0; x < cell_x; x++)
                    {
                    
                            if(Grid[y][x].Cell_Get() == false)
                                cout << " . ";
                        
                            else if(Grid[y][x].Cell_Get() == true)
                                cout << " * ";
                        
                    }
            
                    cout << endl << endl;
            }
     }                      
    /**************************************************************************************************************/
    
    /*********************************MAIN GAME OF LIFE************************************************************/
    
    int flag = 0;
    char Gen, G = 'y';
    
    cout << "Do you want to view all generations(a) one by one or a specific generation(s) \20\20\20 " << endl;
    cin >> Gen;
    
    fflush(stdin);
    
    if(Gen == 's')
    {
           cout << "Enter generation e.g. 25 for 25 generation \20\20\20 ";
           cin >> GEN;
    }
    for(int k = 0; ; k++)
    {
            for(int j = 1; j <= cell_y; j++)
            {
                    for(int i = 1; i <= cell_x; i++)
                    {
                            Dead[j][i].Cell_Set(Grid[j - 1][i - 1].Cell_Get());
                    }
            }
            
            int c = 0;
            
            for(int i = 1; i <= cell_y; i++)
            {
                    for(int j = 1; j <= cell_x; j++)
                    {
                            if(Dead[i][j + 1].Cell_Get() == true)
                            c++;
                            if(Dead[i][j - 1].Cell_Get() == true)
                            c++;
                            if(Dead[i + 1][j].Cell_Get() == true)
                            c++;
                            if(Dead[i + 1][j + 1].Cell_Get() == true)
                            c++;
                            if(Dead[i + 1][j - 1].Cell_Get() == true)
                            c++;
                            if(Dead[i - 1][j].Cell_Get() == true)
                            c++;
                            if(Dead[i - 1][j + 1].Cell_Get() == true)
                            c++;
                            if(Dead[i - 1][j - 1].Cell_Get() == true)
                            c++;
                            
                            if(c < 2)
                            Next_Grid[i - 1][j - 1].Cell_Set(false);
                            
                            if(c == 3 && Dead[i][j].Cell_Get() == false)
                            Next_Grid[i - 1][j - 1].Cell_Set(true);
                            
                            if((c == 2 || c == 3) && Dead[i][j].Cell_Get() == true)
                            Next_Grid[i - 1][j - 1].Cell_Set(true);
                            
                            c = 0;
                    }
            }
            
            if(Gen == 's')
            {
                   if(k == GEN)
                   break;
            }
            
            /*************************************CHECKS*FURTHER*GENERATIONS*ARE*SAME*******************************************/
            flag = 0;
            
             for(int y = 0; y < cell_y; y++)
                       {
                               for(int x = 0; x < cell_x; x++)
                               {
                      
                                       if(Grid[y][x].Cell_Get() != Next_Grid[y][x].Cell_Get())
                                       flag = 1;
                               }
                       }
           if(flag == 0)
           {            
            cout << "further generations are all same";
            break;
           }      
          
           /*****************************************************************************************************************/
          
           /***********************************ALL*GENERATIONS*******************************************************/
            if(Gen == 'a')
            {
                   if(G == 'y')
                   {
                       cout << "Generation #" << k + 1 << " \20\20\20" << endl;                      
                        
                       for(int y = 0; y < cell_y; y++)
                       {
                               for(int x = 0; x < cell_x; x++)
                               {
                                       if(Grid[y][x].Cell_Get() == false)
                                       cout << " . ";
                        
                                       else if(Grid[y][x].Cell_Get() == true)
                                       cout << " * ";
                        
                               }
            
                               cout << endl << endl;
                        }
                      
                   }
                  
                   else
                   break;                      
                  
                   G == getch();
            }
            
            for(int j = 0; j < cell_y; j++)
            {
                    for(int i = 0; i < cell_x; i++)
                    {
                            Grid[j][i].Cell_Set(Next_Grid[j][i].Cell_Get());
                            Next_Grid[j][i].Cell_Set(false);
                    }
            }
            for(int j = 0; j < cell_y + 2; j++)
            {
                    for(int i = 0; i < cell_x + 2; i++)
                    {
                            Dead[j][i].Cell_Set(false);
                    }
            }
    }
    /**********************************************************************************************************/
    /*******************************************N*GENERATION*************************************************************/
    if(Gen == 's')
    {
         cout << "Generation # " << GEN << " \20\20\20" << endl;
                        
                       for(int y = 0; y < cell_y; y++)
                       {
                               for(int x = 0; x < cell_x; x++)
                               {
                    
                                       if(Grid[y][x].Cell_Get() == false)
                                       cout << " . ";
                        
                                       else if(Grid[y][x].Cell_Get() == true)
                                       cout << " * ";
                        
                               }
            
                               cout << endl << endl;
                        }  
    }
/**********************************************************************************************************/

  system("pause");

}
Reply
Thanks given by:
#12
It has been a year and a half, and I think I have made lots of progress on this!

Here is a description of the style I've went with:
-Local variables are in lower case. Words are separated by the underscore character "_". e.g "blue_berry"
-Class member variables follow the above rule, except that they start with the underscore character "_".  e.g "_blue_berry"
Format:
  • Class names are in upper camel case. e.g: "ObjectInstance".
  • Functions which are in global scope are in camel case and first letter capitalized. e.g: "UpdateMode()"
  • Class member function are in camel case, but the first letter isn't capitalized. e.g: "renderEntities()"
  • Global variables / preprocessor defined values are in ALLCAPS.

After rereading this thread, it turned out there was one thing I left out, and that was the character per line limit =(. But I hope I will be able to get used to that as well.

Some other paradigms I follow:
  • Longer self-explanatory variables all the way. While that can make a line of code much longer than it could be, I try not to use 1 character variable name ("i", "j"..etc in for loops) except in cases where the block of code is short enough.
  • Not to use the default constructor when writing classes but get an "init()" function instead which takes the parameters and does the construction/initialization for you.

Finally, class files for stuff I've written recently (perhaps you could give it a skim and see if there you catch something I can improve):
http://pastebin.com/2YSsnLCG  #AExpression.h
http://pastebin.com/KmDzFgx6  #AExpression.cpp
http://pastebin.com/R6RWcKTE  #AScope.h
http://pastebin.com/WmWFuuWr  #AScope.cpp

I am still not the best with for loops and such, but I try to attain a balance between my focus on what the code does and its readability.

I'd be interested in listening to newer guys' opinions on this topic also.
[Image: signature.png]
A-Engine: A new beat em up game engine inspired by LF2. Coming soon

A-Engine Dev Blog - Update #8: Timeout

Reply
Thanks given by:
#13
So, this is my 2c after working in a software company for a while.

@A-man:
  • I used to write code like that, and actually I used to write it much, muuuch worse than that (to the point my current self got lost reading it ._.). You're on track to getting good; I think you're at the point that you need to see good practise code to produce it.
  • The code you're showing is a parser, so it's going to be longer than "normal", but these functions are still quite big, and not "simple enough to understand by skimming":
        C++-Code:
    AExpression<numeraltype>::processScopeTokens(std::string expression_string)
    AExpression<numeraltype>::init
    AExpression<numeraltype>::handlePrecedenceOfOperators()
    AScope<numeraltype>::getFuncParameter(std::string line, int number)

  • Reduce the number of lines per function, i.e. break down the function into calling smaller functions
  • Make smaller classes, such that you have one class per responsibility. When I've got time I'll elaborate on this, but google for solid design principles.
Reply
Thanks given by: A-Man
#14
(10-30-2015, 05:37 PM)Azriel Wrote:  I used to write code like that, and actually I used to write it much, muuuch worse than that (to the point my current self got lost reading it ._.). You're on track to getting good; I think you're at the point that you need to see good practise code to produce it.
lol, thank you very much, glad to hear. I really need to start getting myself into the git community for feedback in what I do.


Quote:[*]The code you're showing is a parser, so it's going to be longer than "normal", but these functions are still quite big, and not "simple enough to understand by skimming":
Code:
AExpression<numeraltype>::processScopeTokens(std::string expression_string)
AExpression<numeraltype>::init
AExpression<numeraltype>::handlePrecedenceOfOperators()
AScope<numeraltype>::getFuncParameter(std::string line, int number)
There you knew it :P. But yes, I see your point. You, as someone who've done this as a profession and have worked with professionals, would you agree to say that it's better to avoid shortforms entirely in naming and stick to full forms even if the names grew long?


Quote:[*]Make smaller classes, such that you have one class per responsibility. When I've got time I'll elaborate on this, but google for solid design principles.
I don't know what other class I could've had in there. AExpressions compile and evaluate expressions, and AScope stores a scope or a "context" of defined variables and functions.

I have read some guides on structure and design, and together with your points, I started trying out different approaches which will hopefully end with better code.

I appreciate your time =)


@Someone else
(03-10-2014, 09:16 PM)Someone else Wrote:  One thing you can do is to split the function parameters across multiple lines like:
Code:
d3d_set_projection_ext(x,y,z,
                      x+sin(dir*pi/180)*cos(zdir*pi/180),y+cos(dir*pi/180)*cos(zdir*pi/180),z+sin(zdir),
                      0,0,1,angle,window_get_width()/window_get_height(),znear,zfar);
This is what I tend to do, but sometimes this can get completely overwhelming (or tiresome for the cpu since it passes a lot of bytes to the function)
In this case you can use a structure to store the values like:
Code:
typedef struct {
   double xfrom;
   double yfrom;
   double zfrom;
   double xto;
   double yto;
   double zto;
   double xup;
   double yup;
   double zup;
   unsigned int angle;
   float aspect;
   unsigned int znear;
   unsigned int zfar;
} ProjectStruct;

//...

ProjectStruct Projection;
Projection.xfrom = x;
//...

d3d_set_projection_ext(&Projection);
Were you talking about designing the function so that it takes a struct instead of multiple parameters or about another way to pass parameters, regardless of how many parameters it's prototyped to take, using a single object instance?

You left me quite confused there, because the compiler complains about the number of arguments, and at the same time the function sounds like something out from a library (direct3d).
[Image: signature.png]
A-Engine: A new beat em up game engine inspired by LF2. Coming soon

A-Engine Dev Blog - Update #8: Timeout

Reply
Thanks given by:
#15
(11-14-2015, 10:27 AM)Doctor A Wrote:  Were you talking about designing the function so that it takes a struct instead of multiple parameters or about another way to pass parameters, regardless of how many parameters it's prototyped to take, using a single object instance?

You left me quite confused there, because the compiler complains about the number of arguments, and at the same time the function sounds like something out from a library (direct3d).
My point was that if a function has a lot of parameters it could be easier to make it take a structure instead, but the way I did it in that example is not how I would do it.
If you had a function like that in your code I would make the individual coordinates into a single vector struct.
The function is from Game Maker.

If you want to discuss coding style more, the best place is probably on IRC as I can respond quicker there.
Age ratings for movies and games (and similar) have never been a good idea.
One can learn a lot from reinventing wheels.
An unsound argument is not the same as an invalid one.
volatile in C++ does not mean thread-safe.
Do not make APIs unnecessarily asynchronous.
Make C++ operator > again
Trump is an idiot.
Reply
Thanks given by:




Users browsing this thread: 1 Guest(s)