-1

I got problem with va_list. I used it in constructor to get unexpected amount of data to my object. The problematic code is there:

#include <cstdarg>
#include <iostream>
using namespace std;

class DATA{
    private:
        int length;
        int* tab;
    public:
        DATA():tab(NULL), length(0){};
        DATA(int x, ...):length(x+1){
            va_list ap;
            va_start(ap, x);
            tab=new int[length];
            for(int i=0; i<length; ++i){
                tab[i]=va_arg(ap, int);
            }
            va_end(ap);
        }
        void showthem()const{
            if(tab!=NULL){
                int x;
                for(x=0; x<length-1; ++x){
                    cout<<tab[x]<<", ";
                }
                cout<<tab[x];
            }
        }
}
ostream & operator<<(ostream & out, const DATA & x){
    x.showthem();
    return out;
}
int main(){
    int table [] = {5,1,2,3,4,5,6};
    DATA x1(*table);
    DATA x2(4,1,2,3,4,5);
    cout << x1 << endl;
    cout << x2 << endl;
}

When I make object naturally by writing all parameters it's okay, but when i try to make it by a table it making problem. I got unexpected data in class tab.

I guess I am making something wrong. In forst way - can I even make object like this via *table? I am giving to the constructor some amount of integers so it should work...

masterq007
  • 37
  • 4
  • 1
    I'm voting to close this question as off-topic because this question does not make sense. – SergeyA Jun 03 '16 at 18:30
  • 4
    First of all, variadic functions are not to be used in C++. Second of all, self-made dynamic arrays are not to be used in C++. Third of all, if you are hell-bent on using variadic functions, you need to learn how to do this. Right now your code makes no sense. – SergeyA Jun 03 '16 at 18:31
  • `DATA x1(*table)` would be `DATA x1(5)`... – Jarod42 Jun 03 '16 at 18:32
  • The table is being passed as a pointer, The constructor is only getting one argument of type int* – Samuel Jun 03 '16 at 18:33
  • `std::vector` is better here. – Jarod42 Jun 03 '16 at 18:33
  • std::initializer_list might be what he wants. But after seeing operator<< implementation, I would recommend to stick with std::vector – KIIV Jun 03 '16 at 18:39

1 Answers1

1

KIIV nailed it. std::initializer_list (documentation linked here) is exactly what OP needs.

Forget about variable arguments lists. They allow hard-to-debug programs several different ways. For example,

DATA x2(41,2,3,4,5)

Whoops! Missed a comma and gonna read waaaaaay out of range on the stack! Or

DATA x2(5, 1,2,3,4,"I am the very model of a modern major-general...")

Which would be a lot easier to spot, but still compiles because the compiler doesn't know or care what types the varadic function allows.

But an initializer_list<int> will only allow ints and you don't have to specify the number of ints. One less potential error right there!

Fixing up a few of the obvious mis-steps and ignoring the elephant in the room, std::vector, for the time being we get something that looks like

#include <initializer_list>
#include <iostream>

class DATA{
    private:
        int length;
        int* tab;
    public:
        DATA(std::initializer_list<int> vals):
            length(vals.size()), tab(new int[length])
        {
            int index = 0;
            for(int val: vals){
                tab[index]=val;
                index++;
            }
        }
        DATA(const DATA& src) = delete;
        DATA& operator=(const DATA& src) = delete;

        friend std::ostream & operator<<(std::ostream & out, const DATA & x){
            if(x.tab!=NULL){
                int index=0;
                out<<x.tab[index];
                for(index++; index < x.length; ++index){
                    out<<", "<<x.tab[index];
                }
            }
            return out;
        }
};
int main(){
    DATA x1({1,2,3,4,5});
    std::cout << x1 << std::endl;
}

Note the {} braces in DATA x1({1,2,3,4,5}); This establishes the list. The list knows how big it is, so you don't have to BLEEP around with telling the function how many items are coming.

Note that I have deleted the copy constructor and assignment operator. This is because of The Rule of Three. What is The Rule of Three? Read the link and find out. It'll save you a lot of future debugging.

This is also ludicrously easy to template:

template <class TYPE>
class DATA{
    private:
        int length;
        TYPE* tab;
    public:
        DATA(std::initializer_list<TYPE> vals):
            length(vals.size()), tab(new TYPE[length])
        {
            int index = 0;
            for(const TYPE& val: vals){
                tab[index]=val;
                index++;
            }
        }

        DATA(const DATA& src) = delete;
        DATA& operator=(const DATA& src) = delete;

        friend std::ostream & operator<<(std::ostream & out, const DATA & x){
            if(x.tab!=NULL){
                int index=0;
                out<<x.tab[index];
                for(index++; index < x.length; ++index){
                    out<<", "<<x.tab[index];
                }
            }
            return out;
        }
};

int main(){
    DATA<int> x1({1,2,3,4,5});
    DATA<std::string> x2({"I", "am", "the", "very", "model", "of", "a", "modern", "major-general"});
    std::cout << x1 << std::endl;
    std::cout << x2 << std::endl;
}
Community
  • 1
  • 1
user4581301
  • 33,082
  • 7
  • 33
  • 54