Generic programming using std::transform()

[L1bXjE8b]

I am working on a program that takes an arbitrary number of command line arguments (at least 1), and outputs the minimum and maximum value, as well as what position in the array they were--e.g., if 1 2 3 is input, the resulting output will be

1 2 3
Largest: 3 at position 2.
Smallest: 1 at position 0.

In order to take the command line arguments and compare compare them with the int data array, I need to transform the command line arguments into integers. I am trying to use this:

transform ( argv+1, argv+argc, data.begin(), atoi );

However, I get this compilation error:

error: member reference base type 'int *' is not a structure or union
  transform ( argv+1, argv+argc, data.begin(), atoi );
                                 ~~~~^~~~~~

What am I doing wrong here? Here is the area I'm working with:

int nData = argc - 1;
string str = argv[i+1];
int* data = new int[nData];

transform ( argv+1, argv+argc, data.begin(), atoi );
istringstream numberIn (str);
numberIn >> data[i];
mnmx.observe(data[i]);

[AbstractionAnon]

Line 3: data is a simple int array. int arrays have no begin() function. You may want to use the std::array container which does have a begin() member function.

[Cubbi]

What am I doing wrong here?

using the keyword "new"

You can use data.begin() if you construct "data" as std::vector<int> data(nData);

(or you could default-construct it as std::vector<int> data; and use std::back_inserter(data) as the target of transform)

[L1bXjE8b]

AbstractionAnon, I used this:

array<int, nData> data;

but that still doesn't work. This is the error message:

error: non-type template argument is not a constant expression
  array<int, nData> data;
             ^~~~~
note: read of non-const variable 'nData' is not allowed in a constant
      expression
note: declared here
  int nData = argc - 1;
      ^
1 error generated.

Am I using std::array incorrectly here?

[AbstractionAnon]

The problem is that nData is not const.

You're going to have to pick an arbitrary max size for nData.

    const int nData = 100; 

[L1bXjE8b]

I can't use a const because I have a find() later on in the program that const breaks. I was able to use a vector of size nData, but I am getting this error, relating to not using a

const[/code:
[output]
error: variable length array of non-POD element type 'vector<int>'
  vector<int> data [nData];
                   ^
1 error generated.
[/output]

I did make [code]nData

a const, but I am still getting this error.

[Chervil]

vector<int> data [nData];
should be
vector<int> data (nData);

nData need not be constant.

See fill constructor,
http://www.cplusplus.com/reference/vector/vector/vector/

[L1bXjE8b]

But neither of those works later on in my find() when I use data+nData.

[Chervil]

If you use a vector, then you may use data.begin() and data.end() instead of data and data+nData

[L1bXjE8b]

Because the result of the find function is being copied into an int, I am getting an error:

error: no viable conversion from 'std::__1::__wrap_iter<int *>' to 'int'

[Chervil]

std::find() returns an iterator.
You can convert that to an integer offset by subtracting data.begin()

[micromech27]

Show your code so we can see what you are talking about. Its hard to help when you can't see what you are doing.

[L1bXjE8b]

Ok, so I am using a for_each to apply operations to every command line argument given. Here is my code as it stands now (with errors):

#include <string>
#include <iostream>
#include <sstream>
#include <vector>
#include <cassert>
#include <algorithm>
#include <iterator>

#include "minimax.h"

using namespace std;

int main (int argc, char **argv)
{
  using namespace std;

  if (argc < 2)
    {
      cerr << "This program needs at least one command line argument."
	   << endl;
      return -1;
    }
  
  MiniMax mnmx;
  int i = 0;

  int nData = argc - 1;
  vector<int> data (nData);

  string str = argv[i+1];

  transform ( argv+1, argv+argc, data.begin(), atoi );

  istringstream numberIn ( str );
  numberIn >> data[i];
  observe ( data[i]));

  std::for_each ( data.begin(), data.end() );//Problem here

  int minPos = std::find ( data.begin(), data.end(), mnmx.getMin() ) -data.begin();
  int maxPos = std::find ( data.begin(), data.end(), mnmx.getMax() ) - data.begin();

  copy ( argv+1, argv+argc, ostream_iterator<const char*> ( cout, " "));
  cout << endl;
  cout << "The smallest value is " << mnmx.getMin()
       << " and can be found in position " << minPos
       << endl;

  cout << "The largest value is " << mnmx.getMax()
       << " and can be found in position " << maxPos
       << endl;

  return 0;
}

Here is what I am trying to do. I know my transform is right, but now I need to make the for_each perform these operations for each command line argument in the data vector:

istringstream numberIn ( str );
numberIn >> data[i];
observe ( data[i]));

Is the best way to do this is write a void function with the command line arguments passed in as parameters and then call that function from the for_each portion? Any suggestions would be appreciated.

[Chervil]

Here is what I am trying to do. I know my transform is right, but now I need to make the for_each perform these operations for each command line argument in the data vector:

istringstream numberIn ( str );
numberIn >> data[i];
observe ( data[i]));

You already stored the values in the data vector. Now you overwrite the first element with the same value derived via a stringstream. What is the purpose of doing that?
what does function observe() do?

As for the rest of the program, did you consider std::min_element / std::max_element?
http://www.cplusplus.com/reference/algorithm/min_element/

[L1bXjE8b]

Would writing a functor work better? That way, I could use it directly within the for_each(), but it would not require writing a function that is used only once?

[Chervil]

I'm still not sure what you're trying to do exactly. Clearly the problem can be solved easily using std::min_element / std::max_element but you wish to do it some other way. Is this a self-education project or one you must do for some homework. In other words, are the constraints imposed by yourself, or externally by the instructions you were given?

[L1bXjE8b]

How could this be solved using std::max_element? For each of the input parameters, I need to read them in and then later determine the minPos and maxPos. The limitations are not mine. Instead, could I do the calls to minPos = find () from within a for_each and use it as a lambda expression?

[L1bXjE8b]

Here is what I have come up with. I have thought of moving the minPos and maxPos positions to be included in each lambda expression for each of the for_each's. Here is what I have right now:

for_each ( data.begin(), data.end(), [] ( int minPos ) {
    //mnmx.observe ( data[i] );
    //istringstream numberIn ( str );
    //numberIn >> data[i];
    MiniMax mnmx;
    minPos = std::find ( data.begin(), data.end(), mnmx.getMin() ) - data.begin();
    cout << "Smallest " << mnmx.getMin()
       << " in position " << &minPos
       << endl;
  } );

  for_each ( data.begin(), data.end(), [] ( int maxPos ) {
    MiniMax mnmx;
    maxPos = std::find ( data.begin(), data.end(), mnmx.getMax() ) - data.begin();
    cout << "Largest " << mnmx.getMax()
       << " in position " << &maxPos
       << endl;

But I get errors for

error: variable 'data' cannot be implicitly captured in a lambda with no
      capture-default specified

[Cubbi]

This is getting out of hand..

CSundergrad wrote:

if 1 2 3 is input, the resulting output will be

1 2 3
Largest: 3 at position 2.
Smallest: 1 at position 0.

int main(int ac, char** av)
{
    std::vector<int> data;
    std::transform(av+1, av+ac, std::back_inserter(data), [](char* p){ return std::stoi(p); });
    for(int n: data) std::cout << n << ' '; std::cout << '\n';
    auto mm = std::minmax_element(data.begin(), data.end());
    std::cout << "Largest: " << *mm.second << " at position " << std::distance(data.begin(), mm.second) << '\n'
              << "Smallest: " << *mm.first << " at position " << std::distance(data.begin(), mm.first) << '\n';
}

full demo with error checking: http://coliru.stacked-crooked.com/a/f09cbf8f4fa461c6

[L1bXjE8b]

The idea is to remove for loops. How could I get around the one you have in line 5?