Passing vectors to c++ function

[28daniela28]

Hi,

I'm trying to end a project that should do next points:
-generating a random vector ( for that I have the function generareVector);
-reading the vector from the file where it was generated ( citireVector);
-sorting the vector using different methods of sort;
-checking if the resulted vector was well sorted (verificareCorectitudine);
-timing each method of sort ();

I implemented 6 methods separately and everything works fine ( I am declaring the vector as a global variable) but when I'm trying to put all together (I should pass the initial vector to each sorting method and then each sorting method should return it's own results that I should print). I thought that I should consider the vector as a pointer but certanely I'm doing it wrong... Could you suggest me what am I doing wrong? Thank you and have a nice evening!

Quick sort (separately):

#include<iostream>
#include<fstream>
#include<ctime>
#include<cstdlib>  
#include<conio.h>

using namespace std;

	int n, v[10000];
	bool corect = true;

int generareVector(){
  srand((int) time(0));
  int lowest = 1;
  int highest = 10000;
  int range = (highest - lowest) + 1;
  int n = lowest+int(range*rand()/(RAND_MAX + 1.0));
  ofstream sortari;
  sortari.open("C:\\Documents and Settings\\S&D\\Desktop\\sortari.txt");
  sortari<<n<<endl;
  for(int i = 0; i < n; i++){
	int index = lowest+int(range*rand()/(RAND_MAX + 1.0));
	sortari<<index<<endl;
  }
  	sortari.close();
	return 0;
}

int  citireVector(){

	ifstream reading;
	reading.open("C:\\Documents and Settings\\S&D\\Desktop\\sortari.txt");
	reading>>n;
	for(int i = 0; i<n; i++){
		reading>>v[i];
	}
	reading.close();
	return 0;
}

int sortare_rapida(int i, int j){
	bool schimb = 0;
	int aux;
	int a = i; int b = j;
	if(j <= i) return 0;
	else{
	while(i<j){
		if(v[i] > v[j]){
			aux = v[i];
			v[i] = v[j];
			v[j] = aux ;
			schimb = !schimb;
		};
		if(schimb == 0) i++;
		else j--;
	};

	sortare_rapida(a, i-1); 

	sortare_rapida(i+1, b); 
	
	};
	return 0;
};


int verificareCorectitudine(){
	
	int i = 0;
	do{
		if(v[i] > v[i+1])
			corect = false;
		i++;
	}
	while(corect == true && i < n-1 );
	return 0;
}

int afisareVectorSortat(int timeQS){
	ofstream results;
	results.open("C:\\Documents and Settings\\S&D\\Desktop\\sortari.txt");
	results<<"Rezultate pentru QuickSort:"<<endl;
	for(int i=0; i<n; i++){
		results<<v[i]<<endl;
	}
	if(corect)
		results<<"Vector sorted"<<endl;
	results<<timeQS<<endl;
	results<<"\n";

	return 0;
}

int main ()
{
  generareVector();
  citireVector();

  int timeBQS = clock();
  sortare_rapida(0, n-1);
  int timeAQS = clock();
  verificareCorectitudine();

  int timeQS = timeAQS - timeBQS;

  afisareVectorSortat(timeQS);

  return 0;
}

QuickSort and Selection together:

#include<iostream>
#include<fstream>
#include<ctime>
#include<cstdlib>  
#include<conio.h>

using namespace std;

	int n;
	bool corect = true;

int generareVector(){
  srand((int) time(0));
  int lowest = 1;
  int highest = 10000;
  int range = (highest - lowest) + 1;
  int n = lowest+int(range*rand()/(RAND_MAX + 1.0));
  ofstream sortari;
  sortari.open("C:\\Documents and Settings\\S&D\\Desktop\\sortari.txt");
  sortari<<n<<endl;
  for(int i = 0; i < n; i++){
	int index = lowest+int(range*rand()/(RAND_MAX + 1.0));
	sortari<<index<<endl;
  }
  	sortari.close();
	return 0;
}

int * citireVector(){

	int v[10000];
	ifstream reading;
	reading.open("C:\\Documents and Settings\\S&D\\Desktop\\sortari.txt");
	reading>>n;
	for(int i = 0; i<n; i++){
		reading>>v[i];
	}
	reading.close();
	return v;
}

int sortare_rapida(int i, int j, int * v){
	bool schimb = 0;
	int aux;
	int a = i; int b = j;
	if(j <= i) return 0;
	else{
	while(i<j){
		if(v[i] > v[j]){
			aux = v[i];
			v[i] = v[j];
			v[j] = aux ;
			schimb = !schimb;
		};
		if(schimb == 0) i++;
		else j--;
	};

	sortare_rapida(a, i-1, v); 

	sortare_rapida(i+1, b, v); 
	
	};
	return 0;
};

int * selectie( int * v){
	int min, aux, poz, i, j;
	for(i=0; i<n-1; i++){
		min = v[i];
		poz = i;
		for(j=i+1; j<n; j++){
			if(v[j] < min){
				min = v[j];
				poz = j;
			}
		}
		if(min != v[i]){
			aux = v[i];
			v[i] = v[poz];
			v[poz] = aux;
		}
	}
return v;
}

int verificareCorectitudine(int * v){
	
	int i = 0;
	do{
		if(v[i] > v[i+1])
			corect = false;
		i++;
	}
	while(corect == true && i < n-1 );
	return 0;
}

int afisareVectorSortat(int * v, int * vv, int timeS, int timeQS){
	ofstream results;
	results.open("C:\\Documents and Settings\\S&D\\Desktop\\sortari.txt");
	results<<"Results for QuickSort:"<<endl;
	for(int i=0; i<n; i++){
		results<<v[i]<<endl;
	}
//	results<<corect<<endl;
	results<<timeQS<<endl;
	results<<"\n";
	results<<"Rezultate pentru selectie:"<<endl;
	for(int i=0; i<n; i++){
		results<<vv[i]<<endl;
	}
	results<<timeS;
	results.close();
	return 0;
}

int main ()
{
	int * vector;
	int * vectorS;
  generareVector();
  vector = citireVector();

  int timeBS = clock();
  vectorS = selectie(vector);
  int timeAS = clock();
  //verificareCorectitudine(vectorS);
  int timeS = timeAS - timeBS;

  int timeBQS = clock();
  sortare_rapida(0, n-1, vector);
  int timeAQS = clock();
  //verificareCorectitudine(vector);
  int timeQS = timeAQS -timeBQS;

  afisareVectorSortat(vector, vectorS, timeS, timeQS);

  return 0;
}

[tetron]

I haven't gone through all your code yet, and there are some dangerous bits of code that are probably not currently causing an issue.

If you have a function outside of a class it is best to pass in a variable to a function rather than access a global directly.

Then there are 2 options when you wish to set a variable one is to change it in the function and the other is to use a return type;

When you are saving n have you checked that the value is what you want.

A more sensible approach would be to use std::vector rather than v[10000] this makes your life easier

your sort selectie appears to be overwriting data before you are finished with it

A class more like this might help:

#include <vector>
#include <string>
class random_vector()
{
public:
random_vector();
std::vector<int> generate_vector(int lowest, int highest);
bool save_vector(std::string &file, std::vector<int> &vi);
std::vector<int> load_vector(std::string &file_name);
std::vector<int> sort_vector(std::vector<int> &vi);
};

the & is called a reference and lets you change a value inside a function

line 53 seems odd to be flipping the bool rather than setting it

by line 137 both vector & vectorS are the same as one another

Hi,

afisareVectorSortat(vector, vectorS, timeS, timeQS);

this would be better if you use more than one container especially one that is designed for the job such as vector. So that you have copies rather than just one vector. I would thoroughly check that n is the value that you expect.

[28daniela28]

I haven't gone through all your code yet, and there are some dangerous bits of code that are probably not currently causing an issue.

If you have a function outside of a class it is best to pass in a variable to a function rather than access a global directly.

Then there are 2 options when you wish to set a variable one is to change it in the function and the other is to use a return type;

When you are saving n have you checked that the value is what you want.

A more sensible approach would be to use std::vector rather than v[10000] this makes your life easier

your sort selectie appears to be overwriting data before you are finished with it

A class more like this might help:

#include <vector>
#include <string>
class random_vector()
{
public:
random_vector();
std::vector<int> generate_vector(int lowest, int highest);
bool save_vector(std::string &file, std::vector<int> &vi);
std::vector<int> load_vector(std::string &file_name);
std::vector<int> sort_vector(std::vector<int> &vi);
};

the & is called a reference and lets you change a value inside a function

line 53 seems odd to be flipping the bool rather than setting it

by line 137 both vector & vectorS are the same as one another

this would be better if you use more than one container especially one that is designed for the job such as vector. So that you have copies rather than just one vector. I would thoroughly check that n is the value that you expect.

Hi,
thanks a lot for looking over my code!
In fact I've never used the predefined c++ class <vector> so that's why it was easier for me to use v[10000], but I understood your ideea that it makes my life easier:) so I'm going to read more about it so I can use it.
Secondly, I dared to let n as global variable because I never change it during the execution of my program, I'm only using it ( it represents the no of elements for my vector), but I'll pass it as a variable to functions that need it.
Meanwhile, I also made a debug for a vector having n = 10 elements, and yes n has the same value along the entire execution of the prog. And you're right about the values of vector and vectorS, theu're the same. The code for which I made the debug is the next one:

#include<iostream>
#include<fstream>

using namespace std;

int n, v[100];

int citire(){
	ifstream citesc;
	citesc.open("C:\\Documents and Settings\\S&D\\Desktop\\vectori.txt");
	citesc>>n;
	for(int i=0; i<n; i++){
		citesc>>v[i];
	}
	return 0;
}

int selectie(){
	int min, aux, poz, i, j;
	for(i=0; i<n-1; i++){
		min = v[i];
		poz = i;
		for(j=i+1; j<n; j++){
			if(v[j] < min){
				min = v[j];
				poz = j;
			}
		}
		if(min != v[i]){
			aux = v[i];
			v[i] = v[poz];
			v[poz] = aux;
		}
	}
return 0;
}
int afisare(){
	ofstream afisez;
	afisez.open("C:\\Documents and Settings\\S&D\\Desktop\\vectori_afisare.txt");
	for(int i=0; i<n; i++)
		afisez<<v[i]<<" ";
return 0;
}

int main(){
	citire();
	selectie();
	afisare();
	return 0;
}

What it seemed wired to me was that when during the execution of functions citire and selectie I had good values for v, but when accesing afisare... I had other values that seem more like adresses rather than my values.

[28daniela28]

Hi,
thanks a lot for looking over my code!
In fact I've never used the predefined c++ class <vector> so that's why it was easier for me to use v[10000], but I understood your ideea that it makes my life easier:) so I'm going to read more about it so I can use it.
Secondly, I dared to let n as global variable because I never change it during the execution of my program, I'm only using it ( it represents the no of elements for my vector), but I'll pass it as a variable to functions that need it.
Meanwhile, I also made a debug for a vector having n = 10 elements, and yes n has the same value along the entire execution of the prog. And you're right about the values of vector and vectorS, theu're the same. The code for which I made the debug is the next one:

#include<iostream>
#include<fstream>

using namespace std;

int n, v[100];

int citire(){
	ifstream citesc;
	citesc.open("C:\\Documents and Settings\\S&D\\Desktop\\vectori.txt");
	citesc>>n;
	for(int i=0; i<n; i++){
		citesc>>v[i];
	}
	return 0;
}

int selectie(){
	int min, aux, poz, i, j;
	for(i=0; i<n-1; i++){
		min = v[i];
		poz = i;
		for(j=i+1; j<n; j++){
			if(v[j] < min){
				min = v[j];
				poz = j;
			}
		}
		if(min != v[i]){
			aux = v[i];
			v[i] = v[poz];
			v[poz] = aux;
		}
	}
return 0;
}
int afisare(){
	ofstream afisez;
	afisez.open("C:\\Documents and Settings\\S&D\\Desktop\\vectori_afisare.txt");
	for(int i=0; i<n; i++)
		afisez<<v[i]<<" ";
return 0;
}

int main(){
	citire();
	selectie();
	afisare();
	return 0;
}

What it seemed wired to me was that when during the execution of functions citire and selectie I had good values for v, but when accesing afisare... I had other values that seem more like adresses rather than my values.

Sorry, the code above is not the one for which I made the debug, the code above runs perfectly but my array is a global variable. The problems appears when I don't want it to be a global variable and I want to pass it to a function.
The code is this one:

#include<iostream>
#include<fstream>

using namespace std;

int n;

int * citire(){
	int v[100];
	ifstream citesc;
	citesc.open("C:\\Documents and Settings\\S&D\\Desktop\\vectori.txt");
	citesc>>n;
	for(int i=0; i<n; i++){
		citesc>>v[i];
	}
	return v;
}

int * selectie(int * v){
	int min, aux, poz, i, j;
	for(i=0; i<n-1; i++){
		min = v[i];
		poz = i;
		for(j=i+1; j<n; j++){
			if(v[j] < min){
				min = v[j];
				poz = j;
			}
		}
		if(min != v[i]){
			aux = v[i];
			v[i] = v[poz];
			v[poz] = aux;
		}
	}
return v;
}
int afisare(int * v){
	ofstream afisez;
	afisez.open("C:\\Documents and Settings\\S&D\\Desktop\\vectori_afisare.txt");
	for(int i=0; i<n; i++)
		afisez<<v[i]<<" ";
return 0;
}

int main(){
	int * vect;
	int * vectS;
	vect = citire();
	//vectS = selectie(vect);
	afisare(vect);
	return 0;
}

[tetron]

Your problem maybe in using [] when you use int * the compiler does not know the size of v[]

I think int [] also works but you want to move onto std::vector and references int &i a reference lets you change the number without needing pointers

try

int afisare(int * v, int sz)
{
int ret(-1);
 ofstream afisez;
 afisez.open("test.txt");
 if(afisez.is_open())
 {
   ret = 1;//file open
   for(int i=0; i<sz; i++)
   {
     afisez<< *v <<" ";
     ++v;
   }
  afisez.close();
  }
}

an example of a function using a vector

void display(std::vector<int> &v1)
{
   int sz = v1.size();
  for(int i = 0; i < sz; ++i)
 {
   std::cout << v1[i] << " " ;
 }
 std::cout << std::endl;
}

also close your open in cites

[28daniela28]

:) Thanks for the exemple with vector..I find it very usefull
I also tried to replace my afisare function with the one you typed for me...but I have exactly the same results...

[tetron]

I think first it is your save function

you have to first write n

int afisare()
{
    std::ofstream afisez;
    std::string fn("C:\\Documents and Settings\\test.txt");
    afisez.open(fn.c_str());
    if(afisez.is_open())
   {
//the missing feature
       afisez << n <<" ";
       for(int i=0; i<n; i++)
      {	
          afisez<<v[i]<<" ";
       }
       afisez.close();
   }
  else
  {
      std::cout << "could not open:" << fn <<  std::endl;
  }
return 0;
}

you should close() in citesr too
now my code works but you need a file to start the sort with
and that should be
sz vals

for example I used

10 0 3 6 9 2 5 8 1 4 7

and this gave

10 0 1 2 3 4 5 6 7 8 9

so the sort seems ok

:) Thanks for the exemple with vector..I find it very usefull
I also tried to replace my afisare function with the one you typed for me...but I have exactly the same results...