Thread: 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.

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.

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.

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?