how horrible is my programming style!

This is a discussion on how horrible is my programming style! within the C++ Programming forums, part of the General Programming Boards category; I have been dabbling with c++ for a while as a hobby as i would like to eventually program simple ...

  1. #1
    Registered User Mark S.'s Avatar
    Join Date
    May 2005
    Location
    England
    Posts
    16

    how horrible is my programming style!

    I have been dabbling with c++ for a while as a hobby as i would like to eventually program simple 2d games. Unfortunately being self taught combined with the fact that my maths is crap means that i basicly suck at this stuff.

    Anyway i know that this program is retarded and that the style and structure is probably abismal . Never the less i am over the moon with it.

    All it does is generate 2 random numbers and then stage by stage go through the process of working out the answer, using a carry if needed.

    As other beginners to c++ will probably agree at the moment for me its not about solving the problem in style but just solving no matter how ugly it looks.

    Code:
    //// Demonstrating subtraction with workings////
    
    #include<iostream>
    #include<iomanip>
    #include<time.h>
    #include<stdlib.h>
    using namespace std;
    
    void Subtraction(int num1, int num2);
    int numLength(int num);
    void NumInArray(int nums[], int num, int numLen);
    void ShowWorking(int nums1[], int num1Len, int nums2[], int num2Len);
    void OutputArray(int nums[], int arrayLength);
    void Pause(void);
    
    
    int main(void)
    {
    	
    	srand((unsigned int)time(NULL));
    
    	for(;;)
    	{
    		cout << "NEW SUM!!!!!\n";
    		int num1 = rand() % 1000 + 1;
    		int num2 = rand() % 1000 + 1;
    
    		if(num1 < num2)
    		{
    			int temp = num1;
    			num1 = num2;
    			num2 = temp;
    		}
    
    		Subtraction(num1, num2);
    	}
    
    	return(0);
    }
    
    void Subtraction(int num1, int num2)
    {
    	int num1Len = numLength(num1);
    	int num2Len = numLength(num2);
    
    	int nums1[16] = {0};
    	int nums2[16] = {0};
    
    	NumInArray(nums1, num1, num1Len);
    	NumInArray(nums2, num2, num2Len);
    	
    	ShowWorking(nums1, num1Len, nums2, num2Len);
    }
    	
    
    void NumInArray(int nums[], int num, int numLen)
    {
    	int mult = 10;
    
    	for(int i = 0; i < numLen; i++)
    	{
    		nums[i] = (num % mult) / (mult / 10);
    		mult *= 10;
    	}
    }
    
    
    int numLength(int num)
    {
    	int numLen = 0;
    	int mult = 1;
    
    	do
    	{
    		mult *= 10;
    		numLen++;
    	}while(num / mult != 0);
    
    	return(numLen);
    }
    
    void ShowWorking(int nums1[], int num1Len, int nums2[], int num2Len)
    {
    	int answer[16] = {0};
    
    	OutputArray(nums1, num1Len);
    	OutputArray(nums2, num1Len);
    	cout << "-----\n";
    	OutputArray(answer, num1Len);
    	cout << '\n';
    
    	for(int i = 0; i < num1Len; i++)
    	{
    		int count = 1;
    
    		if(nums1[i] >= nums2[i])
    		{
    			answer[i] = nums1[i] - nums2[i];
    		}
    		else	// Nick one from next door
    		{
    			if(nums1[i+1] > 0)
    			{
    				nums1[i+1] -= 1;
    				nums1[i] += 10;
    			}
    			else		// If there isnt one next door go further a field
    			{
    				do
    				{
    					count++;
    				}while(nums1[i + count] == 0);
    				
    				cout << "count " << count << '\n';
    				nums1[i] += 10;
    				nums1[i+count] -= 1;
    
    				for(int j = 1; j < count ; j++)
    				{
    					nums1[i+j] += 9;
    				}
    			}
    			answer[i] = nums1[i] - nums2[i];
    		}
    		
    		Pause();
    
    		OutputArray(nums1, num1Len);
    		OutputArray(nums2, num1Len);
    		cout << "-----\n";
    		OutputArray(answer, num1Len);
    		cout << '\n';
    	}
    
    }
    
    void OutputArray(int nums[], int arrayLength)
    {
    	for(int i = arrayLength - 1; i >= 0; i--)
    	{
    		
    		cout << nums[i];
    	}
    	cout << "\n";
    }
    
    void Pause(void)
    {
    	for(int i = 0; i < 50000; i++)
    	{
    		for(int j = 0; j < 30000; j++)
    		{
    		}
    	}
    }
    if anyone can point out any subtle changes i would be interested.

    Thanks
    Mark S.

  2. #2
    The superhaterodyne twomers's Avatar
    Join Date
    Dec 2005
    Location
    Ireland
    Posts
    2,262
    What's the purpose of Pause();? If it's solely to pause the program, I would recommend a Sleep(); function, in <windows.h> enter the number of miliseconds to pause the program for.

  3. #3
    Registered User
    Join Date
    Aug 2006
    Posts
    163
    First off, it's not really that bad for the size of the program and your skill level.
    One thing I would suggest would be commenting when you have closing braces after more than 4 or so lines of code.

    Code:
    for(int i = 0; i < num1Len; i++)
    	{
    		int count = 1;
    
    		if(nums1[i] >= nums2[i])
    		{
    			answer[i] = nums1[i] - nums2[i];
    		}
    		else	// Nick one from next door
    		{
    			if(nums1[i+1] > 0)
    			{
    				nums1[i+1] -= 1;
    				nums1[i] += 10;
    			}
    			else		// If there isnt one next door go further a field
    			{
    				do
    				{
    					count++;
    				}while(nums1[i + count] == 0);
    				
    				cout << "count " << count << '\n';
    				nums1[i] += 10;
    				nums1[i+count] -= 1;
    
    				for(int j = 1; j < count ; j++)
    				{
    					nums1[i+j] += 9;
    				}
    			}
    			answer[i] = nums1[i] - nums2[i];
    		}
    		
    		Pause();
    
    		OutputArray(nums1, num1Len);
    		OutputArray(nums2, num1Len);
    		cout << "-----\n";
    		OutputArray(answer, num1Len);
    		cout << '\n';
    	}
    where does the for loop end here?

    This is a good example of what I'm talking about. When you're skimming through the program, and you have lots of nested loops, it can get confusing.

    Next, when you only have one line of code after your condition, you don't need braces.

    Code:
    void OutputArray(int nums[], int arrayLength)
    {
    	for(int i = arrayLength - 1; i >= 0; i--)
    	{
    		
    		cout << nums[i];
    	}
    	cout << "\n";
    }
    would look better like this

    Code:
    void OutputArray(int nums[], int arrayLength)
    {
    	for(int i = arrayLength - 1; i >= 0; i--)		
    		cout << nums[i];
    
    	cout << "\n";
    }
    Hope that helped a bit.

  4. #4
    The superhaterodyne twomers's Avatar
    Join Date
    Dec 2005
    Location
    Ireland
    Posts
    2,262
    Quote Originally Posted by System_159
    Next, when you only have one line of code after your condition, you don't need braces.

    Code:
    void OutputArray(int nums[], int arrayLength)
    {
    	for(int i = arrayLength - 1; i >= 0; i--)
    	{
    		
    		cout << nums[i];
    	}
    	cout << "\n";
    }
    would look better like this

    Code:
    void OutputArray(int nums[], int arrayLength)
    {
    	for(int i = arrayLength - 1; i >= 0; i--)		
    		cout << nums[i];
    
    	cout << "\n";
    }
    Hmmm, I don't know if I'd agree with you on that one! You don't need them, as you say, but I generally prefer to leave them in, basically as a visual reminder of the scope of the loop. Alternatively, if you really wanted, you could do this::

    Code:
    for ( int i=0; i<x; i++ ) cout<< "\n"; // Just as a one liner
    of
    Code:
    for ( int i=0; i<x; cout<< "\n", i++ );
    I can never remember whether the latter works. I'm quite sure it does though.

    However, System, I do agree with your first point! if I had this (intentionally badly formatted!)::

    Code:
    int main()
    {
    for ( int i=0; i<24562; i++ )
    {
    if ( i%34 == 0 )
    {
    while ( i%34 != 0 )
    {
    cout<< "sdajkl";
    }
    }
    else if
    {
    whatever
    }
    }
    return 0;
    }
    nobody could deciher it!

    Code:
    int main()
    {
    for ( int i=0; i<24562; i++ )
    {
    if ( i%34 == 0 )
    {
    while ( i%34 != 0 )
    {
    cout<< "sdajkl";
    }
    } // end of if
    else if
    {
    whatever
    } // end of else
    } // end of for loop
    return 0;
    } // End of Main
    That's not to say I do it
    Last edited by twomers; 09-01-2006 at 08:49 AM.

  5. #5
    Code Goddess Prelude's Avatar
    Join Date
    Sep 2001
    Posts
    9,796
    >how horrible is my programming style!
    Your style is fine. Stop worrying about it. Consistency is key, and you're consistent.

    >if anyone can point out any subtle changes i would be interested.
    The only serious change I would suggest is to get rid of the antisocial pause. A busy wait loop is a poor way to pause the execution, and I honestly don't see the need for it in this program.
    My best code is written with the delete key.

  6. #6
    (?<!re)tired Mario F.'s Avatar
    Join Date
    May 2006
    Location
    Portugal
    Posts
    7,436
    >I can never remember whether the latter works. I'm quite sure it does though.

    It does, but it sure doesn't beat the readability of the previous one.
    The programmer’s wife tells him: “Run to the store and pick up a loaf of bread. If they have eggs, get a dozen.”
    The programmer comes home with 12 loaves of bread.


    Originally Posted by brewbuck:
    Reimplementing a large system in another language to get a 25% performance boost is nonsense. It would be cheaper to just get a computer which is 25% faster.

  7. #7
    The superhaterodyne twomers's Avatar
    Join Date
    Dec 2005
    Location
    Ireland
    Posts
    2,262
    Quote Originally Posted by Mario F.
    It does, but it sure doesn't beat the readability of the previous one.
    Agreed. But what are aesthetics where exe's are concerned?

  8. #8
    (?<!re)tired Mario F.'s Avatar
    Join Date
    May 2006
    Location
    Portugal
    Posts
    7,436
    > Agreed. But what are aesthetics where exe's are concerned?

    Let me put it this way... showing Armani on the outside will still not score you any brownie points if your underwear is dirty.
    The programmer’s wife tells him: “Run to the store and pick up a loaf of bread. If they have eggs, get a dozen.”
    The programmer comes home with 12 loaves of bread.


    Originally Posted by brewbuck:
    Reimplementing a large system in another language to get a 25% performance boost is nonsense. It would be cheaper to just get a computer which is 25% faster.

  9. #9
    The larch
    Join Date
    May 2006
    Posts
    3,573
    The code doesn't look too bad at all. And it compiles OK.

    The pause loop is not probably the best way to do it. I have a quite slow computer, and I'm not sure it is meant to wait so long between doing something.

    However, I don't understand the output. What I see is something like that:
    NEW SUM!!!!!
    999
    175
    ---------
    004 //<- some random value, mostly zeros.
    What you seem to be doing here might be called reinventing the wheel. While there is a good reason for a learner to reinvent the wheel (to understand how things work), if you want to use this for a more complex program, it would be a good idea to use whatever C++ has to offer to make the task easier.

    In this case I think you might take a look at stringstreams that make converting strings into other types and the other way round very easy. (You might even add that reference page to your favourites as it's full of useful stuff.)

    Code:
    for(;;)
    	{
    		cout << "NEW SUM!!!!!\n";
    		int num1 = rand() % 1000 + 1;
    		int num2 = rand() % 1000 + 1;
    
    		if(num1 < num2)
    		{
    			int temp = num1;
    			num1 = num2;
    			num2 = temp;
    		}
    
    		Subtraction(num1, num2);
    	}
    This loop doesn't look good. If you don't have any statements to put in the for loop then a while loop might look better. More importantly, this loop has no exit condition. It means, the program will never end properly (with return 0;).
    Last edited by anon; 09-01-2006 at 09:32 AM.

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. WM_CAPTION causing CreateWindowEx() to fail.
    By Necrofear in forum Windows Programming
    Replies: 8
    Last Post: 04-06-2007, 08:23 AM
  2. Button handler
    By Nephiroth in forum Windows Programming
    Replies: 8
    Last Post: 03-12-2006, 05:23 AM
  3. Which style of if loops is most common?
    By dwks in forum A Brief History of Cprogramming.com
    Replies: 38
    Last Post: 08-25-2005, 03:18 PM
  4. WS_EX_COMPOSITED style (double buffering) problems
    By JasonD in forum Windows Programming
    Replies: 2
    Last Post: 10-12-2004, 11:21 AM
  5. Tab Controls - API
    By -KEN- in forum Windows Programming
    Replies: 7
    Last Post: 06-02-2002, 09:44 AM

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