C++ Image transformation (function help really)

[mkienbau]

Homework Statement

I'm making a program in C++ to take an image in .ppm format and either create its inverse or binary threshold.

2. The attempt at a solution

#include <iostream>
#include <fstream>
#include <cmath>
#include <cstdlib>	//for exit()
#include <string>
#include <vector>
using namespace std;

vector<vector<int> >binaryThreshold(vector<vector<int> > A);
vector<vector<int> >inverse(vector<vector<int> > A);

int main()
{
  string m_n;
  int i,j,w,h,maxvalue;
  int selection=0;
 

	// Filenames
	string inFilename = "";
	string outFilename = "";

	// For file I/O
	ifstream fin;	//input file stream
	ofstream fout;	//output file stream

	cout<<"Enter the name of the input file: ";
		cin>>inFilename;

	/**
	 * Handle file opening for reading
	 */
	fin.open(inFilename.c_str());	//open the input file

	// check for validity.
	if (!fin.is_open())
	{
		cout << "Problem opening file " << inFilename << " for reading." << endl;
		exit(1);
	}

	// read from input PPM file
	fin>>m_n>>w>>h>>maxvalue;
	// declare a vector of vectors of read-in dimensions
	vector<vector<int> >img(h,w);

	// fill vector of vectors with read-in pixel values
for(i=0; i<=(h); i++)
{
	for(j=0; j<=(w); j++)
	{
		fin>>img[i][j];
	}
}
	// close in-file
	fin.close();

	
	// Ask user for type of transformation

	while (selection<1 || selection>2)
	{
	cout<<"Transformation Options: "<<endl<<"1)Binary Threshold"<<endl<<"2)Inverse Transformation"<<endl;
	cout<<"Your selection: "<<endl;
	cin>>selection;

	if (selection==1)
	  {
		  	vector<vector<int> > binaryThreshold(vector< vector<int> > A);
	  }
	if (selection==2)
	  {
		    vector<vector<int> > inverse(vector< vector<int> > A);
	  }
	}
	
	/**
	 * Handle file opening for writing
	 */
	fout.open(outFilename.c_str());	//open the input file

	if (!fout.is_open())
	{
		cout << "Problem opening file " << outFilename << " for writing." << endl;
		exit(1);
	}

	// file opened okay... start writing

	// close file
	fout.close();

	return 0;
}

//your function definitions here
vector<vector<int> >binaryThreshold(vector<vector<int> > A)
{
	int i,j,w,h;
	for(i=0; i<=h; i++)
	{
		for(j=0; j<=w; j++)
		{
			if (A[i][j]<128)
			{
				return A[i][j]=0;
			}
			else
			{
				return A[i][j]=255;
			}
		}
	}
}
vector<vector<int> >inverse(vector<vector<int> > A)
{
int i,j,w,h;
	for(i=0; i<=h; i++)
	{
		for(j=0; j<=w; j++)
		{
			A[i][j]=A[i][j]-255;
		}
	}
}

I'm not really sure how to use functions properly (thats probably VERY obvious). I'm just looking for a push in the right direction here, the rest of the program seems to run just dandy.

Pretty much, I'm confused about how to take the variables from main () and apply them to the function, is it necessary to re-declare them in the function itself?

Additionally, I can read the original image vector just fine (I think at least, I tested the read of the magic number, dimensions, etc by outputting them.) However, I think my modification process is incorrect in terms of reading the original vector in the function, and assigning the new values to a new vector, or do i just re-assign the values within the same original vector of "img"?

I think those are the main problem areas within the program itself, any other tips would be great, thanks!

 

[mkienbau]

Anyone have any tips? I figured out I'm supposed to use pass by reference rather than by value as I've implemented in here. Still doesn't work though.

 

[Hurkyl]

Let's look at binaryThreshold.

---------------------------------------------------------------
Problem 1:

The declaration
vector<vector<int> >binaryThreshold(vector<vector<int> > A)

is a contract that says "You will give me a vector<vector<int> > (and nothing else), and I will make a local copy. I will create a vector<vector<int> >, and give it to you."

Similarly
vector<vector<int> >binaryThreshold(const vector<vector<int> > &A)

is a contract that says "You will give me a vector<vector<int> > (and nothing else), and I will use it, without making any modifications. I will create a vector<vector<int> >, and give it to you."

and

void binaryThreshold(vector<vector<int> > &A)

is a contract that says "You will give me a vector<vector<int> > (and nothing else), and I will use it and may make changes to it. I will not return anything to you." I strongly suggest you use this option -- you don't want to be making unnecessary copies of large matrices!


Either way you wind up doing it, I hope you now recognize that
return A[j]=0;

is wrong: the expression A[j]=0 is a number. (More precisely, it is a reference to an element of A) So, it violates the contract when you try to return that number, since you promised to return a vector<vector<int> >.

Logically, it is wrong too, because you intended to do something to every element of A, but instead you tried to return after making just one change -- you want to make all the changes, and then return A. (Or, if you used the third declaration I mentioned above, you would return nothing)


---------------------------------------------------------------
Problem 2:

As you seem to be aware, the function binaryThreshold isn't aware of main's copy of the values w and h. You could write binaryThreshold to take those values as arguments, but there's a better way: A already knows its dimensions!

In particular, for any type T, vector<T> is aware of how big it is. So, the function call

A.size()

returns the number of vector<int>'s that A contains -- i.e. that's how you can obtain the number of rows in A.

Similarly, A[0] knows how many int's it contains, so A[0].size(). If you assume all of the rows have equal size, then that is exactly the number of columns in the matrix A represents.

caveat: if A is empty... that is, it has no rows... then A[0].size() has undefined behavior; it could crash your program. You might want to make sure A.size() > 0 before proceeding with the rest of your function. (Similarly, you ought to ensure that each row of A really does have the same length)


---------------------------------------------------------------
Problem 3:

This command doesn't actually work:

vector<vector<int> > img(h,w);

The contract for this constructor is:

vector<vector<int> > img( /* number of elements */, /* a vector<int> value */).

and this constructor would fill img with the specified number of copies of that value. So, that second argument shouldn't be an integer, but instead an actual row. You want something that looks like:

vector<vector<int> > img( h, vector<int>(w, 0) )

which sets each entry of img to be equal to a w-long vector of zeroes.

 

[mkienbau]

Thanks for explaining all of that, it really helped clear up a lot! Its working too! Thanks again!