-2

I'm trying to convert some research-related code written in C by somebody I don't know and absolutely no documentation into C++. Basically converting T* and T** objects into vectors and more user-friendly containers.

Atm I have two variables,

double* data
double* references

I saw a trick here on SO that makes a regular vector easily assignable to the double*, like so:

references = &(*m_DataMaxPoints)[0];

so that's okay.

However, data* is supposed to be set from a std::vector<std::vector<double>>* m_Data;

I don't know how I should assign this. I tried &(*m_Data)[0][0];, but I really don't know what I should do here. Any tips would be welcome.. :(

Edit 1: Original handling of data* in the C-program

double* data is declared in main()

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

    int verbose_flag = FALSE;


    double *data = NULL, *reference = NULL, *maximum = NULL;
    int *cumsizes = NULL;
    int nobj = 0, nruns = 0;
    int k;
    double volume = 0;
    double time_elapsed;

read_input(stdin, "stdin", &data, &nobj, &cumsizes, &nruns);

read_input parses numbers from a text file into the arrays.

#define PAGE_SIZE 4096          // allocate one page at a time    
#define DATA_INC (PAGE_SIZE/sizeof(double))
#define SET_INC 128     // assume 128 datasets (can grow)  

int read_input(FILE *infile, const char *filename, double **datap, int *ncolsp, int **cumsizesp, int *nrunsp)
{
    char b[2];
    double number;

    int newline;                // last input was newline
    int datai;                  // the current element of (*datap)
    int col;                    // the column being read
    int set = *nrunsp;          // the current data set
    int ncols = *ncolsp;        // the number of columns

    int datasize;
    int setsize;

    datai = (set == 0) 
        ? 0 
        : ncols * (*cumsizesp)[set-1];

    setsize = (set == 0) 
        ? 0 
        : ((set-1) / SET_INC + 1) * SET_INC;

    *cumsizesp = realloc(*cumsizesp, setsize * sizeof(int));

    datasize = (datai == 0) 
        ? 0 
        : ((datai - 1) / DATA_INC + 1) * DATA_INC;

    *datap = realloc(*datap, datasize * sizeof(double));

    // remove leading whitespace
    fscanf (infile, "%*[ \t\n]");

    if (feof(infile)) {
        warnprintf("%s: file is empty.", filename);
        return -1;
    }

    do {
        newline = 0;
        if (set == setsize) {
            setsize += SET_INC;
            *cumsizesp = realloc(*cumsizesp,  setsize * sizeof(int));
        }

        (*cumsizesp)[set] = (set == 0) ? 0 : (*cumsizesp)[set-1];       // beginning of data set

        while (newline == 0) {
        col = 0;        // beginning of row
            while (newline == 0) {
        if (fscanf(infile, "%lf", &number) != 1) {
                    char buffer[64];

                    fscanf(infile, "%60s", buffer);
                    errprintf(
                        "could not convert string `%s' to double, exiting...", 
                        buffer);
                }

                if (datai == datasize) {
                    datasize += DATA_INC;
                    *datap = realloc(*datap,  datasize * sizeof(double));
                }
                (*datap)[datai++] = number;

#if DEBUG > 2
                fprintf(stderr, "set %d, row %d, column %d, x = %g\n", 
                        set, (*cumsizesp)[set], col, number);
#endif
                col++;    // new column

                fscanf(infile, "%*[ \t]");
                newline = fscanf(infile, "%1[\n]", b); 
            }

            if (!ncols) 
                ncols = col;
            else if (col != ncols) {
                if ((*cumsizesp)[0] == 0) 
                    errprintf ("reference point has dimension %d"
                               " while input has dimension %d",
                               ncols, col);
                else
                    errprintf("row %d has different length (%d)"
                              " than previous rows (%d), exiting...",
                              (*cumsizesp)[set], col, ncols);
            }

            (*cumsizesp)[set]++;

            fscanf (infile, "%*[ \t]"); 
        newline = fscanf (infile, "%1[\n]", b); 
    }
#if DEBUG > 1
        fprintf (stderr, "Set %d, %d rows in total\n", set, (*cumsizesp)[set]);
#endif
    set++; // new data set

        fscanf (infile, "%*[ \t\n]");

    } while (!feof(infile)); 

    *ncolsp = ncols;
    *nrunsp = set;

    return 0;
}

What I am trying to do is convert as little as possible to C++ to make it easier to use and implement it into a larger project by making a wrapper class for this interface.

So, the file parsing and data allocation is done with vectors (they don't need to be pointers).

After parsing, I'd prefer to convert the vectors into C-style arrays so that I can call the original code's function without having to add vector to support to all of that, because I don't understand how to read it in pure C (multi-objective evolutionary algorithm implementation).

The original code finally calls:

volume =  hv(data, nobj, cumsizes[nruns-1], reference);

While I'd like to do this:

double* data = &(*m_Data)[0][0]; // my 2d vector<double>
double* references = &(*m_DataMaxPoints)[0]; // 1d vector<double>
int cumsizes = m_Data->at(0).size(); 
m_Volume = hv(data, Objectives(), cumsizes, references);
  • 4
    Can you show a more complete example of the code you're converting? It's too ambiguous, based on the limited code provided here, exactly what the intent is. – Xirema Aug 13 '18 at 17:40
  • 1
    Is `data` supposed to be a contiguous 2D array? For example would data have been used as `data = (double*)malloc(width * height * sizeof(double));`? If that's the case, you can't use `vector>` for your conversion since that doesn't store all the data contiguously. You could instead use a single `vector` of size `width*height` and implement your own x,y accessing logic. – alter_igel Aug 13 '18 at 17:42
  • 2
    If you are tryinig to convert the code to C++, why not just stick with `std::vector` and `std::vector::iterator` instead of trying to get pointers to the underlying data? – R Sahu Aug 13 '18 at 17:42
  • 1
    Also, I would recommend not using a pointer to `std::vector`, it would be better if you make that member a value type or use a smart pointer like `std::unique_ptr` if you really need to. – alter_igel Aug 13 '18 at 17:44
  • 1
    What makes you think you need a pointer to a vector? – eesiraed Aug 13 '18 at 17:44
  • 1
    Note, too, that you need to be very careful about context. For example, in C, one might pass a `double **` to a function that expects to access an existing array of pointers to `double`, but one could also pass a value of the same type to a function that was expected to dynamically allocate an array of `double`, and update a variable designated by the caller with a pointer to the allocated space. How you would want to convert those two cases to C++ is totally different. – John Bollinger Aug 13 '18 at 17:49
  • You may be better off [with a simple matrix class](https://stackoverflow.com/a/43552983/4581301) that operates on a 1 dimensional array and makes it look to users like a 2 (or N) dimensional array. Often significant advantages in performance and ease of management. – user4581301 Aug 13 '18 at 18:13
  • @Xirema I've updated to a more complete sample. I hope it helps – Mads Midtlyng Aug 13 '18 at 18:16
  • @alterigel Please see the updated example on how data is used in the original code. – Mads Midtlyng Aug 13 '18 at 18:16
  • @FeiXiang Pointer or no pointer, does it affect performance? – Mads Midtlyng Aug 13 '18 at 18:17
  • Probably not, but if there isn't a reason to use pointers, don't use them to avoid confusion like what you're having now. Also, learn about [premature optimization](http://wiki.c2.com/?PrematureOptimization), which seems to be what you're doing now. – eesiraed Aug 13 '18 at 18:20
  • @FeiXiang Thanks for the tip. I've been using pointers out of habit because the usual program runs on all cores performing heavy calculations, so I'd rather use pointers than copy an object. But in this very case, as you say, I agree it's not necessary. – Mads Midtlyng Aug 13 '18 at 18:24
  • Everything you dynamically allocate has a cost. Whatever's managing the dynamic memory has to search for a suitable block of free storage (this may or may not be cheap) and it is highly recommended that you return the memory when you are done with it. If you allocate the `vector` in Automatic storage (commonly the stack), usually the only additional cost is a moved stack pointer. – user4581301 Aug 13 '18 at 18:24
  • Sidenote: `vector` is designed to handle dynamic memory management for you. Dynamically allocating a `vector` eliminates a not-insignificant portion of its benefits. Recommended reading: [What is meant by Resource Acquisition is Initialization (RAII)?](https://stackoverflow.com/questions/2321511/what-is-meant-by-resource-acquisition-is-initialization-raii) Note that you can pass around pointers (and [references](https://en.cppreference.com/book/intro/reference)) without having to dynamically allocate. – user4581301 Aug 13 '18 at 18:27
  • Another sidenote: `while (!feof(infile));` is a common bug source: [Why is “while ( !feof (file) )” always wrong?](https://stackoverflow.com/questions/5431941/why-is-while-feof-file-always-wrong). The C++ version of this is [Why is iostream::eof inside a loop condition considered wrong?](https://stackoverflow.com/questions/5605125/why-is-iostreameof-inside-a-loop-condition-considered-wrong). TL;DR version: This results in testing for failure AFTER using data that failed to be read. – user4581301 Aug 13 '18 at 18:31
  • @user4581301 Thanks for all the tips. Regarding while (!feof(infile)); this is used in the original code's file parsing. The C++based parser I am using does not rely on feof(). The original parser is just there for others to help me understand – Mads Midtlyng Aug 13 '18 at 18:36
  • Understood. Just keep an eye out for that bug and its friends. Always make sure input is good before you try to use it. It also makes a good marker. When you see a mistake like that in code, you should slow down and make sure the writer of the code hasn't made other sloppy mistakes before you start porting their algorithm. It're really easy to make C and C++ code that looks like it works, and porting code is a great way to expose unseen errors. That said, this one is doing pretty well up to a few unchecked scanfs at the end of the loop. – user4581301 Aug 13 '18 at 18:47

2 Answers2

2

Using pointers on vectors is not necessary the way to go, as mentionned in the comment of your question iterators however would be a good choice. Anyway, if you really want to use pointers here is how it goes :

vector< vector<double> >* vec = new vector< vector<double> >(16);

for (size_t i = 0; i < 16; ++i) {
    (*vec)[i].resize(16);
}

(*vec)[4][2] = 6.66;

This code creates a pointer ( vec ) to a vector of vectors of doubles (of dimensions 16x16) uninitialized with the value 6.66 at the position [4][2] and I think you get the synthax now.

tom gautot
  • 378
  • 2
  • 10
0

std::vector is meant to handle all memory allocations. It doesn't need malloc, realloc, or new. You can't implement this effectively without rewriting much of your code.

If you want your C code to compile in C++, it needs minor changes such as casting:

*datap = (double*)realloc(*datap, datasize * sizeof(double));

Moreover, you can pass C++ objects by reference, therefore you don't need a pointer. Pass the vector by reference.

Easiest way it to keep the existing C code, then copy the data from datap to vec. Example:

void foo(std::vector<std::vector<double>> &vec)
{
    //copy from datap to vec:

    //add a new row:
    vec.push_back(std::vector<double>{});

    //add coloumns
    vec.back().push_back(11.0f);
    vec.back().push_back(12.0f);
    vec.back().push_back(13.0f);

    //add another row:
    vec.push_back(std::vector<double>{});

    //add coloumns
    vec.back().push_back(21.0f);
    vec.back().push_back(22.0f);
    vec.back().push_back(23.0f);
}

int main()
{
    std::vector<std::vector<double>> vec;
    foo(vec);
    for(size_t row = 0; row < vec.size(); row++)
    {
        for(size_t col = 0; col < vec[row].size(); col++)
            printf("%3.0f, ", vec[row][col]);
        printf("\n");
    }
    return 0;
}

Or you might as well rewrite the whole code in C++. Example:

#include <iostream>
#include <string>
#include <fstream>
#include <vector>
#include <sstream>

void read_matrix(std::vector<std::vector<double>> &vec)
{
    std::ifstream fin("file.txt");
    if(!fin)
        return;

    std::string line;
    while(std::getline(fin, line))
    {
        //add a new row:
        vec.push_back(std::vector<double>{});

        //add columns
        std::stringstream ss(line);
        double value;
        while(ss >> value)
            vec.back().push_back(value);
    }
}

int main()
{
    std::vector<std::vector<double>> vec;
    read_matrix(vec);
    ...
    return 0;
}
Barmak Shemirani
  • 30,904
  • 6
  • 40
  • 77