C++

Forum

 
Improving the code

Kernul (74)
Is it possible to improve this code?
I did it with the help of my friend and it works fine but I'm not so sure about the use of the struct in this case. Is there a way to have the same output without using the struct?
Thanks for your time!

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
 
/*
Create a program that, given a matrix, sums the number in lines and then order them in increasing order.
*/

#include <iostream>
#include <fstream>
#include <cstdlib>

using namespace std;

struct position{
	int summation;
	int pos;
};

//Prototypes Functions
void LoadMatrix(int ncol, int nrow, int** mat);
void RowSum(int nrow, int ncol, position* sum, int** mat);
void RowsOrder(int nrow, int ncol, position* sum);
void PrintMatrix(int nrow, int ncol, position* sum, int** mat);

fstream file;
const int N=100;

int main(int argc, char** argv) {
	int** mat;
	int ncol=0, nrow=0;
	char* row=new char [N];
	int numbmat=0;
	int vect[N];
	
	file.open("Matrix.txt", ios::in);
	if(!file) {
		cout<<"File not found!\n";
		exit(1);
	}
	
	int i=0;
	//Calculating number of lines.
	while(!file.eof()) {
		file.getline(row, 100);
		i++;
	}
	nrow=i;
	
	i=0;
	file.seekg(0);
	
	//Calculating all the number of the matrix.
	while(!file.eof()) {
		file>>numbmat;
		i++;
	}
	
	//Calculating number of columns.
	ncol=i/nrow;
	
	//Allocating matrix.
	mat=new int* [nrow];
	for(int j=0; j<nrow; j++) {
		mat[j]=new int [ncol];
	}
	
	//Allocating array of struct.
	position* sum= new position [nrow];
	
	LoadMatrix(ncol, nrow, mat);
	RowSum(nrow, ncol, sum, mat);
	RowsOrder(nrow, ncol, sum);
	PrintMatrix(nrow, ncol, sum, mat);
	
	system("pause");
	return 0;
}
/*
Function that load the matrix from the file.
*/
void LoadMatrix(int ncol, int nrow, int** mat) {
	file.seekg(0);
	for(int r=0; r<nrow; r++) {
		for(int c=0; c<ncol; c++) {
			file>>mat[r][c];
		}
	}
}
/*
Function that sums the number in each row.
*/
void RowSum(int nrow, int ncol, position* sum, int** mat) {
	for(int r=0; r<nrow; r++) {
		int a=0;
		for(int c=0; c<ncol; c++) {
			a=a+mat[r][c];
		}
		sum[r].summation=a;
		sum[r].pos=r;
	}
}
/*
Function that sort the rows.
*/
void RowsOrder(int nrow, int ncol, position* sum) {
	int temp1=0, temp2=0;
	int i=0;
	do{
		for(int j=0; j<nrow-1; j++) {
			if(sum[j].summation>sum[j+1].summation) {
				temp1=sum[j].summation;
				sum[j].summation=sum[j+1].summation;
				sum[j+1].summation=temp1;
				temp2=sum[j].pos;
				sum[j].pos=sum[j+1].pos;
				sum[j+1].pos=temp2;
			}
		}
		i++;
	}while(i<ncol);
}
/*
Function that print the matrix.
*/
void PrintMatrix(int nrow, int ncol, position* sum, int** mat) {
	for(int r=0; r<nrow; r++) {
		for(int c=0; c<ncol; c++) {
			cout<<mat[sum[r].pos][c]<<" ";
		}
		cout<<endl;
	}
}
jlb (4973)
Is it possible to improve this code?

Probably, yes. A couple of things stand out at a quick glance.

First why are you using a global variable for your fstream instance? You should make this function local to the function that first uses the file, in this case main() and pass the opened stream to the functions that need it.

Second where are your delete calls for all those new calls?

Third you need to be careful with your file handling. Until C++11 a seekg() doesn't reset the error state. If the stream is in an error state the seekg() call silently fails.

I did it with the help of my friend and it works fine but I'm not so sure about the use of the struct in this case.

What don't you understand about the use of the structure?
Kernul (74)
First why are you using a global variable for your fstream instance? You should make this function local to the function that first uses the file, in this case main() and pass the opened stream to the functions that need it.


So in my case it'll be okay just putting fstream file; into the main, since it's the only one who use it.

Second where are your delete calls for all those new calls?


I asked the same thing to my friend. He just said that when the program ends everything is deleted. So I have to put delete mat; and delete sum; at the end of the main, right?

Third you need to be careful with your file handling. Until C++11 a seekg() doesn't reset the error state. If the stream is in an error state the seekg() call silently fails.


So what do I have to do? I know what seekg() does but I didn't know about the error state.


What don't you understand about the use of the structure?


It's not that I don't understand the use of the structure, it's only because I wanted to find another way without using it. I was thinking about the vectors but then I don't know how to pass them into the matrix at the function PrintMatrix.
jlb (4973)
So in my case it'll be okay just putting fstream file; into the main, since it's the only one who use it.

Yes putting it into main() would be preferable, however you'll need to pass it into at least one function.

I asked the same thing to my friend. He just said that when the program ends everything is deleted.

While this may be true with today's modern operating systems you should get into the habit of always deleting the memory you allocate because there are occasions when the operating system is either not present or doesn't reclaim "lost" memory.

So I have to put delete mat; and delete sum; at the end of the main, right?

Don't forget to delete the both pointers of the mat variable. And remember you delete what you new and delete[] what you new[], make sure to use the correct delete.

So what do I have to do? I know what seekg() does but I didn't know about the error state.

If you're sure you're using a C++11 compiler you don't have to do anything. But if you're not sure you should clear() the error state before you call seekg(). When you read the file to the end of file you set the eof() error state so it need to be cleared.

It's not that I don't understand the use of the structure, it's only because I wanted to find another way without using it.

Why a structure or class is a good way to group variables that have things in common? By using a structure your function calls can have fewer parameters.

I was thinking about the vectors but then I don't know how to pass them into the matrix at the function PrintMatrix.

You would use a vector to replace your dynamic arrays.

I don't know how to pass them into the matrix at the function PrintMatrix.

You could pass them like any other POD variable, or better yet pass the vector into PrintMatrix() using a const reference. And since a vector knows it's size you won't need to pass the number of rows and columns into this function either.


1
2
3
4
5
6
7
8
9
10
 
void PrintMatrix( const vector<position> &sum, const vector<vector<int>> &mat)
{
        // Print out the matrix.
	for(size_t r = 0; r < mat.size(); r++) // Note the use of size_t and the added spacing.{
		for(size_t c = 0; c < mat[r].size(); c++) {
			cout << mat[r][c] << " ";
		}
		cout << endl;
	}
}



AbstractionAnon (6954)
kernul wrote:
I asked the same thing to my friend. He just said that when the program ends everything is deleted.

That may be true, but if you keep allocating memory without releasing what you're done with, you can exhaust available memory. That's why it is best practice to always delete what you new as soon as possible.
Kernul (74)
Yes putting it into main() would be preferable, however you'll need to pass it into at least one function.


How should I pass the file to a function? I mean, what should I write between the () ?

Don't forget to delete the both pointers of the mat variable. And remember you delete what you new and delete[] what you new[], make sure to use the correct delete.


Got it.

If you're sure you're using a C++11 compiler you don't have to do anything.


How do I check if I'm using a C++11 compiler?

You could pass them like any other POD variable, or better yet pass the vector into PrintMatrix() using a const reference. And since a vector knows it's size you won't need to pass the number of rows and columns into this function either.


Oh, you recreated the matrix with the vector library. I wasn't thinking about that. XD
And by the way, why in the second for there is mat[r].size() while in the first there isn't?
In the first you'll know the size of the entire matrix(so every number in it) while in the second you'll know the size of only one row(only the number in that specific row), right?
jlb (4973)
How should I pass the file to a function? I mean, what should I write between the () ?

You should pass it by reference. Do you know how to pass variables by reference?

How do I check if I'm using a C++11 compiler?

This depends upon what compiler and IDE you are actually using. If you're using g++ you use the -std=c++11 switch when compiling.

And by the way, why in the second for there is mat[r].size() while in the first there isn't?

Because the first is getting the size of the outer vector, the second is getting the size of one of the inner vectors.

In the first you'll know the size of the entire matrix(so every number in it)

No not really the size of the entire matrix, this is the number of rows in the matrix.
while in the second you'll know the size of only one row(only the number in that specific row),

Yes this is the column in the current row.

Basically a vector acts almost identically to an array.

Kernul (74)
You should pass it by reference. Do you know how to pass variables by reference?


Yeah I guess.
It should be something like:
void name_function (..., fstream& file, ...)
Am I right?

If you're using g++ you use the -std=c++11 switch when compiling.


Yes, I'm using g++. How and where do I use it?

Basically a vector acts almost identically to an array.


Okay, I got it.
jlb (4973)
Am I right?

Yes.

Yes, I'm using g++. How and where do I use it?

How are you compiling your program? Are you using an IDE? Command line?

If command line:

g++ -g -Wall -Wextra -Wpendatic -std=c++11 ... (the rest of your compile line).

If you use an IDE the IDE documentation should tell you where to add the proper values.
Kernul (74)
What does IDE mean?
Anyway, I'm using Dev-C++ 5.7.1 and there's an icon I click(or by simply pressin F9) that compile everything.
jlb (4973)
What does IDE mean?

Integrated Development Environment. In this case Dev-C++ .

I'm not familiar with this IDE but you should be able to find the documentation to find out how to change the compile options. Normally most IDEs using g++ will compile code as C++98 and with few warning messages emitted so you need to probably change the project settings to emit more warnings and compile as C++11 (if desired). If you can't find the documentation or can't figure out how to change the settings you may want to start a new thread concerning that issue. Be sure your post states the compiler version, IDE and it's version and the operating system version you're using.


Kernul (74)
Okay! Thank you so much! I'll mark this thread as solved.
Topic archived. No new replies allowed.