Intermediate Homework 4 *UPDATED*

The Partridge Family were neither partridges nor a family. Discuss.
BerlingSwe
Posts: 13
Joined: June 17th, 2017, 5:46 pm
Location: Sweden

Intermediate Homework 4 *UPDATED*

Post by BerlingSwe » December 12th, 2017, 10:51 pm

Hello. I spent an hour or two doing the homework for tutorial 4 of intermediate series and was wondering if someone more experienced than me could take a look at my solution and give feedback. Thank you in advance.

Code: Select all

#include "stdafx.h"
#include <conio.h>
#include <fstream>

//Global variables
int index = 0;
int nameIndex = 0;
int ageIndex = 0;

//Names and ages
char names[10][10];
char ages[10][5];

namespace tomato
{
	void print(char* readChar)
	{
		for (; *readChar != 0; readChar++)
		{
			_putch(*readChar);
		}
	}

	void read(char* readChar)
	{
		for (char c = _getch(); c != 13; c = _getch(), readChar++)
		{
			_putch(c);
			*readChar = c;
		}
		*readChar = 0;
	}

	int str2int(char* pLeft)
	{
		char* pRight = pLeft;
		for (; *pRight >= '0' && *pRight <= '9'; pRight++);
		pRight--;

		int sum = 0;
		int multiplier = 1;

		for (; pLeft <= pRight; pRight--)
		{
			sum += (*pRight - '0') * multiplier;
			multiplier *= 10;
		}
		return sum;
	}

	void strrev(char* pl)
	{
		char* pr = pl;

		for (; *pr != 0; pr++);
		pr--;

		for (; pl < pr; pl++, pr--)
		{
			char temp = *pl;
			*pl = *pr;
			*pr = temp;
		}
	}

	void int2str(char* buf, int val, int size)
	{
		char* const bufStart = buf;
		char* pEnd = buf + size;
		for (; (buf + 1) < pEnd && val > 0; val /= 10, buf++)
		{
			*buf = '0' + val % 10;
		}
		*buf = 0;

		strrev(bufStart);
	}
}

using namespace tomato;

void Load()
{
	//Ask the user to enter the file they want to load. Has to enter whole filename + extension
	char load[15];
	print("Enter the file you want to load: ");
	read(load);
	print("\n");

	std::ifstream in(load);

	//Put characters from file to screen
	for(char c = in.get(); in.good(); c = in.get())
	{
		_putch(c);
	}
}

void Save()
{
	//Ask the user to enter the name of the file they want to save to. Has to enter whole filename + extension
	print("Enter the name of the file you want to save to: ");
	char save[15];
	read(save);

	std::ofstream out(save);

	//Print names and ages in pair by global index
	for (int i = 0; i < index; i++)
	{
		//Print the names to the save file
		for (int j = 0; j < sizeof(names[i]); j++)
		{
			out.put(names[i][j]);
		}
		out.put(32);

		//Print the ages to the save file
		for (int k = 0; k < sizeof(ages[i]); k++)
		{
			out.put(ages[i][k]);
		}
		out.put(10);
	}
}

void Add()
{	
	//Reset the nameIndex and ageIndex every time we add new names and ages
	nameIndex = 0;
	ageIndex = 0;

	//Add name
	char addName[10];
	print("Enter a name: ");
	read(addName);

	//Copy the added name char by char into the global name array
	for ( ; addName[nameIndex] != 0; nameIndex++)
	{
		names[index][nameIndex] = addName[nameIndex];
	}

	print("\n");

	//Add age
	char addAge[4];
	print("Enter age: ");
	read(addAge);

	//Copy the added age char by char into the global age array
	for (; addAge[ageIndex] != 0; ageIndex++)
	{
		ages[index][ageIndex] = addAge[ageIndex];
	}

	//Increase index of name and age by one
	index++;
}

void Quit()
{

}

void Print()
{
	//Print the names and ages
	for (int i = 0; i < index; i++)
	{
		print(names[i]);
		print(" ");
		print(ages[i]);
		print("\n");
	}
}

void Menu()
{
	print("\n(l)oad (s)ave (a)dd (q)uit (p)rint\n");

	//Get input from user and take action on it
	char input = _getch();
	switch (input)
	{
	case 'l':
		Load();
		Menu();
		break;
	case 's':
		Save();
		Menu();
		break;
	case 'a':
		Add();
		Menu();
		break;
	case 'q':
		print("Press any key to quit...");
		break;
	case 'p':
		Print();
		Menu();
		break;
	default:
		print("Invalid input");
	}
}

int main()
{
	Menu();
	
	while (!_kbhit());
	return 0;
}
Last edited by BerlingSwe on December 13th, 2017, 8:40 pm, edited 1 time in total.

User avatar
chili
Site Admin
Posts: 3948
Joined: December 31st, 2011, 4:53 pm
Location: Japan
Contact:

Re: Intermediate Homework 4

Post by chili » December 13th, 2017, 2:06 am

I will if you post a link to a repo.

GitHub or GitOUT :lol:
Chili

albinopapa
Posts: 4373
Joined: February 28th, 2013, 3:23 am
Location: Oklahoma, United States

Re: Intermediate Homework 4

Post by albinopapa » December 13th, 2017, 8:23 am

I know chili wants you to upload to github, so you should also do that if you want advice from The Chili. For what it's worth, here's my two cents.

In your Menu() function, you call Menu() in every case except 'q'. There are times where recursion is a good thing and makes sense, but I personally would opt for a while loop instead in this case. The reason being is each time a function is called, memory is used up. That being said, an algorithm that uses recursion IMO, should have a definite exit strategy. In this scenario, you do have a way out, user pressing 'q', but it's non deterministic as to how many times the user will choose an option. Again, not an issue for such a small program, but that's my outlook on recursion ( calling the function from within that function ).

In your Menu function, you only check for lower case letters, what happens if caps lock is on? You get a "Invalid input" message and the program ends. You could convert the upper case to lower case by checking if the input is between 'A' and 'Z' and if it is, add ' ' or 32 to the value to get the lower case version between 'a' and 'z'.

Code: Select all

    if(input >= 'A' && input <= 'Z')
        input += 32;
A different way would be make additional cases for the switch and allowing the upper to fall through to the lower case:

Code: Select all

switch(input)
{
    case 'L':
    case 'l':
        Load()
        break;
    case 'A':
    case 'a':
        Add()
        break;
};
This approach is more work though. The last thing would be to change to if/else if/else statements:

Code: Select all

if(input == 'a' || input == 'A')
    Add();
else if(input == 'l' || input == 'L')
    Load();
else
    "Incorrect input"
The biggest thing though is at this point, you should be using classes or structs, not globals. You can create a class that can store those values, names and ages. You could create an a simple class that stores a single name/age pair, then a separate class that stores an array of object of that class.

Code: Select all

class Database
{
    class Entry
    {
    public:
        Entry( char *pName, int Age );
    private:
        char name[10];
        int age;
    };
public:
    void AddEntry( char* pName, int Age );
    Entry entries[10]
};
You Database class can have the Load and Save functions.

Hope this helps.
If you think paging some data from disk into RAM is slow, try paging it into a simian cerebrum over a pair of optical nerves. - gameprogrammingpatterns.com

BerlingSwe
Posts: 13
Joined: June 17th, 2017, 5:46 pm
Location: Sweden

Re: Intermediate Homework 4

Post by BerlingSwe » December 13th, 2017, 5:51 pm

Alright. Here is the initial solution in Git: https://github.com/BerlingSwe/OLD_Homework4

And here is my reworked solution using classes: https://github.com/BerlingSwe/Homework-4

In my first solution, I did not use classes because I didn't really feel confident enough to use them and found it easier just creating functions, but after spending about 3 hours today trying to figure out how to get this all in classes I feel like I have learned alot, especially about classes. I have not yet implemented the save and load features because I don't have more time today and because I don't know how to do it yet, but I will figure it out. Thanks albinopapa for reviewing my code, really appreciate it!

I did use chilis repo for the homework as a small guide a tiny bit, just to get me in the right direction and I was quite clueless at the start on how I would get it all to work with classes.

albinopapa
Posts: 4373
Joined: February 28th, 2013, 3:23 am
Location: Oklahoma, United States

Re: Intermediate Homework 4 *UPDATED*

Post by albinopapa » December 13th, 2017, 9:10 pm

Hey, whatever works, just glad you got some learning in.
If you think paging some data from disk into RAM is slow, try paging it into a simian cerebrum over a pair of optical nerves. - gameprogrammingpatterns.com

albinopapa
Posts: 4373
Joined: February 28th, 2013, 3:23 am
Location: Oklahoma, United States

Re: Intermediate Homework 4 *UPDATED*

Post by albinopapa » December 14th, 2017, 8:32 am

Looks pretty good. Still got a global there lol.

I decided to mess around with this homework assignment and decided to make my own String class using a char array of 512 bytes for the buffer, char pBuffer[512]{}. Moved the string functions into this class and also kept a size variable, so when I figure out the size of the array, I cache it, that way when I want the length of the array, I use the cached version instead of iterating through the string again.

Honestly, it is a bit extra work, but it cleans up the code and is good practice working with classes if you want some extra practice.

Here's the class signature, you'll have to come up with the implementation on your own.

Code: Select all

class String
{
public:
    String();
    String(const String&) = default; // don't implement this
    String(String&&) = default; // don't implement this
    String( const char* Str );
    explicit String( const int Val );     // explicit means you have to construct a string from Val -> String valstr( 35 );

    // Useful for creating padding or as in the homework, a bunch of '========' for instance.
    String( const char C, int Count );  // Creates a string with the size of Count filled with the given value of C

    String& operator=(const String&) = default; // don't implement this
    String& operator=(String&&) = default; // don't implement this
    String& operator=( const int Val );

    int GetSize()const;
    
    int ToInt()const;
    String& Reverse();        // Use this version to change self
    String Reverse()const;  // Use this version to create a new string that is the reverse of self, don't change self.

    // This is the index operator, it allows you to treat a class as if it was the array itself
    // String a( "Hello" );
    // for(int i = 0; i < a.GetSize(); ++i)
    // {
    //     if( a[i] >= 'A' && a[i] <= 'Z' ) a[i] += 32;
    // }
    // result would be "hello"
    char& operator[]( int Index );        // Use this version to be able to change the value at Index
    char operator[]( int Index )const;  // Use this version to copy of the value at Index
    
    // I use this function to make the last element in the string a 0 and subtract 1 from size
    void PopBack();
    // I use this function to add a char to the end of the string and add one to size, 
    // this is how I get the size of a string to cache it
    void PushBack( const char C );

    //  I encourage you to only use these functions when necessary, 
    // instead create functions that take a String object, String& or const String&
    char* GetBuffer();           // This returns the buffer array and allows read/write access
    const char* GetBuffer();  // This returns the buffer array and allows only read access

public:
    static constexpr int maxStringLength = 512;
private:
    char buffer[maxStringLength]{};
    int size = 0;
};
If you don't want to or don't have time to mess with it then no worries, just wanted to throw it out there, maybe someone else will find it handy.
If you think paging some data from disk into RAM is slow, try paging it into a simian cerebrum over a pair of optical nerves. - gameprogrammingpatterns.com

BerlingSwe
Posts: 13
Joined: June 17th, 2017, 5:46 pm
Location: Sweden

Re: Intermediate Homework 4 *UPDATED*

Post by BerlingSwe » December 14th, 2017, 9:58 am

If you are talking about the global tomato::Database db; then I first had it inside my Menu function but when it was there, every time I added a new name it would delete itself as soon as I exitet the menu funcition. At this point I was using recursion so looking back I’m pretty sure that’s the reason (A new version of db is created every time the function is called). But I went the easy way and made it global. Now that I use a while loop instead I think it should be fine inside the menu fuction. Thanks again

I wil definitely check out and experiment with the code you posted!

User avatar
chili
Site Admin
Posts: 3948
Joined: December 31st, 2011, 4:53 pm
Location: Japan
Contact:

Re: Intermediate Homework 4 *UPDATED*

Post by chili » December 14th, 2017, 11:36 am

Noice, got a repo. Not sure if I can add anything that papa hasn't already covered, but I will take a look in a day or two and get back.
Chili

BerlingSwe
Posts: 13
Joined: June 17th, 2017, 5:46 pm
Location: Sweden

Re: Intermediate Homework 4 *UPDATED*

Post by BerlingSwe » December 18th, 2017, 3:34 pm

chili wrote:Noice, got a repo. Not sure if I can add anything that papa hasn't already covered, but I will take a look in a day or two and get back.
Hey. I cleaned the code up and implemented the save/load function. Mind taking a look?
https://github.com/BerlingSwe/Homework-4

albinopapa
Posts: 4373
Joined: February 28th, 2013, 3:23 am
Location: Oklahoma, United States

Re: Intermediate Homework 4 *UPDATED*

Post by albinopapa » December 18th, 2017, 11:02 pm

Code: Select all

void Menu()
{
	// If you are just doing this to get a feel for allocating memory, then ignore.
	// You don't have to though
	//Database* db = new Database;

	// You could just allocate on stack, it will automatically be destroyed when Menu() ends
	Database db;

	char buffer2[16];
	char buffer[16];
	char save[16];
	bool running = true;

	do
	{
		print("(l)oad (s)ave (a)dd (q)uit (p)rint\n");

		//Get input from user and take action on it
		char input = _getch();
		if (input >= 'A' && input <= 'Z')
		{
			input += 32;
		}
			
		switch (input)
		{
		case 'l':
			print("File loaded!\n");
			//db->Load();
			db.Load();
			break;
		case 's':
			print("File saved!\n");
			//db->Save();
			db.Save();
			break;
		case 'a':
			print("Enter a name: ");
			read(buffer);
			print("\nEnter last name: ");
			read(buffer2);
			_putch('\n');
			//db->Add(buffer, buffer2);
			db.Add(buffer,buffer2);
			break;
		case 'q':
			running = false;
			break;
		case 'p':
			//db->Print();
			db.Print();
			break;
		default:
			print("Invalid input");
		}
	} 
	while(running);

	//delete db;
}
I only bring this up because manual memory management can be a nightmare. Prefer stack allocation on things that can fit on the stack, especially temporaries. For things that are too large for the stack ( stack size is usually 1 or 2 MB for Visual Studio ), for now you can use new/delete, but later I'm sure chili will cover std::unique_ptr which is a better option for dynamic allocation. It encapsulates the delete in the destructor.

Code: Select all

		void Load()
		{
			std::ifstream in("savefile.txt");

			for (char c = in.get(); in.good(); c = in.get())
			{
				_putch(c);
			}
		}
I think your Load function is suppose to load entries into the database, not just print the text file.

The rest looks good.
If you think paging some data from disk into RAM is slow, try paging it into a simian cerebrum over a pair of optical nerves. - gameprogrammingpatterns.com

Post Reply