My Super Program - Page 4 - Overclock.net

dankoni · 05-28-08

Quote:

Originally Posted by Stormwolf

You can then structure your submenus within your worker functions similarly. Each task should be contained within the scope of its own function... they do teach you functions in your programming class right?

'do_stuff()' and 'do_more_stuff()' are not good names btw - they're just placeholders

He might not be at functions quite yet. IIRC, my class did loops before functions.

BassThatHz · 06-09-08

I can tell just from looking at the code that you are new to programming (in general).

It's good to see that you are using constants, don’t forget to use them for filenames too "Information.txt".

Note: This is all just constructive criticism BTW.

Here are a few tell-tale signs:
1) You don't have enough (good) code comments.
Statements like "My Super program that does super'dy-duper things" isn't going to help anyone understand the purpose of why it exists. I understand that your excited for you app, but unless its breaking NSA codes or something... there really isn't anything "super" about cout'ing strings.

2) You threw everything into the main function, like a gigantic if-statement run-on novel.
There is an OO term called Abstraction, the idea is to encapsulate the implementation details away from the instance of its usage; to increase readability and re-use and reduce complexity. For example you don’t need to understand the implementation of NTFS and disk spin to use fstream; the code inside fstream and the OS/BIOS takes care of that. For non-techie: A customer in a restaurant, doesn’t need to know that their pancakes contain 20ml of baking-soda… so on and so forth.

3) Since everything is in the same function, you are forced to change variable names (like a,b,c,d,e) even though they all are similar in purpose (like a counter or array index).

4) No input validation or exception handling. What if the user types z for a menu choice?

Keep your main loop small, readable and abstracted; use loops, structures and functions to your advantage.

Here is a rough example of what I am talking about, much easier to read and maintain. Scriptorum thinks a lot like I do

I am not a C coder, but I have over 12yrs of programming experience so here goes...

off the seat of my pants.

Code:

struct AnswerType
{
int QuestionID;
string AnswerText;
bool IsValid;
bool ExitCode;
}

int main ()
{
        //Init ViewState
	AnswerType Answer;
	Answer.QuestionID = -1;
	Answer.AnswerText = “”;
	Answer.ExitCode = false;
	Answer.IsValid = true;

	//Main Render loop, ValidateAnswer sets ExitCode
	while(!Answer.ExitCode)
	{
		//formulate a question and render it
		//keeps re-asking until answer is valid
		Answer.QuestionID = RenderQuestion(Answer);

		//Get answer to question from user
		Answer.AnswerText = GetKeyboardAnswer(); 

		//makes sure user answers correctly
		ValidateAnswer(Answer);

		//if user answered correctly, do something		
		if (Answer.IsValid)
			ProcessValidAnswer(Answer);
	}
}

int RenderQuestion(AnswerType Answer)
{ 
	int AskedQuestion = 0; //Question picked at random; 0 default
         //Store Question List
	const int QuestionCount = 3;
	string[] Questions = new string[QuestionCount];
	Questions[0] = "What's fav color?";
	Questions[1] = "blaa blaa blaa?";
	Questions[2] = "fdsdsdsaf?";

	if (!Answer.IsValid)
	{
		//re-ask the same question over and over
		AskedQuestion = Answer.QuestionID;
	}
	else  
	{
		//formulate a new question and random
		AskedQuestion = rand()
	}

	//Render Question to screen
	cout << Questions[AskedQuestion] << endl;
	return AskedQuestion;
}

string GetKeyboardAnswer()
{ return cin; }

void ValidateAnswer(AnswerType& Answer)
{ 
  Const exitcode = 14;
  if (Answer.AnswerText == “”)
	Answer.IsValid = false;
  if (Answer.AnswerText == exitcode)
	Answer.ExitCode = true;
       Answer.IsValid = false;
}


void ProcessValidAnswer(AnswerType Answer)
{  switch(Answer.QuestionID)
   {
   case 0: calc_tenth_digit_of_PI_Etc();
   };
}

You can abstract it even further by breaking it into classes, interfaces and pointers.

06-09-08	#32 (permalink)
BassThatHz New to Overclock.net Join Date: Jun 2008 Posts: 1 Rep: 0 Unique Rep: 0 Trader Rating: 0	I can tell just from looking at the code that you are new to programming (in general). It's good to see that you are using constants, don’t forget to use them for filenames too "Information.txt". Note: This is all just constructive criticism BTW. Here are a few tell-tale signs: 1) You don't have enough (good) code comments. Statements like "My Super program that does super'dy-duper things" isn't going to help anyone understand the purpose of why it exists. I understand that your excited for you app, but unless its breaking NSA codes or something... there really isn't anything "super" about cout'ing strings. 2) You threw everything into the main function, like a gigantic if-statement run-on novel. There is an OO term called Abstraction, the idea is to encapsulate the implementation details away from the instance of its usage; to increase readability and re-use and reduce complexity. For example you don’t need to understand the implementation of NTFS and disk spin to use fstream; the code inside fstream and the OS/BIOS takes care of that. For non-techie: A customer in a restaurant, doesn’t need to know that their pancakes contain 20ml of baking-soda… so on and so forth. 3) Since everything is in the same function, you are forced to change variable names (like a,b,c,d,e) even though they all are similar in purpose (like a counter or array index). 4) No input validation or exception handling. What if the user types z for a menu choice? Keep your main loop small, readable and abstracted; use loops, structures and functions to your advantage. Here is a rough example of what I am talking about, much easier to read and maintain. Scriptorum thinks a lot like I do I am not a C coder, but I have over 12yrs of programming experience so here goes... off the seat of my pants. Code: struct AnswerType { int QuestionID; string AnswerText; bool IsValid; bool ExitCode; } int main () { //Init ViewState AnswerType Answer; Answer.QuestionID = -1; Answer.AnswerText = “”; Answer.ExitCode = false; Answer.IsValid = true; //Main Render loop, ValidateAnswer sets ExitCode while(!Answer.ExitCode) { //formulate a question and render it //keeps re-asking until answer is valid Answer.QuestionID = RenderQuestion(Answer); //Get answer to question from user Answer.AnswerText = GetKeyboardAnswer(); //makes sure user answers correctly ValidateAnswer(Answer); //if user answered correctly, do something if (Answer.IsValid) ProcessValidAnswer(Answer); } } int RenderQuestion(AnswerType Answer) { int AskedQuestion = 0; //Question picked at random; 0 default //Store Question List const int QuestionCount = 3; string[] Questions = new string[QuestionCount]; Questions[0] = "What's fav color?"; Questions[1] = "blaa blaa blaa?"; Questions[2] = "fdsdsdsaf?"; if (!Answer.IsValid) { //re-ask the same question over and over AskedQuestion = Answer.QuestionID; } else { //formulate a new question and random AskedQuestion = rand() } //Render Question to screen cout << Questions[AskedQuestion] << endl; return AskedQuestion; } string GetKeyboardAnswer() { return cin; } void ValidateAnswer(AnswerType& Answer) { Const exitcode = 14; if (Answer.AnswerText == “”) Answer.IsValid = false; if (Answer.AnswerText == exitcode) Answer.ExitCode = true; Answer.IsValid = false; } void ProcessValidAnswer(AnswerType Answer) { switch(Answer.QuestionID) { case 0: calc_tenth_digit_of_PI_Etc(); }; } You can abstract it even further by breaking it into classes, interfaces and pointers. Last edited by BassThatHz : 06-09-08 at 04:14 PM.

		Overclock.net - Overclocking.net > Software, Programming and Coding > Coding and Programming > Application Programming
My Super Program

Currently Active Users Viewing This Thread: 1 (0 members and 1 guests)