# User Specified Array Read and Modify

How may this code be improved upon, the user inputs array of specified length and replaces each element that is smaller than the mean of the first and last element with the mean.

Is there a better way to pass the array to the function (ref/ptr, etc.), or more efficient way to manage memory (when handling array)?

I know the vector container from STL is ideal but just wanted to gain some insight on fundamentals.

#include <iostream>
using namespace std;

void replaverage(int arr[], size_t n){
cout<<"Enter array length"<<endl;
cin>> n;
for(int i=0; i<n; i++){
cout<<"Enter the numbers"<<endl;
cin>>arr[i];
cout<<endl;
}
int f=arr[0];
int l=arr[n-1];
double av=(f+l)/2;
for(int j=0; j<n; j++){
if(arr[j]<av){
arr[j]=av;
}
}
for (int i=0; i<n; i++){
cout<<arr[i]<< " ";}
}

int main(int argc, char* argv[]){
size_t x=10;
int arr[x];
replaverage(arr,x);
return 0;
}

• In c++ you should use a vector<int> instead of a raw c-style array. Feb 17 '20 at 0:11
• Welcome to Code Review! I have rolled back your last edit. Please do not update the code in your question to incorporate feedback from answers, doing so goes against the Question + Answer style of Code Review. This is not a forum where you should keep the most updated version in your question. Please see what you may and may not do after receiving answers. Feb 17 '20 at 5:06

Something to keep in mind is that this statement is truncating data:

        arr[j]=av;


because it is assigning a double to an int.

As was mentioned in a comment there are c++ container types that would be better than a old C style array, two of these are std::array<type, arraySize> and std::vector<type>. std::array is a fixed size and std::vector is a variable size. Both would allow the use of iterators that might simplify the code.

## Avoid using namespace std;

If you are coding professionally you probably should get out of the habit of using the using namespace std; statement. The code will more clearly define where cout and other identifiers are coming from (std::cin, std::cout). As you start using namespaces in your code it is better to identify where each function comes from because there may be function name collisions from different namespaces. The identifiercout you may override within your own classes, and you may override the operator << in your own classes as well. This stack overflow question discusses this in more detail.

## std::endl

It is better for performance if "\n" is used rather than std::endl in output, std::endl flushes the output buffer as well as putting out a new line, and this can slow down loops.

## Complexity

The function replaverage should really be 2 functions, one to input the data and a second to process the data. Perhaps a third to print the data.

## Horizontal Spacing

It would be better if there were spaces around operators that separate operands.

#include <iostream>
#include <vector>

std::vector<int> getData()
{
int n;
std::vector<int> intputData;
std::cout << "Enter array length\n";
std::cin >> n;
std::cout << "Enter the numbers\n";
for(int i = 0; i < n; i++){
int tmpIn;
std::cin >> tmpIn;
intputData.push_back(tmpIn);
}

return intputData;
}

void modifyData(std::vector<int> &data)
{
int f=data[0];
int l=data[data.size() - 1];
double av = (f + l)/2;
for (auto j: data)
{
if(j < av){
j = av;
}
}
}

void printData(std::vector<int> &data)
{
for (auto i: data)
{
std::cout << i << " ";
}
}

int main(int argc, char* argv[]){

std::vector<int> data = getData();
modifyData(data);
printData(data);

return 0;
}

• The two places where std::endl are used both immediately precede reading from std::cin, where a flush is clearly desirable. That said, I would prefer to see it written explicitly (...\n" << std::flush) to ensure readers know it's intended and not just a newline. Feb 17 '20 at 11:03

Always check input succeeded.

std::cin >> n;

    std::cin >> arr[i];


In both these lines, we ignore all errors, and will produce the wrong output without any warnings, and happily return 0 from main(). This is bad for any program used as a processing step (e.g. driven by Make).

• Check success of input via try-catch or throws? Feb 19 '20 at 9:14
• Yes, you can check using exceptions, if you set the stream to throw on error (using std::cin::exceptions(std::ifstream::eofbit | std::ifstream::badbit), for example). Or check explicitly with if (!std::cin) { handle errors; }. Feb 19 '20 at 9:19
• cin.fail() seems to be a good solution. Feb 19 '20 at 9:39

I restructured the code into object oriented format. Incorporated changes suggested by pacmaninbw.

#include <iostream>
#include <vector>

class replace_avg{

private:
int n;
int tmpIn;
double f;
double l;
double av;
std::vector<double> intputData;

public:
std::vector<double> getData()
{
std::cout << "Enter array length\n";
std::cin >> n;
std::cout << "Enter the numbers\n";
for(int i = 0; i < n; i++){
std::cin >> tmpIn;
intputData.push_back(tmpIn);
}
return intputData;
}

void modifyData(std::vector<double> &inputData)
{
f = inputData[0];
l = inputData[inputData.size() - 1];
av = (f + l)/2;
for (auto &j: inputData)
{
if(j < av){
j = av;
}
}
}

void printData(const std::vector<double> &inputData)
{
for (auto &i: inputData)
{
std::cout << i << " ";
}
std::cout << '\n';
}
};

int main(){

replace_avg vect;
std::vector<double> v1 = vect.getData();
vect.modifyData(v1);
vect.printData(v1);

return 0;
}


Consider absl::Span<T>. (or boost's) It may one day be replaced with std::span

It's actually quite hard to make what you're asking for, despite this being extremely useful and desired for a very long time.

However, with it, you can write:

void ReadFromIntArray(absl::Span<int const> int_array);

There's tremendous downsides to using iterators or pointer+count. Pointer+count is wildly dangerous and severely impairs readability. Iterators impair readability further and may also obliterate an architecture; either the interface becomes tightly coupled with std::vector, or the interface requires a template parameter adding a huge unnecessary compilation cost to every client.