55502f40dc8b7c769880b10874abc9d0

for (i=0; i<n; i++)
for (j=0; j<m; j++)

{
sum=sum+A[i][j];
}


for (i=0; i<n; i++)
for (j=0; j<n; j++)

{
x=A[i][j];

if (x % 2 != 0)

{
sr=sr+x; br++;
}
}

if (br != 0)

{
sr=sr/br;
}

cout<<"Elementa s nai-golqma stoinost e: A["<<indx<<"]["<<indy<<"] - "<<max<<endl;
cout<<"Sumata na elementite e: "<<sum<<endl;
cout<<"Srednoaritmetichnata stoinost na nechetnite elementi e: "<<sr<<endl;

#include <iostream.h>

#define arrMax 100 


void begin(int &n, int &m);
void arrScan(int arr[arrMax], int n);
void arrPrint(int arr[arrMax], int n);
void arrAlign(int arr[arrMax], int n);
void arrJoin(int arrA[arrMax], int arrB[arrMax], int arrC[arrMax], int n, int m);
 


void main()

{
	int a[arrMax], b[arrMax], c[arrMax], n, m;




	begin(n, m);
	
	
	cout<<endl;
	cout<<"Set A:"<<endl;
	arrScan(a, n);
	
	cout<<endl;
	cout<<"Set B:"<<endl;
	arrScan(b, m);



	arrAlign(a, n);
	arrAlign(b, m);
	
	
	cout<<"=========AFTER========="<<endl;

	cout<<endl;
	cout<<"Print A:"<<endl;
	arrPrint(a, n);
	cout<<endl;


	cout<<endl;
	cout<<"Print B:"<<endl;
	arrPrint(b, m);
	cout<<endl;

	arrJoin(a, b, c, n, m);

	cout<<endl;
	cout<<"Print C:"<<endl;
	arrPrint(c, n+m);
	cout<<endl;
}


void begin(int &n, int &m)
{
	do
	{
		cout<<"N: ";
		cin>>n;
	}
	while(n<1 || n>100);

	do
	{
		cout<<"M: ";
		cin>>m;
	}
	while(n<1 || n>100);
}

void arrScan(int arr[arrMax], int n)
{
	int i;

	for(i=0; i<n; i++)
	{
		cout<<"["<<i+1<<"]: ";
		cin>>arr[i];
	}
}

void arrPrint(int arr[arrMax], int n)
{
	int i;

	for(i=0; i<n; i++)
	{
		cout<<"["<<i+1<<"]: "<<arr[i]<<endl;
	}
}

void arrAlign(int arr[arrMax], int n)
{
	int i, j, temp;
	
	for(i=0; i<n-1; i++)
	{
		for(j=i+1; j<n; j++)
		{
			if(arr[i] > arr[j])	
			{
				temp = arr[i];
				arr[i] = arr[j];
				arr[j] = temp;	
			}
		}		
	}
}

void arrJoin(int arrA[arrMax], int arrB[arrMax], int arrC[arrMax], int n, int m)
{
	int i, j, temp, z=0;
	
	for(i=0; i<n; i++)
	{
		arrC[i] = arrA[i];
	}
	
	for(j=n, i=0; j<n+m; j++)
	{
		arrC[j] = arrB[i]; i++;
	}




	for(i=0; i<n+m-1; i++)
	{
		for(j=i+1; j<n+m; j++)
		{
			if(arrC[i] > arrC[j])	
			{
				temp = arrC[i];
				arrC[i] = arrC[j];
				arrC[j] = temp;	
			}
		}		
	}



}

Refactorings

No refactoring yet !

D41d8cd98f00b204e9800998ecf8427e

Yaverot

October 29, 2010, October 29, 2010 02:39, permalink

No rating. Login to rate!

This looks very C like for C++. I've changed the include to the C++ version, and the #define macro to a language construct.
I added the missing using statements so that it compiles.
I changed "void main" to "int main" so that it compiles.
I removed the begin() method, and initialized n&m using RAII, allowing them now to be const.
I removed excessive enters, so that even main() fits on my screen & added spaces around some of the operators to improve legibility.
I used std::swap instead of your manual version; it shows intent.
Since arrJoin() ended by sorting the new bigger array, I just had it call arrAlign(), after collecting the values from both arrays, instead of having another copy of bubble-sort in the code.
I stuck in consts where I knew they were safe. No warnings or errors under: g++ -W -Wall -Wextra

This seams like homework, which is why I didn't pull out all of the duplication. I'd generally recommend using better variable and function names, they don't all have to be single letters. I'd really avoid using the same name within a function as it is called outside of it, that helps me with scoping rules and avoiding masking of variables. Arrays may be what you are required to use, but I would look into vectors or another STL container. Then I'd use BOOST_FOREACH on them for reporting.

#include <iostream>

const int arrMax = 100;

using std::cout;
using std::endl;
using std::cin;
using std::swap;

int grab_value_inbounds(std::string message);
void arrScan(int arr[arrMax], const int n);
void arrPrint(int arr[arrMax], const int n);
void arrAlign(int arr[arrMax], const int n);
void arrJoin(int arrA[arrMax], int arrB[arrMax], int arrC[arrMax], const int n, int m);

int main()
{
	int a[arrMax], b[arrMax], c[arrMax];


	const int n = grab_value_inbounds("N: ");
	const int m = grab_value_inbounds("M: ");

	cout<<endl;
	cout<<"Set A:"<<endl;
	arrScan(a, n);

	cout<<endl;
	cout<<"Set B:"<<endl;
	arrScan(b, m);


	arrAlign(a, n);
	arrAlign(b, m);


	cout<<"=========AFTER========="<<endl;

	cout<<endl;
	cout<<"Print A:"<<endl;
	arrPrint(a, n);
	cout<<endl;


	cout<<endl;
	cout<<"Print B:"<<endl;
	arrPrint(b, m);
	cout<<endl;

	arrJoin(a, b, c, n, m);

	cout<<endl;
	cout<<"Print C:"<<endl;
	arrPrint(c, n+m);
	cout<<endl;
}

int grab_value_inbounds(std::string message)
{
	int n;
	do
	{
		cout << message;
		cin >> n;
	}
	while (n<1 || n>100);
	return n;
}


void arrScan(int arr[arrMax], const int n)
{
	int i;

	for (i=0; i<n; i++)
	{
		cout << "[" << i+1 << "]: ";
		cin >> arr[i];
	}
}

void arrPrint(int arr[arrMax], const int n)
{
	int i;

	for (i=0; i<n; i++)
	{
		cout << "[" << i+1 << "]: " << arr[i] << endl;
	}
}

void arrAlign(int arr[arrMax], const int n) // bubblesort
{
	int i, j;

	for (i=0; i<n-1; i++)
	{
		for (j=i+1; j<n; j++)
		{
			if (arr[i] > arr[j])
			{
				swap (arr[i], arr[j]);
			}
		}
	}
}

void arrJoin(int arrA[arrMax], int arrB[arrMax], int arrC[arrMax], const int n, const int m)
{
	int i, j;
	//unused variable ‘z’ removed

	// consider using a merge sort instead, you already know both arrays are sorted.
	for (i=0; i<n; i++)
	{
		arrC[i] = arrA[i];
	}

	for (j=n, i=0; j<n+m; j++)
	{
		arrC[j] = arrB[i];
		i++;
	}

	arrAlign(arrC, n+m);  
}
F5e5dd7322f363c947bbbc0463a16c79

Razvan

November 8, 2010, November 08, 2010 14:07, permalink

No rating. Login to rate!

Why did you hard-coded arrary sixe in grab_value_inbounds()?

int grab_value_inbounds(std::string message)
{
	int n;
	do
	{
		cout << message;
		cin >> n;
	}
	while (n<1 || n>arrMax );
	return n;
}
Ac50375cbea7c3c2588744659e2fff91

romkenny

April 6, 2011, April 06, 2011 18:42, permalink

No rating. Login to rate!

first of all, if you are going to use stl go all the way
instead of int a[arrMax] use a vector<int> created with the proper size
use printContainer instead of looping
use std::sort(vect.begin(), vect.end()) instead of hand crafted sorts
other than educational purposes i don't see the purpose of having two containers when you can just append the second data to the first one

std::copy(vect.begin(), vect.end(), std::ostream_iterator<int>(cout, " "));

or, more fancy

template <class T> void printContainer(T const& container, ostream& toHere = cout)
{
	typename T::const_iterator pos(container.begin());
	typename T::const_iterator end(container.end());

	for (; pos!=end; ++pos)
	{ 
		toHere << *pos << ' '; 
	} 
	toHere << std::endl; 
}

Your refactoring





Format Copy from initial code

or Cancel