Click to See Complete Forum and Search --> : How's this for my first C program


Fryguy8
09-20-2004, 10:10 PM
Yes this is a homework assignment. Does anyone see any obvious mistakes or inefficiencies? any improvements?

This is my first C program, so be gentle ;)


/**************************************************
Bryan Alves
Assignment 1 - Manual Long Division
Takes 4 values:
-dividend and divisor: The division to do
-start and end: Will print the numbers from the division in this precision
**************************************************/

#include <stdio.h>

int main()
{
/* Declare the 4 main variables */
unsigned long int dividend, divisor, start, end;

/* Declare variables used for division */
unsigned long int quotient = 0, remainder = 0;
unsigned short int intLength = 0;

unsigned long int i;

/* Read Values */
printf("Dividend: ");
scanf("%d", &dividend);

printf("Divisor: ");
scanf("%d", &divisor);

printf("Start: ");
scanf("%d", &start);

printf("End: ");
scanf("%d", &end);

/* Verify validity of entered parameters */
if (start > end)
{
printf("Error: Start must be before end\n");
return 1;
}
if (start == 0)
{
printf("Error: Cannot start before the first digit\n");
return 1;
}

/*
The largest number of digits you can have after doing the first step
of division is something like 4 billion divided by 3, which yields
10 digits, plus one for the decimal, plus one for the null terminator.
The longest length you therefore need is 12.
*/

/* Wasting a couple bytes of memory is probably more efficient than
manually doing the first divison using a loop. */

char firstDivide[12] = {0,0,0,0,0,0,0,0,0,0,0,0};
char *p = firstDivide;

/* Calculate the quotient's integer part. */
quotient = dividend / divisor;
remainder = dividend % divisor;

/* Fill firstDivide and return the length actually used. */
intLength = sprintf(firstDivide, "%d", quotient);

/* Initialize the loop variable to begin after the decimal point. */
i = intLength + 2;

if (start <= intLength + 1) /* Start printing from within the first divison. */
{
/* Walk the array to the start position. */
p += start - 1;

if (end < intLength) /* We end before the decimal point. */
firstDivide[end] = '\0';
else if(end > intLength) /* We end after the decimal point. */
firstDivide[intLength] = '.';

printf(p);

}

else /* Skip numbers that are before the start of printing */
{
for (; i < start; i++)
{
/* No need to track quotient; it just gets thrown out. */
dividend = remainder * 10;
remainder = dividend % divisor;
}
}

/* These values we print, so do the division and print the numbers one by one. */
for (; i <= end; i++)
{
dividend = remainder * 10;
quotient = dividend / divisor;
remainder = dividend % divisor;
printf("%d", quotient);
}

return 0;
}

bryan.6
09-20-2004, 10:46 PM
that's your first program? wow... i'm impressed...
i think mine was like:

#include <stdio.h>

void main( void ) {
printf("Um... Yeah, first program...");
}

JayMan8081
09-20-2004, 10:56 PM
I don't know that the statement:
printf(p); will actually compile. Just one thing I noticed during a quick glance through.

Fryguy8
09-20-2004, 11:23 PM
it compiles and runs, i'm more or less just looking for style/efficiency comments. Can the code be made better?

fatTrav
09-20-2004, 11:43 PM
looks ok. getting console input and having to wait for it always looks yucky.

ah, my first C program was a kernel module that would crash the system by executing a fork bomb and print some colorful stuff to the logs. Fun times...

one bit of criticism ... commenting is good, but don't over do it. it looks like you might have commented a bit much, but that's better than not commenting enough.

Fryguy8
09-20-2004, 11:47 PM
fat, like I said, this is a homework assignment, so overcommenting is wanted. And asking for console input was the spec, so it's what I did.

I will agree with you however :)

Assigning the 12-element array seems a little janky, is there a better way to do it?

fatTrav
09-21-2004, 01:22 AM
It would be nice to allow the user to not input anything for the start/end and have the program return full precision.

I can't think of a better way to solve this off the top of my head than using the 12 char array. But then again I see no point in why you'd not always want full precision, but I'm sure the assignment was to be able to specify the precision.

Oh, wait. Maybe a better way to solve this would be to round the number to the nearest place for precision. I don't know off hand if C has rounding functions built-in. My K&R book does not mention them when they cover math.h (Aside, if not in math.h then *where* would they be?) I dislike C because it is too small of a language and also like C because it is a small language.

btw, travis or trav please. "fat" might refer to too many board members and cause confusion:D

truls
09-21-2004, 10:28 AM
What if the user is an idiot and enters zero as divisor?

Fryguy8
09-21-2004, 12:52 PM
The point was for exact precision, not rounding off. We were specifically instructed to not use any floating-point methods.

I think I'm just going to hand it in like this, seems pretty optimal to me.

Originally posted by fatTrav
It would be nice to allow the user to not input anything for the start/end and have the program return full precision.

I can't think of a better way to solve this off the top of my head than using the 12 char array. But then again I see no point in why you'd not always want full precision, but I'm sure the assignment was to be able to specify the precision.

Oh, wait. Maybe a better way to solve this would be to round the number to the nearest place for precision. I don't know off hand if C has rounding functions built-in. My K&R book does not mention them when they cover math.h (Aside, if not in math.h then *where* would they be?) I dislike C because it is too small of a language and also like C because it is a small language.

btw, travis or trav please. "fat" might refer to too many board members and cause confusion:D

fatTrav
09-21-2004, 02:32 PM
Originally posted by Fryguy8
The point was for exact precision, not rounding off. We were specifically instructed to not use any floating-point methods.

I think I'm just going to hand it in like this, seems pretty optimal to me.

ah, you have to love programming homework assignments. nothing as useful as a homework assignment :D And, from someone who graded assignments for the first two programming classes at my university, your program looks like full credit or damn well near it. And that's all that matters.

JohnT
09-21-2004, 02:44 PM
Excelent work, but I arrive at an entirely different solution. :p

evac-q8r
09-21-2004, 04:11 PM
The overcommenting is cool if you wish, however your comments are placed in very inconspicuous places and it' s hard to distinguish between comments and actual code. Like I would not comment where the else statement is and I'd space between the comments and variable declarations.

EVAC

bwkaz
09-21-2004, 06:48 PM
It is legal C to do this:

printf(p); assuming "p" is a char pointer.

However, it is not good style, and it does open your code up to certain types of security holes ("format string vulnerabilities"). Basically, if an attacker can change the string pointed to by p to contain percent characters and other format specifiers, then they can use that printf call to cause problems in your program.

A better way to print a string is the following:

printf("%s\n", p); where the \n isn't required.

This way, even if percent characters and whatnot get into p, it doesn't do any good -- they'll simply be printed out by printf as-is.

truls
09-22-2004, 05:44 AM
Ok, since I obviously suck at giving hints:

If divisor is zero your program will crash with a "Division by zero" error. You should always check user input for sanity.

So basically you should do a:
if( divisor!=0 )
before trying to use it as a divisor.

T.

blingbling!!
09-22-2004, 09:08 AM
Have you considered moving some of the code out into functions? This might demonstrate a nicer style, although for a little program like this it's probably not needed.

hth
--Robin

Fryguy8
09-22-2004, 10:21 AM
yah, i added a divison by 0 check before i handed it in. I also moved the actual work of doing the long division out to a function.

Thanks for the input guys.

ggforrest
09-27-2004, 08:05 AM
A more user friendly method of handling user input errors is the do...while loop. For example,

do
{
printf("Start: ");
scanf("%d", &start);

printf("End: ");
scanf("%d", &end);

if(start > end)
{
printf("Error: Start must be before end\n");
}

} while( start > end );