macOS cleaning up my code

[ryanbowk]

hey guys, great forum. this is my first post, i have been learning objective c for a few weeks now and i have made a program to help me with work, its kind of a calculator. i programmed it in cmd line utility, foundation tool. i would like to get the program ready for a GUI. i have designed a GUI already in interface builder, but i know i need to do a lot of work on my code i.e creating IVars, methods, setters, and getters (i think) but working with objects seems a little confusing at first, although i am getting there. but could any body point me in the right direction as where to start with it. or just tell me i am bonkers and to start again i guess any response would be great goo or bad. anyhow here is the main file with my code

#import <foundation/Foundation.h>


int main(int argc, char const * argv [])
{
	NSAutoreleasePool * pool = [[NSAutoreleasePool alloc]init];
	
	float sand = 50;
	float totalNumWalls = 0;
	float wallLength = 0;
	float heightOfWall = 0;
	float areaOfMeters = 0;
	float pricePer = 0;
	float bagsPlaster = 10;
	char choice;
options///////////////////////////////////////////////////////////////////////////////

			NSLog(@"     Please choose an option...\n");
			NSLog(@"     --------------------------");
			NSLog(@"1)  Calculates the area, price, and amount of bags needed for internal skimming.");
			NSLog(@"2)  Calculates area, price, and amount of sand nedded for external render.");
			NSLog(@"3)  Calculates area, price, and amount of plaster needed for ceilings.");
			NSLog(@"q)  Quits the program.");
			scanf("%c", &choice);
-----------------------------------------------------------------------------options//

// price per squared meter/////////////////////////////////////////////////////////

		NSLog(@"Please enter a price per m2");
		scanf("%f",&pricePer);
	       NSLog(@"£%.2f", pricePer);
	   	switch (choice)
		{
//------------------------------------------------------------------------------price//

// calculates price, area, and amount of bags needed for internal skimming///

			case '1':
				NSLog(@"Please enter an overall height of the wall");
				scanf("%f", &heightOfWall);
				for(;;)
				{
					NSLog(@"Please enter length of wall. Enter a negative number to finish");
					scanf("%f",&wallLength);
					NSLog(@"%.1f", wallLength);
					if (wallLength  <= 0)
					{
						break;
					}
					totalNumWalls += wallLength;
				}
				NSLog(@"Area = %.1f sq'Meters", totalNumWalls * heightOfWall);
				NSLog(@"price = £%.2f", totalNumWalls * heightOfWall * pricePer);
				NSLog(@"amount of plaster needed = %.1f Bags", totalNumWalls * heightOfWall / bagsPlaster);
				break;
//------------------------------------------------------------------------skimming//

// calculates price, area, and amount of sand needed for external rendering//
				
			case '2':
				for(;;)
				{
					NSLog(@"please enter length of wall. Enter a negative number to finish ");
					scanf("%f",&wallLength);
					if (wallLength <=0)
					{
						break;
					}
					NSLog(@"please enter height of wall.");
					scanf("%f", &wallLength);
					areaOfMeters = wallLength * wallLength;
					//totalNumWalls += areaOfMeters;
				}
				NSLog(@"Area = %.1f sq'Meters", areaOfMeters /*totalNumWalls*/); 
				NSLog(@"price = £%.2f", areaOfMeters /*totalNumWalls*/ * pricePer);	
				NSLog(@"Amount of sand needed = %.1f Ton", areaOfMeters /*totalNumWalls*/ / sand);
				NSLog(@"\n");          
				break;
//------------------------------------------------------------------------rendering//

// calculates price, area, and amount needed for single areas or ceilings/////

			case '3':
				for(;;)
				{
					NSLog(@"please enter length of ceiling. Enter a negative number to finish ");
					scanf("%f",&wallLength);
					if (wallLength <=0)
					{
						break;
					}
					NSLog(@"please enter width of ceiling.");
					scanf("%f", &wallLength);
					areaOfMeters = wallLength * wallLength;
					//totalNumWalls += areaOfMeters;
				}
				NSLog(@"Area = %.1f sq'Meters", areaOfMeters /*totalNumWalls*/); 
				NSLog(@"price = £%.2f", areaOfMeters /*totalNumWalls*/ * pricePer);	
				NSLog(@"amount of plaster needed = %.1f Bags ", areaOfMeters /*totalNumWalls*/ / bagsPlaster);
				NSLog(@"\n");          
				break;
//---------------------------------------------------------------------------ceilings//
				
// default option if the user enters a wrong option///////////////////////////////
		
			default:
				NSLog(@"That is not an option");
//---------------------------------------------------------------------------default//		
		
	}
	[pool drain];
	
	return 0;
	
	
	
}

 

[larswik]

I'm probably not the right person to respond to this, since I am also learning, so take my response with a grain of salt. Your float variables are not float's, they are integers like float sand = 50; should be float sand = 50.0; I don't know that that will make any difference, but grammatically you should have 50.0

As for working with objects I don't see any objects in there? You have not defined any classes to create objects from? I just went through this a couple days ago with my code. I wrote it all in a procedural style like you did.

Why is it so indented like that?

Is this your first language you are learning?

Keep working at it but work slow. I only created my first GUI after a year of learning coding. Develop good skills, like I am trying to.

It's a lot to learn.

 

[chrono1081]

Yikes. I wish I remembered more Objective-C (I'm mainly C++)

Some quick things I see:

1. Formatting. (It COULD be the forums making your code look worse than it is, so if it is disregard).

First, I see your variables all up top which is fine, however they are declared as floats and even though it doesn't matter if you assign an int to it, it makes for better readability to assign them as floats, for example:

float sand = 50.0;

instead of

float sand = 50

By seeing the decimal, I know instantly its either a float or double.

The second part, the menu, could be put into a custom function (sorry I forget the syntax in Objective-C) that way you can call just one line instead of 6.

Something like: displayMenu();

instead of the 6 lines you have for the menu.

The other stuff is a bit hard to explain without remembering my Objective-C syntax but one thing I noticed is lines like this:

NSLog(@"price = £%.2f", areaOfMeters /*totalNumWalls*/ * pricePer);

that doesn't even look like legal syntax to me. I would suggest not putting comments inside of statements, but instead after them. Like this:

NSLog(@"price = £%.2f", areaOfMeters * pricePer); //totalNumWalls

This way its easy to tell what the calculation is. The way you had it is fairly confusing since an astrisk and slash can mean multiplication or division or it can be a statement. Also for single line statements use // instead of /*, its easier to read.

 

[SatyMahajan]

Fundamentally you want to separate View code, Data (Model) code, and Controller code.

Right now everything is all grouped together, which will make it difficult to transition to GUI without an entire rewrite. In your case, you aren't saving anything, so you don't really need data code, just View/Controller.

Here's what I would suggest:

1.) turn your calculations into functions
2.) In order to do 1, you'll need to first gather the data from the user, store it, AND THEN calculate using functions that take that data as inputs. Don't perform the calculations as your getting data from the user.
3.) print out the returned value of the functions instead of the calculation within NSLog.

This does a couple things.

1.) Separates your View (scanf and NSLog right now) code from your actual calculations.
2.) Since calculations are separated from scanfs and NSLogs, you can easily replace those with text boxes and labels in a GUI.

I hope this helps.

 

[PatrickCocoa]

Welcome!

Welcome to the club, the membership secretary will show you the secret handshake on the way out.

Keep plugging away, work on projects you enjoy, keep asking questions, you'll run into roadblocks and frustration but be rewarded with elation and a sense of accomplishment.

One thing, your code is very linear, I'm-in-control-ish. In Cocoa, Cocoa is in control, Cocoa controls the run loop, your code just sits there and is called by Cocoa when Cocoa feels it's appropriate. You'll need to change to a style where you write smaller methods that Cocoa calls. And as SatyMahajan suggests, these smaller methods should be split between Views and Models.

If that make no sense, just ignore it - put it on your calendar to read in six months.

 

[ryanbowk]

Yikes. I wish I remembered more Objective-C (I'm mainly C++)

Some quick things I see:

1. Formatting. (It COULD be the forums making your code look worse than it is, so if it is disregard).

First, I see your variables all up top which is fine, however they are declared as floats and even though it doesn't matter if you assign an int to it, it makes for better readability to assign them as floats, for example:

float sand = 50.0;

instead of

float sand = 50

By seeing the decimal, I know instantly its either a float or double.

The second part, the menu, could be put into a custom function (sorry I forget the syntax in Objective-C) that way you can call just one line instead of 6.

Something like: displayMenu();

instead of the 6 lines you have for the menu.

The other stuff is a bit hard to explain without remembering my Objective-C syntax but one thing I noticed is lines like this:

NSLog(@"price = £%.2f", areaOfMeters /*totalNumWalls*/ * pricePer);

that doesn't even look like legal syntax to me. I would suggest not putting comments inside of statements, but instead after them. Like this:

NSLog(@"price = £%.2f", areaOfMeters * pricePer); //totalNumWalls

This way its easy to tell what the calculation is. The way you had it is fairly confusing since an astrisk and slash can mean multiplication or division or it can be a statement. Also for single line statements use // instead of /*, its easier to read.

yes sorry about that i intended to remove the statments. they are there purly because when i first wrote the code they were part of it, then i decided to try the code without them so instead of deleting them straight away i turned them into statements incase i needed them again noob mistake i guess.
and as for the float int advise yes it would appear i do need a decimal "doh". any way your responses have been tremendously helpful thanks.