Function for store data - C++ Given the following definition for a NODE in a linked list:

struct NODE {
int data;
NODE* next;
};


I want a function to store the nodes with odd data to a given file (each line should contain a single integer number) [use c++ classes from ]

I wrote this code but there is some problems with the output .. Where is the wrong or what is the best solution for the question??

void storeOddElements(NODE *head, char *FileName)
{
    NODE *p = head;
    int count = 0;

    ofstream fp(FileName, ios::out);
    if(!fp.is_open()) return;

    while (p != NULL)
    {
        if ((p-&gt;data % 2) == 1)
            count++;
        p = p-&gt;next;

    }

    int odd_store[count], i=0;
    while (p !=NULL) {
        if ((p-&gt;data % 2) == 0)
            p = p-&gt;next;
        else
        {
            odd_store[i] = p-&gt;data;
            p = p-&gt;next;
            i++;
        }
    }

    int temp;
    for (int z = 0; z &lt;= i-1; z++) {
        for (int j = i+1; j &lt;= i; j++) {
            if (odd_store[z] &lt;= odd_store[j]) {
                continue;
            }
            else
            {
                temp = odd_store[z];
                odd_store[z] = odd_store [j];
                odd_store[j] = temp;
            }
        }
    }
    for (int x = 0; x &lt;= i; x++) {
        fp &lt;&lt; odd_store[x] &lt;&lt; endl;
    }
    fp.close();
}
Why are you passing a file name?

void storeOddElements(NODE *head, char *FileName)


A file stream is the same concept as a stream. So writ the code to use a stream. Then you can pass std::cout while you are testing then swap in std::ofstream object when you are ready to pass a file.

Technically this is illegal C++;

int odd_store[count];


To be legal count must be determined at compile time. Though some compilers support this as an extension. It is better to use std::vector&lt;&gt; for this situation.

BUT why are you counting and storing the output values into an array in the first place? That seems like a redundant waste of time. Just print them directly to the file stream.

Then you sort the stored values. I see no requirement that the data be sorted.

Looking at your sort you implement a swap operation rather than using std::swap(). But you don't even need to implement a sort function there is a standard one std::sort that implements a better algorithm than the bubble sort you are using.

Bu the easiest way is to store them in a sorted container when you retrieve them from the list std::set would work for that.

Lastly the copy from the container to the output uses a loop. Learn how to use the standard algorithm or the range based for to make the code easier to read.

Also prefer to use "\n" rather than std::endl. Using std::endl will only slow your code down.

Personally I would write it like this:

void storeOddElements(NODE *head, std::ostream&amp; output)
{
    std::copy_if(NodeIterator(head), NodeIterator(),
                 std::ostream_iterator&lt;int&gt;(output, "\n"),
                 [](auto const&amp; val){return val % 2 == 1;});
}


Now you only need to define a NodeIterator that can iterate over a list of Node. :-)

OK Got bored. Here is a NodeIterator.

class NodeIterator
{
    Node* current;
    public:
        NodeIterator(Node* begin = nullptr)
            : current(begin)
        {}
        NodeIterator&amp; operator++()   {current = current-&gt;next;return *this}
        NodeIterator  operator++(int){NodeIterator result(*this); (*this)++; return result;}

        bool operator==(NodeIterator const&amp; other) const {return current == other.current;}
        bool operator!=(NodeIterator const&amp; other) const {return !(*this == other);}
        int operator*()                       {return current-&gt;data;}

        // iterator traits
        using difference_type   = long;
        using value_type        = int;
        using pointer           = int*;
        using reference         = int&amp;;
        using iterator_category = std::forward_iterator_tag;
};


If we need to sort the data before output then we can use:

void storeOddElements(Node *head, std::ostream&amp; output)
{
    std::set&lt;int&gt;  sortedData;
    std::copy_if(NodeIterator(head), NodeIterator(),
                 std::inserter(sortedData, sortedData.end()),
                 [](auto const&amp; val){return val % 2 == 1;});

    std::copy(std::begin(sortedData), std::end(sortedData),
              std::ostream_iterator&lt;int&gt;(output, "\n"));
}