Can someone please clean up my code

Hi everyone, new member here. For my assignement I have to write a program that takes a couple of things like ISO and shutter speed and the works out the correct exposure time for digital camera. Here is what I have so far ( it is not finished ) . I have no previous C experience so to get this far I got a LOT of help from other people and Internet so this looks rather messy . I have learned a lot already and while its imprortant to get a good mark in this assignement more imprtantly I need to leran implementation of various commands. Can someone please go through what I have so far and maybe clean it up a little and if possible explain what was wrong with it. Much appreciated

Code:

#include <stdio.h>

typedef enum {FALSE, TRUE} BOOL;//This I do not know what it does. It looks unnecesary to me ?

void main(void)
{
// Mainline Variable Declarations
// FILE * output = stdout;
// FILE * input = stdin;

int option=0;

char name[20]; //length limited upto 20 chars only
printf("Enter name:");
scanf("%s",name);

//I have no idea how to separate these 3 options so they are not on one line 
printf("Choose from one of the options");
printf("1 - Enter ISO value");

printf("2 - Enter Shutter speed");

printf("3 - Help/n");

scanf("%d", &option);

if (option==1) goto label_1;
else;
if (option==2) goto label_2;
// Help bit I still haven't added
label_1:
// type int needed for 400,800,1600,3200
int ISO=0;
// type float needed for 2 2.8 4 5.6 8 11 16 22
float aperture=0;

BOOL ReadSpeed = FALSE;
BOOL ReadAperture = FALSE;

while ((ReadSpeed == FALSE) || (ReadAperture == FALSE))
{
if (ReadSpeed == FALSE)
{
// prompt user for ISO setting.
// The ONLY settings allowed are 50,100,200,
// 400,800,1600,3200.

printf("Hello %s Enter ISO settings: ", name);
scanf("%d",&ISO);
// flush the input stream in case of bad input
fflush(stdin);
printf("\n");

switch(ISO)
{
case 50:
printf("50 is selected ISO=%d\n", ISO);
ReadSpeed = TRUE;
break;

case 6:
case 8:
case 10:
case 12:
case 16:
case 20:
case 25:
case 32:
case 40:
case 64:
case 80:
case 100:
case 125:
case 160:
case 200:
case 250:
case 320:
case 400:
case 500:
case 640:
case 800:
case 1000:
case 1250:
case 1600:
case 2000:
case 2500:
case 4000:
case 5000:
case 6400:
// Shows how you can use multiple
// case statements together
printf("The selected ISO=%d\n", ISO);
ReadSpeed = TRUE;
break;

case 3200:
printf("3200 is selected ISO=%d\n", ISO);
ReadSpeed = TRUE;
break;

default:
printf("Invalid ISO=%d\nPlease Try Again\n", ISO);
ReadSpeed = FALSE;
break;
}
}

if ((ReadAperture == FALSE) &&
(ReadSpeed == TRUE))
{
label_2:
// input the Shutter Speed .
// The allowed values are : 2 2.8 4 5.6 8 11 16 22

printf("%s please Enter shutter speed: ",name);
scanf("%f", &aperture);
// flush the input stream in case of bad input
fflush(stdin);
printf("\n");

if (aperture == 1.2f)
{
printf("Selected apecture is=%2.1f\n", aperture);
ReadAperture = TRUE;
}
else if (aperture == 1.4f)
{
printf("Selected apecture=%2.1f\n", aperture);
ReadAperture = TRUE;
}
else if (aperture == 1.8f)
{
printf("Selected apecture=%2.1f\n", aperture);
ReadAperture = TRUE;
}
else if ((aperture == 2.0f) || (aperture == 2.8f) ||
(aperture == 4.0f) || (aperture == 5.6f) ||
(aperture == 8.0f) || (aperture == 11.0f) || (aperture == 16.0f) || (aperture == 22.0f) || (aperture == 32.0f) )
{
printf("The selected apecture=%2.1f\n", aperture);
ReadAperture = TRUE;
}
else if (aperture == 22.0f)
{
printf("Selected apecture=%2.1f\n", aperture);
ReadAperture = TRUE;
}
else
{
printf("Invalid Aperture Selected =%f\nPlease Try Again\n",
aperture);
ReadAperture = FALSE;
}
}
}

printf("\nThe Entered Vales are ISO=%d and Aperture=%2.1f\n", ISO, aperture);
{
printf("This is the end of program --- still in development");
}
}

Oh btw this needs to run on Unix system . If you have Unix do you mind checking if it runs properly please. Again much appreciated.

Ben

Download the context editor and retype everything into that and it will automatically start setting the right indentations, or go and clean it up yourself.

Start by properly indenting the code (your IDE might have a code beautifier).

Code:

typedef enum {FALSE, TRUE} BOOL;//This I do not know what it does. It looks unnecesary to me ?

Because your code uses TRUE and FALSE as "keywords" or constants and BOOL as a "datatype".

Code:

void main(void)

That should be int main(void).

Code:

char name[20]; //length limited upto 20 chars only

Use a constant, such as maxLenght.

Code:

if (option==1) goto label_1;
else;
if (option==2) goto label_2;

Put the code you want to happen if option is 1 or 2 right there and get rid of goto-s. Better yet, put that code in separate functions and call them here instead of goto-s.

The red semicolon is a mistake: because of that the second if is not part of an "else if" but starts a new if block.

Code:

char name[20]; //length limited upto 20 chars only

Also, it can only hold 19 characters, and the \n to terminate the string. Just so you can understand future bugs if you rely on this 20 char max.

I'm not sure if the latest C standard allows this, but as far as I know, the //... single line comments are C++ only. Use /* ... */ comments in C.

Originally Posted by cpjust

I'm not sure if the latest C standard allows this, but as far as I know, the //... single line comments are C++ only. Use /* ... */ comments in C.

They're allowed in C99 but not C90.

Originally Posted by Wraithan

Also, it can only hold 19 characters, and the \n to terminate the string. Just so you can understand future bugs if you rely on this 20 char max.

you mean \0, not \n.

Originally Posted by robatino

They're allowed in C99 but not C90.

Annoyingly, GCC and MSVC both allow them, so often you don't even realize you've done it.

Originally Posted by brewbuck

Annoyingly, GCC and MSVC both allow them, so often you don't even realize you've done it.

GCC will complain when given -ansi, so it's easy enough to just compile once with "-W -Wall -ansi -pedantic" when the code is done to purge these things.

Hi killer.
Im quite new to C aswell so the help that i give you might not be correct but ill do my best.

you need a #include <stdbool.h> aswell at the top.
Also as much as i hate fprintf u will lose marks if u dont use fprintf and fscanf.
in your 10 line or so you forgot to include & in the fscanf command just before name.
you can not use semicolons after else

It is also a bit difficult to read it when its not indented its best if u do that first, you will lose a lot of marks if u dont do it anyway.

plus i was on the impression that the psuedocode is due this week shouldnt you be working on that???

Originally Posted by juststartedC

you can not use semicolons after else

No true. It just doesn't do anything. If you have a semicolon after an else, you might as well delete the else statement

QuantumPete