Passing array values using pointer

Question

I have two functions one is for file reading and another one for little sorting of numbers. With function read() I am reading file's each line and put each line into array. File looks like:

1
2
3

With function sort() I want to print only numbers with value greater than 1.

What is wrong: I got printed two arrays, but my sort array still printing all values, not only greater than 1.

My code:

    #include <iostream>
    #include <fstream>
    #include <string>
    #include <cstdlib>

    using namespace std;

    class UZD
    {
     private:
    int numLines;
    int *value;
    public:
        UZD();
        int * read();
        int sort();
    }; 
   // ========================================================= 
     UZD::UZD() 
    {
    }
    // ========================================================= 
    int * UZD::read(){  
    ifstream myfile("stu.txt");
    int value[20];
    string line[20]; 
    int i=0;
    while(!myfile.eof() && i < 20) {
      getline(myfile,line[i]); 
      ++i;
            }
     numLines = i;
    for (i=0; i < numLines; ++i) {
        value[i] = atoi(line[i].c_str());
      cout << i << ". " << value[i]<<'\n'; 
            }
    return value;
    }
    // ========================================================= 
    int UZD::sort(){
    int *p;
    p = read();
    int i;
    if(p[i] > 1) {
      cout << p <<'\n'; 
       }
    }
    // ========================================================= 
    int main() {
    UZD wow;
    wow.read();
    wow.sort();
    }

The right tool to solve such problems is your debugger. You should step through your code line-by-line *before* asking on Stack Overflow. For more help, please read [How to debug small programs (by Eric Lippert)](https://ericlippert.com/2014/03/05/how-to-debug-small-programs/). At a minimum, you should [edit] your question to include a [Minimal, Complete, and Verifiable](http://stackoverflow.com/help/mcve) example that reproduces your problem, along with the observations you made in the debugger. — πάντα ῥεῖ, Jun 17 '17 at 17:33
@πάνταῥεῖ I am debuggig all day long, I don't get it what is wrong. — Diggs, Jun 17 '17 at 17:33
Your `read()` function returns a *local* array that is destroyed when the function exits. — Galik, Jun 17 '17 at 17:36
Why don't you use std::array or a vector instead of pointers? They will be much easier to handle and debug. Is it some legacy code with restriction on cpp version? — Sumit Jindal, Jun 17 '17 at 17:36
@Diggs Use `std::vector`, or if you must use native arrays use `new int[]`. — Galik, Jun 17 '17 at 17:37
@SumitJindal I have no clue how to use vectors. And I am always failing to learn them. — Diggs, Jun 17 '17 at 17:38

score 0 · Accepted Answer · answered Jun 17 '17 at 17:58

There are many issues in your code, the most obvious one is "return value" in the read() method. Value is a local array and will be gone once out of scope of read(). Also the design seems faulty. You are calling read() twice, once from the main() and again internally from sort(). I have written a working code, using vectors. Probably this is what you are expecting:

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

using namespace std;

class UZD
{
        private:
                int numLines;
                vector<int> value;
        public:
                UZD();
                vector<int> & read();
                void sort();
};
// =========================================================
UZD::UZD()
{
}
// =========================================================
vector<int> & UZD::read(){
        ifstream myfile("stu.txt");
        vector<string> line(20);
        int i=0;
        while(!myfile.eof() && i < 20) {
                getline(myfile,line[i]);
                ++i;
        }
        numLines = i;
        cout << "After reading file: " << endl;
        for (i=0; i < numLines; ++i) {
                value.push_back(atoi(line[i].c_str()));
                cout << i << ". " << value[i]<<'\n';
        }
        return value;
}
// =========================================================
void UZD::sort(){
        cout << "Inside sort()" << endl;
        for(int i=0; i<value.size(); ++i){
                if(value[i] > 1)
                        cout << value[i] << endl;
        }
}
// =========================================================
int main() {
        UZD wow;
        wow.read();
        wow.sort();
        return 0;
}

I have kept the variable names same for clarity. Let me know if you don't get anything.

Thank you very much. I write another function with taking sort fuction array and writing to file instantly with Your example. Thank You again. — Diggs, Jun 17 '17 at 18:08
@Diggs I am happy to help. I have added one more answer, without vectors and corrected your own code. Check that. — Sumit Jindal, Jun 17 '17 at 18:19

score 0 · Answer 2 · answered Jun 17 '17 at 18:01

There are a lot of issues in your program; just to mention some of them: In sort, you use variable i uninitialized, which is undefined behaviour (probably crashes); You write while(!myfile.eof()..., which is usually considered wrong (see this SO answer; The use of atoi is not recommended, as it is not safe if the parameter does not represent a number; You return a pointer to a local variable, which is destroyed one going out of scope; You declare member variables, but pass through local ones (e.g. value)...

See the following code which demonstrates the usage of int* and a vector<int>; hope it helps.

class UZD
{
private:
    int numLines;
    int *value = nullptr;
public:
    ~UZD() {
        if (value)
            delete value;
    };
    void read();
    void print();
};

// =========================================================
void UZD::read(){
    ifstream myfile("stu.txt");
    value = new int[20];
    int val;
    numLines = 0;
    while(numLines < 20 && myfile >> val) {
        value[numLines] = val;
        numLines++;
    }
}
// =========================================================
void UZD::print(){
    for (int i=0; i<numLines; i++)
        cout << value[i] << endl;
}


class UZD_vector
{
private:
    vector<int> values;
public:
    void read();
    void print();
};

// =========================================================
void UZD_vector::read(){
    ifstream myfile("stu.txt");
    int val;
    while(myfile >> val) {
        values.push_back(val);
    }
}
// =========================================================
void UZD_vector::print(){
    for (auto val : values)
        cout << val << endl;
}

// ========================================================= int main() {

cout << "with int[]:" << endl;
UZD wow;
wow.read();
wow.print();

cout << "with vector:" << endl;
UZD wow_vector;
wow_vector.read();
wow_vector.print();

}

score 0 · Answer 3 · answered Jun 17 '17 at 18:08

0

Here is your own code rectified, just in case you find vectors too difficult to learn (which should not be true, though)

#include <iostream>
#include <fstream>
#include <string>
#include <cstdlib>

using namespace std;

class UZD
{
        private:
                int numLines;
                int *value;
                int num;
        public:
                UZD();
                void read();
                void sort();
};
// =========================================================
UZD::UZD():num(20)
{}
// =========================================================
void UZD::read(){
        ifstream myfile("stu.txt");
        value = new int[num];
        string line[num];
        int i=0;
        while(!myfile.eof() && i < num) {
                getline(myfile,line[i]);
                ++i;
        }
        numLines = i;
        for (i=0; i < numLines; ++i) {
                value[i] = atoi(line[i].c_str());
                cout << i << ". " << value[i]<<'\n';
        }
}
// =========================================================
void UZD::sort(){
        cout << "After sorting: " << endl;
        for (int i = 0; i < num; ++i) {
                if(value[i] > 1)
                        cout << value[i] << endl;
        }
}
// =========================================================
int main() {
        UZD wow;
        wow.read();
        wow.sort();
        return 0;
}

answered Jun 17 '17 at 18:08

Sumit Jindal

194
2
13

just dumping a code, without explaining where is the mistake in the OP code and without explaining how does this code improves on the original is not helpful. – bolov Jun 17 '17 at 18:36
@bolov I have not done "just dumping a code". This answer is a continuation to my previous answer, where I clearly pointed out the major issues in the original code and implemented a solution using vectors. Then I rectified the original code thinking that there might be restriction on using vectors. You did not check the earlier answers and just criticised :-) – Sumit Jindal Jun 18 '17 at 07:52
No, I have not checked the other answers. And I shouldn't need to in order to understand this answer. An answer should stand alone, you can't count on people reading all the answers in order to understand one answer. On posts with many answers your two answers can become very far apart. I say again: just as a question should have legs on it's own, without needed to check some external links to understand it, an answer should be understood alone. – bolov Jun 18 '17 at 13:21
You can combine your two answers into one (I don't see why not) or you could at least begin with "this is a complementary answer to my previous one and insert a link" – bolov Jun 18 '17 at 13:22
Thanks, I will keep these points in mind next time, and will also edit this answer to make it more meaningful and complete! – Sumit Jindal Jun 18 '17 at 18:54

Passing array values using pointer

3 Answers3