0

I am new in programming and working on c++ (even the concept of problem is same in c, I guess).

I am reading a file as sole arguments which contains alphabtes(symbols in my code) like "aabbacceaad" to calculate frequency. My code calculate frequency correctly.I am sure of that. The problem is that in my code Node *tree pointer variable (which is of type node). I am using it to create tree.But when i try to create tree from the frequency calculated from repeated symbols then this tree pointer variable only remembers the last executed frequency outside the for-loop. Please take care i just have to use pointers not arrays.

I mean suppose i have symbols in input file like this "aabcddeeebbaa". And the expected output for it is this:

0  symbol:a  Freq:4  Left 0  Right 0  Index1
1  symbol:b  Freq:3  Left 0  Right 0  Index2
2  symbol:c  Freq:1  Left 0  Right 0  Index3
3  symbol:d  Freq:2  Left 0  Right 0  Index4
4  symbol:e  Freq:3  Left 0  Right 0  Index-1

But the output of my code is like this:

0  symbol:e  Freq:3  Left:0  Right:0  Next:5 //Last "e" is executed,tree variable forgot a,b,c and d. 
1  symbol:e  Freq:3  Left:0  Right:0  Next:5
2  symbol:e  Freq:3  Left:0  Right:0  Next:5
3  symbol:e  Freq:3  Left:0  Right:0  Next:5
4  symbol:e  Freq:3  Left:0  Right:0  Next:-1

My full c++ code to do so is :

#include <iostream> 
#include <stdlib.h> 
#include <fstream> 
#include <cassert> 
#include <vector>

using namespace std;

class Huffman {
    public: int data_size,
    length;
    Huffman(char * argv);
    ~Huffman() {};
    vector < char > storesym;
    vector < int > storefreq;
    struct Node
    {
        int value;
        int freq, next;
        short flag;
        unsigned char symbol;
        struct Node * left, * right;
    };
    Node * tree;
};
/////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////       CONSTRUCTOR definition        ////////////////////////////////////////////////////////////////////////////////
Huffman::Huffman(char * argv) 
{
    char c;
    int count = 0;
    int flag[256]; //this flag is to know if the alphabet is already counted or not.If counted that i set it to "1" other wise it is "0".Please see below in my code
    int j = 0;
    FILE * input_file;
    int  *symbolarray;
    symbolarray=new int[30];
    char i, n;
    input_file = fopen(argv, "rb");
    c = fgetc(input_file);
    //////////////////////////////////////////////////////////////////////////////   From here to down i read the alphabbets from file taken as sole argument ///////////////////////////////////////
    while (c != EOF && c != '\n' && c != '\r')
    {
        symbolarray[count] = c;
        count++;
        c = fgetc(input_file);
    }
    fclose(input_file);

    for (i = 0; i < count; i++)
        flag[i] = {0 };
    int fcount1 = 0;
    for (i = 0; i < count; i++)
    {
        if (flag[i] == 0)
        {
            for (j = i; j < count; j++) 
            {
                if (symbolarray[i] == symbolarray[j]&& flag[j] == 0) 
                {
                    fcount1++;
                    flag[j] = 1; //**I am setting flag to 1 those alphabets to 1 so that they will not be counted again on next iteration**
                }
            }
            storesym.push_back(symbolarray[i]);
            storefreq.push_back(fcount1);
        }
        fcount1 = 0;
    }
    cout << endl;
    //////////////////////////////////////////////////////////////////////////  ERROR PRONE PART STARTS NOW  /////////////////////////////////////////////////////

    for (i = 0; i < storesym.size(); i++) 
    {
        tree = new Node;  // the problem is here this tree pointer don't store the values for all alphabets, it just remembers the last executed alphabet after this for loop.
        tree -> left = NULL;
        tree  ->right = NULL;
        tree -> symbol = storesym[i];
        tree  -> freq = storefreq[i];
        tree -> flag = 0;
        tree -> next = i + 1;
        cout<<"check1 : "<<tree -> symbol<<endl;
    } 
    ////////////////////////////////////////////////////////////////////  eror PRONE PART ENDS HERE  ///////////////////////////////////////////////////////////////////////////////////
    cout << "Here is the data read from file" << endl;
    n = storesym.size() - 1;
    for (int i = 0; i < storesym.size(); i++)
    {
        if (n == i)
        {
            tree  -> next = -1;
            cout << i << "  symbol:" << tree  -> symbol << "  Freq:" << tree  ->freq << "  Left:" << tree  -> left << "  Right:" << tree  -> right << "  Next:" << tree  -> next << endl;
            break;
        } else 
        {
            cout << i << "  symbol:" << tree  -> symbol << "  Freq:" << tree -> freq << "  Left:" << tree  -> left << "  Right:" << tree  ->right << "  Next:" << tree  -> next << endl;
        }
    }
}
//////////////////////////////////////////////////////////////////////////////////////////////////////
int main(int argc, char * * argv)
 {
    int freq[256] = {0};
    if (argc < 2) {
        cout << "Ohh.. Sorry , you forgot to provide the Input File please" << endl;
        return (0);
    }
    Huffman Object1(argv[1]);
    return (0);
}

**Please help me how to keep in memory all the "a,b,c,d and e" (Not just last "e") ? I know there is something to be done with pointers only.

Sss
  • 1,519
  • 8
  • 37
  • 67
  • Step through the code in a debugger, line by line. It's usually very helpful. – Some programmer dude Feb 20 '14 at 14:36
  • @JoachimPileborg Ok thanks (1)I am working in Notepad++, which debugger you should me to work on, any name please, i have null idea ? (2) I think there is some logical error related to pointers, Am i right ? – Sss Feb 20 '14 at 14:40
  • @JoachimPileborg well i tried to debug using cout<<"check1 :"< symbol inside the for loop, I found that all the alphabtes are read but only last alphabet is kept in memory outside the for loop. Could you please help me in solving this ? THnaks – Sss Feb 20 '14 at 14:42
  • [How to Compile and Debug C++ in Notepad++ using Turbo C++ Compiler](http://stackoverflow.com/questions/8836400/how-to-compile-and-debug-c-in-notepad-using-turbo-c-compiler) (the answer is basically - get a (proper?) IDE) – Bernhard Barker Feb 20 '14 at 14:43
  • @Dukeling thanks for the suggestion but i guess it's a logical problem related to pointers, Do you have any idea what it could be ? – Sss Feb 20 '14 at 14:47
  • That's the wrong response! There may be someone willing to carefully work thru your code to find a problem, but there may well not. **You** should debug your code and at least find out where it is going wrong, then get back to us. – Tim Bergel Feb 20 '14 at 14:51
  • @TimBergel Thanks for pointing out,I have just edited the code showing the error prone part. where *tree* which a pointer to node is not able to store the alphabtes, Now may i request help to make solution of this problem ? Thanks again – Sss Feb 20 '14 at 14:57
  • OK, in that code you have a loop that repeatedly allocates a node object and stores the address of the new object in tree. Each time you do this you throw away the previous one! So you are only left with the last one. You should be doing something like allocating tree the first time, then on second & subsequent times round allocate another node & then connect that up to tree using the left and right pointers. – Tim Bergel Feb 20 '14 at 15:06
  • @TimBergel Any little piece of code of help please, i will make it as reference to do(i just need the implementation of what you just said to help me), After that i can do it myself.Because i couldn't understand what you have explained,So that i could mark it as vote up. – Sss Feb 20 '14 at 15:19
  • @TimBergel If it was an array i could have done tree[i].symbol=storesym[i];tree[i].left=tree[i].right =NULL; inside the for loop. But don't know how to do the equivalent with pointers, I just Need help of a piece of a code in it's pointer quivalent after i can do it myself, Thanks – Sss Feb 20 '14 at 15:57

1 Answers1

0

This bit

while (c != EOF && c != '\n' && c != '\r')
{
    tree[count].symbol = c;
    count++;
    c = fgetc(input_file);
}

dereferences the uninitialized pointer tree.
This is a big no-no, and means that your program is formally undefined.
You've just been unlucky in that it hasn't crashed.

You're doing it again a bit further down.

Then you allocate in this loop:

for (i = 0; i < storesym.size(); i++) 
{
    tree = new Node;  // the problem is here this tree pointer don't store the values for all alphabets, it just remembers the last executed alphabet after this for loop.
    tree -> left = NULL;
    tree  ->right = NULL;
    tree -> symbol = storesym[i];
    tree  -> freq = storefreq[i];
    tree -> flag = 0;
    tree -> next = i + 1;
    cout<<"check1 : "<<tree -> symbol<<endl;
} 

which repeatedly allocates one Node and points tree at it.
In other words, it's not building a tree, and you need to rewrite it so it actually does.
Whatever course material you're following should have covered trees recently.

The best advice I can give is: start over, read more, and don't write so much code before testing.

molbdnilo
  • 64,751
  • 3
  • 43
  • 82
  • I have just changed the code and removed the first problem (which you suspect to crash the program). For the allocation part i need help, Could you please help me in that, so that i would be able to mark your answer as solved, thanks – Sss Feb 20 '14 at 15:45
  • @user234839 This is the point at which you open your book and study. If you're at school, talk to your classmates and teachers. – molbdnilo Feb 20 '14 at 15:57
  • Actually i have already done it using arrays, f it was an array i could have done tree[i].symbol=storesym[i];tree[i].left=tree[i].right =NULL; inside the for loop. But don't know how to do the equivalent with pointers. So ijust need help in this.(This was the subject of question).So , i actually needed help in doing the same using pointers only (arrays i have deon myself). – Sss Feb 20 '14 at 16:02