Fitting an image into a border c++

[CookieMaster]

I have an interesting one for you guys. I have the image properly loading from the array and file. I have the border box working properly (see response 3) My question is how do I get the image to fit and get inside of this border box that I have. I know that the size might need to be changed but it can easily be changed. I will post to .asc file to pastebin as well so you can see the image that I'm talking about. Thank you. Also, feel free to point out anything else I could do differently on here, make sure to include the code, I'm still learning  

 

https://pastebin.com/FEYKwJBY

 

#include "stdafx.h"
#include <cstdlib> 
#include <iostream>
#include <fstream>
#include <string>

using namespace std;

int main()
{
	int response;
	string newname; // This is for the user to enter a new name when saving a file.
	string filename = "cookies"; // Change this if different file name.
	string type = ".asc"; // Change this if different file type.
	string img; // File later needs to be passed to a string so it can be seen.
	string *array = nullptr; // Used for array.
	int arraySize = 0; // Used for array.
	ifstream File(filename + type); // Load file and file type stated above.
	// The following is an array, the file is loaded into the array before any option is chosen. This is so option 1 and 4 can both occur. 
	// This is messy but works. 

	{
		while (getline(File, img)) 
		{
			string *newArray = new string[arraySize + 1];
			for (int i = 0; i < arraySize; i++) 
			{
				newArray[i] = array[i];
			}
			newArray[arraySize] = img;
			arraySize++;
			array = newArray;
		}
	}
	// What the user sees.
	cout << "Welcome to Tx Paint (Tunxis Paint)" << '\n';
	cout << "[1] Load image" << '\n';
	cout << "[2] Draw line in image" << '\n';
	cout << "[3] Draw box in image" << '\n';
	cout << "[4] Save image" << '\n';
	cout << "[5] Exit" << '\n';
	cin >> response;

	if (response == 1)

	{
		for (int i = 0; i < arraySize; i++) 
		{
			cout << array[i] << '\n'; // Display from array.
		}
		main();

	}

	if (response == 2)

	{
		for (int i = 0; i < arraySize; i++)
		{
			cout << array[i] << '\n'; // Display from array.
		}
		main();
		// ToDo: Draw line through image.
	}

	if (response == 3)

	{
			for (int column = 0; column < 80; ++column)
			{
				cout << "*";
			}

			{
				cout << "\n";
			}

			for (int row = 0; row < 20; ++row)
			{
				cout << "*";
			
				for (int column = 0; column < 78; ++column)
				{
					cout << " ";
				}
				
				cout << "*\n";
			}
			for (int column = 0; column < 80; ++column)
			{
				cout << "*";
			}

			{
				cout << "\n";
			}
			main();
	}

	if (response == 4)

	{
		cout << "Enter a file name to save." << '\n';
		cin >> newname;
		ofstream OutFile(newname + type);
		{
			for (int i = 0; i < arraySize; i++) 
			{
				OutFile << array[i] << '\n'; // Save from array. 
			}
			
		}
		main();
	}

	if (response == 5)

	{
		exit;
	}

		return 0;
}

 

[CookieMaster]

Hate to double post but if you think their is an easier way to drawn a border around the image, also let me know

[Unimportant]

4 hours ago, CookieMaster said:

I have an interesting one for you guys. I have the image properly loading from the array and file. I have the border box working properly (see response 3) My question is how do I get the image to fit and get inside of this border box that I have. I know that the size might need to be changed but it can easily be changed. I will post to .asc file to pastebin as well so you can see the image that I'm talking about. Thank you. Also, feel free to point out anything else I could do differently on here, make sure to include the code, I'm still learning

 

You should not be calling main recursively, just use a loop.

And of course, you should be using std::vector in stead of manually handing the array, but you know that from last time.

 

As for a possible solution to the question:

	//loop trough all strings in array and find the longest one, remember it's length
	int longestLine = 0;
	for (int i = 0; i < arraySize; ++i)
	{
		if (array[i].length() > longestLine)
		{
			longestLine = array[i].length();
		}
	}

	//Use longest line length to draw our box...
	//std::setw requires #include <iomanip>
	std::cout << std::string(longestLine + 2, '*') << '\n';
	for (int i = 0; i < arraySize; ++i)
	{
        //pop_back removes the last character from the string, in this
        //case, the newline read from file.
		array[i].pop_back();
		std::cout << '*' << std::setw(longestLine) << array[i] << "*\n";
	}
	std::cout << std::string(longestLine + 2, '*') << '\n';

 

[CookieMaster]

2 hours ago, Unimportant said:

You should not be calling main recursively, just use a loop.

And of course, you should be using std::vector in stead of manually handing the array, but you know that from last time.

 

As for a possible solution to the question:

	//loop trough all strings in array and find the longest one, remember it's length
	int longestLine = 0;
	for (int i = 0; i < arraySize; ++i)
	{
		if (array[i].length() > longestLine)
		{
			longestLine = array[i].length();
		}
	}

	//Use longest line length to draw our box...
	//std::setw requires #include <iomanip>
	std::cout << std::string(longestLine + 2, '*') << '\n';
	for (int i = 0; i < arraySize; ++i)
	{
        //pop_back removes the last character from the string, in this
        //case, the newline read from file.
		array[i].pop_back();
		std::cout << '*' << std::setw(longestLine) << array[i] << "*\n";
	}
	std::cout << std::string(longestLine + 2, '*') << '\n';

 

Thanks, it works but with one slight problem. The text (the image) seems to get a little funky on the bottom, see the screenshots linked. 

 

[Unimportant]

5 hours ago, CookieMaster said:

Thanks, it works but with one slight problem. The text (the image) seems to get a little funky on the bottom, see the screenshots linked. 

 

My bad, the padding needs to be on the other side obviously, change the line in the second loop to:

std::cout << '*' << std::setw(longestLine) << std::left << array[i] << "*\n";

Added std::left to put the padding on the other side.

[Dat Guy]

Random OT:

 

using namespace std;

 

Please don't do that.

[stmfd sp!, {lr}]

1 hour ago, Dat Guy said:

Random OT:

 

using namespace std;

 

Please don't do that.

Nah, it's perfectly on-topic in my opinion and there's actually a lot more issues that must be addressed in OP's code.

OP:

1) Don't use int to represent any form of size. Use an unsigned type; most people would go for size_t.

2) Don't create a std::string pointer as it doesn't serve any purpose. If it's for an array, you should prefer an STL container. Strings are technically meant to be considered immutable hence why they are passed by const-reference so often but, if you know you're going to need a copy anyway, you should just copy it. You could actually move it (shallow copy it) if your intention is to just pass it and be done with it. If the strings are relatively short though, they'll be optimised via SSO and exist on the stack. So a move on an optimised string has the same overhead as copying anyway.

3) You never free any of the strings you create.

4) You've got a random {} scoped block. There is no benefit of having this. 

5) Use a switch statement.

6) I'm not 100% convinced that ostream will handle this for you but: in files, newlines actually differ between Windows and *nix. 

7) You've got no actual input validation or checking that things worked (e.g. checking std::cin's failbit, making sure file streams are valid, etc.).

[Unimportant]

11 minutes ago, stmfd sp!, {lr} said:

1) Don't use int to represent any form of size. Use an unsigned type; most people would go for size_t.
 

I don't really agree with this one. Unless the sizes you're dealing with are so huge you really need the extra bit, I like to keep them signed.

Somewhere, someplace, those sizes will eventually be used for some arithmetic / comparisons, etc, and before you know it you've got a signed/unsigned bug biting your ass.

 

Using unsigned to document a value can never be negative is a bad idea imo, and I agree with google's style guide on that one: Use a assert to test values that can never be negative for being negative, that will actually catch the value red handed being negative, in stead of being converted to some huge unsigned number and possibly escape detection.

[stmfd sp!, {lr}]

Just now, Unimportant said:

I don't really agree with this one. Unless the sizes you're dealing with are so huge you really need the extra bit, I like to keep them signed.

Somewhere, someplace, those sizes will eventually be used for some arithmetic / comparisons, etc, and before you know it you've got a signed/unsigned bug biting your ass.

 

Using unsigned to document a value can never be negative is a bad idea imo, and I agree with google's style guide on that one: Use a assert to test values that can never be negative for being negative, that will actually catch the value red handed being negative, in stead of being converted to some huge unsigned number and possibly escape detection.

No, you should always do it to show intention.

An auto in unsigned contexts (e.g. a simple loop) should yield unsigned and warn you of unsigned to signed comparisons anyway. 

[Unimportant]

11 minutes ago, stmfd sp!, {lr} said:

No, you should always do it to show intention.

An auto in unsigned contexts (e.g. a simple loop) should yield unsigned and warn you of unsigned to signed comparisons anyway. 

int i = 5;
unsigned int j = 6;
std::cout << i - j << std::endl; 

Does not give any warnings, but the result is not what you'd normally expect.

Off course, in a silly example like this it's easy to see what's happening, but in a large project a bug like this is simple to overlook when a lot of arithmetic is being done.

 

And like I said, it's a big help, while debugging, to see things that can't be negative being negative.

 

I like to get everything to signed as quickly as possible so arithmetic works as expected, many company style guides even require it for this reason.

 

But let's agree to disagree, this has been a long time standing debate anyway in which both sides are unlike to ever agree

[CookieMaster]

4 hours ago, stmfd sp!, {lr} said:

Nah, it's perfectly on-topic in my opinion and there's actually a lot more issues that must be addressed in OP's code.

OP:

1) Don't use int to represent any form of size. Use an unsigned type; most people would go for size_t.

2) Don't create a std::string pointer as it doesn't serve any purpose. If it's for an array, you should prefer an STL container. Strings are technically meant to be considered immutable hence why they are passed by const-reference so often but, if you know you're going to need a copy anyway, you should just copy it. You could actually move it (shallow copy it) if your intention is to just pass it and be done with it. If the strings are relatively short though, they'll be optimised via SSO and exist on the stack. So a move on an optimised string has the same overhead as copying anyway.

3) You never free any of the strings you create.

4) You've got a random {} scoped block. There is no benefit of having this. 

5) Use a switch statement.

6) I'm not 100% convinced that ostream will handle this for you but: in files, newlines actually differ between Windows and *nix. 

7) You've got no actual input validation or checking that things worked (e.g. checking std::cin's failbit, making sure file streams are valid, etc.).

I think I'm gonna need more clarification on that, sorry. I understand number 7, and I totally forgot to plug that in. 

What do you mean in 1? Do you mean the array size? I did have to get help on the array so if there is a better way let me know showing code. 

Number 2 i think I know where you are going at but do you mean on the top or more towards where the array is? (I'm sorry i can't think of proper words today) 

3. Don't know how to do that.

4. where did I use it, trying to figure out lol

5. Where do i use it

6. It does but thanks for the pointer, everything is tested on windows.

8 hours ago, Unimportant said:

My bad, the padding needs to be on the other side obviously, change the line in the second loop to:

std::cout << '*' << std::setw(longestLine) << std::left << array[i] << "*\n";

Added std::left to put the padding on the other side.

Thank you very very much. And I see why I shouldn't call back main, I already saw the issue of it displaying again and again...

 

6 hours ago, Dat Guy said:

Random OT:

 

using namespace std;

 

Please don't do that.

Why not?

[stmfd sp!, {lr}]

Just now, CookieMaster said:

I think I'm gonna need more clarification on that, sorry. I understand number 7, and I totally forgot to plug that in. 

What do you mean in 1? Do you mean the array size? I did have to get help on the array so if there is a better way let me know showing code. 

Number 2 i think I know where you are going at but do you mean on the top or more towards where the array is? (I'm sorry i can't think of proper words today) 

3. Don't know how to do that.

4. where did I use it, trying to figure out lol

5. Where do i use it

6. It does but thanks for the pointer, everything is tested on windows.

Thank you very very much. And I see why I shouldn't call back main, I already saw the issue of it displaying again and again...

 

Why not?

1) To show intention, if you've got something that is pretty much guaranteed to be unsigned, you should use an unsigned type. Signed would imply there can be negative numbers. You're not gonna have an, for example, array's length as -4 or something. It's always >= 0. Hence things like strlen() return size_t which is unsigned.

2) You could use an STL container instead of allocating everything on the heap manually and having to free it (which you haven't done).

3) You'd have to delete all of the strings you create. You loop and use "new" (allocate on the heap) and never free the allocations. 

4) The while getline loop.

5) Replace all the if statements checking the response with a switch statement.

[Dat Guy]

"Why not?": Because you clutter the main namespace. Good luck finding your methods if anything breaks. Or when adding a library which adds methods under identical names.

[fizzlesticks]

6 minutes ago, stmfd sp!, {lr} said:

Hence things like strlen() return size_t which is unsigned.

Which every C++ designer / implementer I've ever heard talk about it called a huge mistake. 

Don't make the same mistake.

Vote Signed.

[CookieMaster]

Bit stuck on where to start for my option 2, I suppose somehow I would get the length and divide it by 2 to get the middle then draw a line and same for height and divide it by 2

[CookieMaster]

Ignore this, solved the issue, see post above still.