Changing loops to call generic functions

[L1bXjE8b]

I am working with this code:

#include <string>
#include <iostream>
#include <sstream>
#include "minimax.h"

#include <algorithm>
#include <iterator>

using namespace std;

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

  if (argc < 2)
    {
      cerr << "Must supply at least one integer on the command line."
	   << endl;
      return -1;
    }

  string str;
  
  MiniMax mnmx;

  int nData = argc - 1;
  int* data = new int[nData];

  for (int i = 0; i < nData; ++i)
    {
      str = argv[i+1];
      istringstream numberIn (str);
      numberIn >> data[i];
      mnmx.observe(data[i]);
    }

  int minPos = -1;
  for (int i = 0; i<nData && minPos==-1; ++i)
    {
      if (data[i] == mnmx.getMin())
	minPos = i;
    }
  int maxPos = -1;
  for (int i = 0; i<nData && maxPos==-1; ++i)
    {
      if (data[i] == mnmx.getMax())
	maxPos = i;
    }

  cout << data[0];
  for (int i = 1; i<nData; ++i)
    {
      cout << ' ' << data[i];
    }
  cout << endl;

  //This is fine beacuse it is not in a loop
  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;
}

Specifically, I am looking at the for loop to print the array of data (lines 50-55). I want to replace this for loop with a call to a std generic function. I am thinking that I could use something like the std copy function, with something like copy ( mmmx, str, stream_iterator<string> ( cout, "\n" ));. However, this syntax is not correct. What should I be using here? Should I even be using copy at all?

[JLBorges]

#include <iostream>
#include <string>
#include <algorithm>

int main( int argc, char* argv[] )
{
    if (argc < 2 )
    {
        std::cerr << "Must supply at least one integer on the command line.\n" ;
        return 1;
    }

    std::cout << "args: " ;
    std::for_each( argv+1, argv+argc, []( auto cstr ) { std::cout << cstr << ' ' ; } ) ;

    const auto cmp_int = [] ( std::string a, std::string b ) { return std::stoi(a) < std::stoi(b) ; };

    try
    {
        const auto minmax = std::minmax_element( argv+1, argv+argc, cmp_int ) ;
        std::cout << "\nmin: " << *minmax.first << "  max: " << *minmax.second << '\n' ;
    }
    catch( const std::exception& )
    {
        std::cerr << "there was at least one invalid command line arg\n" ;
    }
}

http://coliru.stacked-crooked.com/a/6d36a71c41ee2c90

std::copy: std::copy( argv+1, argv+argc, std::ostream_iterator<const char*>( std::cout, " " ) ) ;
http://coliru.stacked-crooked.com/a/cc515b72d045eaa9

[L1bXjE8b]

Thank you for your response. Since I am working with other files, I was not able to use your entire program, but I was able to adapt your copy to work. One more question...If I were to replace the for loops in 37-42 and 43-48, would you use for_each? I was thinking originally to use bool functions for minPos and maxPos, but that would cause loops elsewhere in the program. What would you suggest?

[JLBorges]

const int minPos = std::find( data, data+nData, mnmx.getMin() ) - data ;
const int maxPos = std::find( data, data+nData, mnmx.getMax() ) - data ;

[L1bXjE8b]

Thank you! Maybe a dumb question here, but how do you determine exactly what parameter goes where in these generic functions? For example, how did you know to use data, data+nData, mmmx.getMax() in that particular order?

[JLBorges]

template< class InputIt, class T >
InputIt find( InputIt first, InputIt last, const T& value );     // (1) 

Parameters
first, last - the range of elements to examine
value - value to compare the elements to

http://en.cppreference.com/w/cpp/algorithm/find

data - pointer to first element, begin of the sequence
data+nData - pointer to one past the last element, end of the sequence
this pair of iterators defines the range: [data, data+nData)

mmmx.getMax() - the value to search for

[L1bXjE8b]

To replace the for loop in lines 29-35, I was thinking of using

  copy ( argv[ i+1 ], argv[i+1]+nData, data );

However, that's not exactly right. It compiles, but it's buggy:

5 7 10 
The smallest value is 0 and can be found in position 1
The largest value is 0 and can be found in position 1

Should I be using copy(), or should I be doing something else?

[JLBorges]

The command line arguments in argv[] are null-terminated multibyte strings (C-style strings); data is a sequence of integers. Each command line argument has to be converted from a C-style string to an int.

std::transform http://en.cppreference.com/w/cpp/algorithm/transform

#include <iostream>
#include <string>
#include <algorithm>
#include <vector>

int main( int argc, char* argv[] )
{
    if (argc < 2 )
    {
        std::cerr << "Must supply at least one integer on the command line.\n" ;
        return 1;
    }

    const auto arg_to_int = [] ( std::string arg ) // non-numeric/badly formed args evaluated as zero
    { try { return std::stoi(arg) ; } catch( const std::exception& ) { return 0 ; } };

    std::vector<int> data(argc-1) ;
    std::transform( argv+1, argv+argc, data.begin(), arg_to_int ) ;

    std::cout << "args: " ;
    for( int v : data ) std::cout << v << ' ' ;
    std::cout << '\n' ;
}

http://coliru.stacked-crooked.com/a/e99fdad56d0887a9