Solved

Overloading operator [] , composing classes, using new and delete

Posted on 2003-12-02
37
1,701 Views
Last Modified: 2013-12-14
Hi all,

I would like to implement an association table that maps names (character strings) into numbers. To do this, I am trying to overload the subscripting operator[].

This uses names to index an array instead of numbers.
If the particular element doesn't exist, then it should be created and initialized. There is a special syntax to tell the program not to create the element, but you do not have duplicate that feature.


The program should be able to do the following in the main program


assoc B;

B["element 1"] = 111; // this would assign 111 into the element

if (B["element 2"] == 4) { // then this would create an element and initialize it to 0


}


I've created a template for this...but am getting the following errors :

error C2107: illegal index, indirection not allowed
error C2440: 'return' : cannot convert from 'int *' to 'int &'

Could someone please help me out here.....

here's my code:
------------------

#include <iostream>
#include <string>

using namespace std;


class assoc {
      int *data;
      char * str;
  public:
      int &operator[] (char *);
};
      

int & assoc::operator[](char * s)
{
       char * str = new char(strlen(s) + 1);
       strcpy(str,s);
       return data[str];
}
      
int main()
{
     
    assoc B;
     B["element 1"] = 111;
     return 0;

}
      
0
Comment
Question by:anushan
  • 14
  • 10
  • 8
  • +2
37 Comments
 
LVL 2

Expert Comment

by:colmcc
Comment Utility
Presumably this is homework.  (Otherwise, the STL container class 'map' already does exactly what you want.)

Regarding your code, it's difficult to see exactly how you are trying to go about this.  However, a few issues are -

1. If 'int* data' is meant to point to an array of ints, then you need to allocate the space for that array. (E.g., in a constructor for the assoc class.)

2. You can't index into that array using a char* (i.e. 'return data[str];'). This will be the cause of the error messages.  The whole point of the exercise you are doing is to create a class capable of this form of indexing.

3. There's no reason to allocate space for a new copy of the index string each time the [] operator is invoked.  Moreover, you're not deleting this space anywhere.  Also, unless you've been told to use char*, you'd be better of using the string type.

4. You haven't included any code to do the actual work of associating ints with strings.  You need a list of int,string pairs. operator[] needs to search that list for an entry containing the given string, and return the corresponding int.

I hope this helps get you started ....

Colin.

0
 

Author Comment

by:anushan
Comment Utility
Hi Colin,

Thanks for getting me started...I am quite unclear on your 4th point.....how do I do the actual work of associating the ints with strings....I wrote up a program that works the other way round...i've modified the code following some of your suggestions......can you please help me to proceed further??


#include <iostream>
#include <string>

using namespace std;

class assoc {
      int *data;
      string str;
        public:
      assoc();
      ~assoc();
      int &operator[] (const string);
};
      
assoc::assoc()
{
                 int *data = new int;
}

assoc::~assoc()
{
      delete data;
}

int & assoc::operator[](const string s)
{
      return data[s];
}

      
int main()
{
      assoc B;
      B["element 1"] = 111;
      return 0;
}
0
 

Author Comment

by:anushan
Comment Utility
And yes..you are right..I can use the STL container class "map"..but have to do it the troublesome way :)....and am stuck in the mapping
0
 
LVL 11

Expert Comment

by:bcladd
Comment Utility
Okay. You need to associate strings with ints. This certainly implies that you have MANY strings, EACH associated with a different integer, right?

Take a look at your data members. How many strings do you have?

Your constructor has two serious flaws:
   (a) if you declare a local variable with the same name as a global OR class level variable, the local variable masks the variable in the outer scope. That means that you have a variable called data that exists solely in your constructor and you assign memory to it. When the constructor finishes, the local data goes away, taking the pointer value along with it.
  (b) you declare space for one (1) integer. Presumably you want more than one integer (or assoc really only associates one string to one int? I don't think so).

Your destructor shoudl check to make sure data is non-NULL before deleting it.

Forget the operator and forget returning a reference: how would you associate a string with an integer so you could use a getValueFor(string) function, something like:

assoc a;
a.setValueFor("abc", 1);
a.setValueFor("zyx", 2);

int i = a.getValueFor("abc");

What would you have to do inside of getValueFor? What data would you have to store?

Hope this gets you thinking (and that it helps), -bcl
0
 
LVL 2

Expert Comment

by:colmcc
Comment Utility
Hi,

OK. A few more hints, but then I'm afraid it's up to you.  This is actually quite a complicated task, so you should not be worried if it is taking you some effort to get to grips with it.  Also, as I don't know what language features you have and haven't covered yet, it's best that I just talk in pseudo-code terms.

Imagine you had a working assoc class.  Then if you were to make use of it with code such as the following -

(1)  assoc b;
(2)  b["Fred"] = 111;
(3)  b["Barney"] = 222;
(4)  b["Barney"] = 99;
(5)  b["Wilma"] = 333;
(6)  cout << b["Barney"] << endl;

- think about what has to happen.  For each [] operation, you need to check to see if a mapping already exists, and to create one if it doesn't.  Most likely you will want to loop through a list of mappings looking for one with a matching string.

So for(2), this is the first time "Fred" has been mentioned and the list is empty.  And after (2) has been executed the list will contain just one entry -
{ {"Fred",111} }

Similarly, (3) is the 1st mention of "Barney".  The list is not empty this time, but when it is searched the only mapping is for "Fred".  So again a new entry is added to the list -
{ {"Fred",111}, {"Barney",222} }

For (4), when the list is searched, "Fred" is not a match but "Barney" is.  So you don't want to add a new entry, but rather modify an existing one -
{ {"Fred",111}, {"Barney",99} }

(5) is the same as (3).  Add a new mapping entry, for "Wilma" this time -
{ {"Fred",111}, {"Barney",99}, {"Wilma",333} }

Finally (6) loops through the list until it finds the entry for "Barney" and returns a reference to the associated integer (99).

One subtlety to note is why operator[] returns a reference to int, and not just an int.  Because the expression b["text"] represents a reference, it can be used both as a value (e.g., in (6) it will be evaluated as 99, which is what will be displayed.), and also as the left-hand-side of an assignment operation, allowing the value to be changed.  Thus if you implement operator[] correctly, you shouldn't have to do anything to get operator= to work.  That should just happen as a natural consequence.

Good luck!
Colin
0
 
LVL 2

Expert Comment

by:colmcc
Comment Utility
>Your destructor shoudl check to make sure data is non-NULL before deleting it.

Actually the C++ standard explicitly says that is not necessary.

"The delete operator may be applied only to a pointer returned by new or to zero.  Applying delete to zero has no effect."

I agree with everything else though. :-)

Regards,
Colin.
0
 
LVL 3

Expert Comment

by:monkesdb
Comment Utility
if your still allowed to use stl but not the stl::map then take a look at these snippets of code.

// a place to store the associations
vector<pair<string, int> > pairs;

// to find an element who's key is searchStr
pairs.iterator p = find_if(pairs.begin(), pairs.end(), compose1(bind2nd(equal_to<string>(), searchStr), select1st<pairs.value_type>()));

// test to see if p was not found
if( p == pairs.end() )
 
// to insert myString : myInt pair
pairs.push_back(pair(myString, myInt));

//to change the value if the string was found
p->second = myInt;



with this code you cn write your [] operator in 6/7 lines.

0
 
LVL 4

Assisted Solution

by:void_main
void_main earned 50 total points
Comment Utility
Hi @all

It is not impossible to associate strings with arrays but you have to use an alias.
A string like this "hello world" represents a pointer! Your compiler creates a "string-table" and replaces any "hello world" by its address in the table (commonly a RAM address like 0x0a49fa3ac) <-- and this one is NEVER a name for an array index!!!

But you can deal with it: (by associating numbers to the strings and working internal with the numbers)

// I keep it as short as possible

typedef struct alias_s           // You can create a linked list if you want...
{
   char name[25];
   int associatedNumber;
} alias_t;

class myArray
{
     public:
        myArray(int elements = 5);
        ~myArray() {delete[] itsData;}
        void addAlias(char *name, int associatedNumber = -1);     // -1 means default
        void removeAlias(char *name);
        int getAlias(char *name, int realNumber = 1);                  // 1 means not to return the associated number, if it differs
        int getElement(char *alias);
        void setElement(char *alias, int value);
    private:
        alias_t allAliases[100];
        int *itsData;
};

myArray::myArray(int elements)
 {
    itsData = new int[elements];      // alloc new memory
    memset(allAliases, 0, 100 * sizeof(alias_s));     // clear alias table
}


void myArray::addAlias(char *name, int associatedNumber)
{
    if (getAlias(name) != -1)
    {
           // Error! Alias exists already
           return;
    }
    for (int x = 0; x < 100; x++)
    {
          if (strlen(allAliases[x].name) == 0)
          {
                 // Found empty alias name
                 strcpy(allAliases[x].name, name);
                 allAliases[x].associatedNumber = (associatedNumber == -1)? x : associatedNumber;
                 break; // VERY important!!!
          }
    }
}

int myArray::getAlias(char *name, int realNum)
{
    for (int x = 0; x < 100; x++)
    {
          if (strcmp(allAliases[x].name, name) == 0)
          {
               // Found alias and return associated number
               if (realNum == 1)
                  return allAliases[x].associatedNumber;

               // no need for else
               return x;
          }
    }
    // Not found
    return -1;
}

void myArray::removeAlias(char *name)
{
     int buffer = getAlias(name, 0);     // get the real index
     if (buffer == -1)
     {
           // Error: Alias does not exist
           return;
     }
     memset(allAliases[buffer], 0, sizeof(alias_s));
}

int myArray::getElement(char *alias)
{
     int buffer = getAlias(name);     // get the index
     if (buffer == -1)
     {
           // Error: Alias name NOT registered
           return;
     }
     return(  itsData[   allAliases[buffer].associatedNumber   ]    );      // Whitespaced to be more clearly
}

void setElement(char *alias, int value)
{
     int buffer = getAlias(name);     // get the index
     if (buffer == -1)
     {
           // Error: Alias name NOT registered
           return;
     }
     itsData[   allAliases[buffer].associatedNumber   ]   = value;      // Whitespaced to be more clearly
}

// and, finally the main
void main()
{
      myArray some(20);      // alloc 20 elements
      some.addAlias("ralph");
      some.addAlias("george");
      some.addAlias("craig");

     cout << "Element \"george\" is " << some.getElement("george") << "\n";

      some.setElement("george", 500);

     cout << "Element \"george\" is now " << some.getElement("george") << "\n";

      some.removeAlias("ralph");         // you don't need to because the aliases are not stored in the heap
      some.removeAlias("george");
      some.removeAlias("craig");

     system("pause");
}

// please note: I do NOT give you the guarantee, that it works
0
 
LVL 2

Expert Comment

by:colmcc
Comment Utility
Hi void_main,

I think we are all aware of the issues involved.  However, we were trying to give the questioner some help with his homework without actually doing it for him.  This is because
a) In the long run that does not help him.
b) It's against the rules.

Please co-opearte with this way of working.

Regards,
Colin
0
 

Author Comment

by:anushan
Comment Utility
Hello Colin, monkesdb, void main and bcladd,

Thanks for all your suggestions...I'm going to try out the problem with some of the hints
suggested by you......

monkesdb....I'm not sure if I could use the stl::vector to complete by program..I will check with my prof on that...

Thanks again..and I will post queries again as I proceed with the problem :)

cheers
0
 
LVL 4

Expert Comment

by:void_main
Comment Utility
Oops!
I did not see that THIS was a homework (he did not wrote this in his question, so I assumed it was not)

And you're right, Colin!
But if I startthinking about any problem in C++ I can't stop until it is complete (in this forum I should pull the break a bit! ;-)  ), if I write a piece of code I almost can't stop  (look my other posts)


regards!
0
 

Author Comment

by:anushan
Comment Utility
Hi all,

I've used some of your help tips and modified the program that you had suggested to work with my main()....and I get 2 errors...here goes :

#include <iostream>
#include <string>

using namespace std;

typedef struct alias_s           // Using a structure called alias...
{
   char name[25];
   int associatedNumber;
} alias_t;

class assoc
{
     public:
        assoc(int);
            assoc();
        ~assoc() {delete[] itsData;}
         int &operator[] (char *);    
        int getAlias(char *name);        

    private:
        alias_t allAliases[100];
        int *itsData;
};

assoc::assoc(int elements)
{
    itsData = new int[elements];      // alloc new memory
    memset(allAliases, 0, 100 * sizeof(alias_s));     // clear alias table
}

assoc::assoc()
{
      itsData = new int;
}


int & assoc::operator[] (char *name)
{
    int buffer = getAlias(name);  // get the index
    if (buffer != -1)
    {
 *Err**      itsData[   allAliases[buffer].associatedNumber   ]   = value;  
    }
    for (int x = 0; x < 100; x++)
    {
          if (strlen(allAliases[x].name) == 0)
          {
                 // Found empty alias name
             strcpy(allAliases[x].name, name);
 *Err** allAliases[x].associatedNumber = (associatedNumber == -1)? x : associatedNumber;
                 break; // VERY important!!!
          }
    }

    return(  itsData[   allAliases[buffer].associatedNumber   ] ) ;
}


int assoc::getAlias(char *name)
{
    for (int x = 0; x < 100; x++)
    {
          if (strcmp(allAliases[x].name, name) == 0)
          {
               // Found alias and return associated number
               
                  return allAliases[x].associatedNumber;

          }
    }
    // Not found
    return -1;
}


void main()
{
      assoc B(20);      // alloc 20 elements
      B["element1"]=111;

       cout << "Element \"element1\" is " << B["element1"] << "\n";

       system("pause");
}


error C2065: 'value' : undeclared identifier
error C2065: 'associatedNumber' : undeclared identifier

The errors are in lines marked ***** in the code. I know why the error for value is turning
up but do not know how to rectify it.
I can't think of a way to pass the integer "value" which is "111" (as per the main() program) to the Subcripting operator [] function.
Please help !

regards
0
 
LVL 11

Expert Comment

by:bcladd
Comment Utility
(1) At the first line with the problem, you don't want to set the value in the element because you don't, can't know the value to set. In fact, at that point the operator[] has no way of knowing if it is to be used as an lvalue or an rvalue. So the response should be to return a reference to the integer that is going to be manipulated and let the rest of the program do what it will with that int.

(2) As above: You have a new entry in the assoc but you still don't know what is needed. Return a reference to the stored int and let main do the work.


The key is if B["abc"] returns a REFERENCE to the int associated with "abc" then you are good to go with an assignment statement IN main(), NOT in operator[]. operator[] returns an lvalue as is required for assignment. When you print out B["abc"] the operator still returns a reference (an lvalue) but the compiler knows that and dereferences it for its rvalue and prints that out.

Hope this helps, -bcl
0
 

Author Comment

by:anushan
Comment Utility
Hi bcl,

I followed your instructions and returned a reference to the int associated with "element1".
Hence I've resolved the problem with the "value" variable. But I still have an 2nd error flagging..I can't pinpoint out on what I am doing wrong....
Herez the modified member functions :

int & assoc::operator[] (char *name)
{
    int buffer = getAlias(name);
    for (int x = 0; x < 100; x++)
    {
          if (strlen(allAliases[x].name) == 0)
          {
             strcpy(allAliases[x].name, name);  // Found empty alias name
 **Err* allAliases[x].associatedNumber = (associatedNumber == -1)? x :associatedNumber;
              break;                          
          }
    }  
      return(  itsData[   allAliases[buffer].associatedNumber  ] ) ;

}

int assoc::getAlias(char *name)
{
    for (int x = 0; x < 100; x++)
    {
          if (strcmp(allAliases[x].name, name) == 0)
          {
                         
                  return allAliases[x].associatedNumber;
                 
          }
    }
    // Not found
    return -1;
}
0
 
LVL 11

Expert Comment

by:bcladd
Comment Utility
What int shoudl you return? Since you copied the name into allAliases[x].name, I would suggest the integer that [name] should refer to is allAliases[x].assocatedNumber.

Why? Because when a new string is used to index the assoc you want to "add" a new value to the assoc (by having an entry indexed by the new string).

Consider what happens  when the following line is executed:

    B["alpha"] = 7;

(a) operator[] is called with name="alpha"
(b) getAlias is called and fails to match "alpha" so
(c) the loop should find an open slot, give it the name "alpha" (so that next time that name is used it will be found) and return the int associated with the newly "allocated" or used element in the assoc.

One problem with your code: You need an if based on what getAlias returns. If it returns -1 (from your previous post that is if it misses) THEN and ONLY THEN do you want to run the loop. If you only want one return statement in the function, you can replace the line with the error by setting buffer to be equal to x and then break out of the loop. That way you always return a reference to an entry in the assoc that has the given name (whether it was found by getAlias or selected from the empty slots by the loop).

-bcl
0
 

Author Comment

by:anushan
Comment Utility
Thanks for your valuable advice bcl......and please be patient with me as I am new to C++.
Sorry for these mulitple messages...but I think am getting there !!

Now I have no error during compilation...but  I get an application error when I execute using VC++ 6.0

int & assoc::operator[] (char *name)
{
    int buffer = getAlias(name);  // get the index
    if (buffer != -1)
    {
     return (itsData[   allAliases[buffer].associatedNumber   ] );
    }
    for (int x = 0; x < 100; x++)
    {
          if (strlen(allAliases[x].name) == 0)
          {
             strcpy(allAliases[x].name, name);
            allAliases[x].associatedNumber = (associatedNumber == -1)? x : associatedNumber;
            buffer = x;
             break; // VERY important!!!
          }
    }

    return(  itsData[   allAliases[buffer].associatedNumber   ] ) ;
}


int assoc::getAlias(char *name)
{
    for (int x = 0; x < 100; x++)
    {
          if (strcmp(allAliases[x].name, name) == 0)
          {
               // Found alias and return associated number
               
                  return allAliases[x].associatedNumber;

          }
    }
    // Not found
    return -1;
}

0
 

Author Comment

by:anushan
Comment Utility
Oops sorry...there is a small mistake in the above [] operator function

here goes..

int & assoc::operator[] (char *name)
{
    int buffer = getAlias(name);  // get the index
    if (buffer != -1)
    {
     return (itsData[   allAliases[buffer].associatedNumber   ] );
    }
    for (int x = 0; x < 100; x++)
    {
          if (strlen(allAliases[x].name) == 0)
          {
             strcpy(allAliases[x].name, name);
            buffer = x;
             break; // VERY important!!!
          }
    }
    return(  itsData[   allAliases[buffer].associatedNumber   ] ) ;
}

int assoc::getAlias(char *name)
{
    for (int x = 0; x < 100; x++)
    {
          if (strcmp(allAliases[x].name, name) == 0)
          {        
                  return allAliases[x].associatedNumber;
          }
    }
    // Not found
    return -1;
}
0
 

Author Comment

by:anushan
Comment Utility
I tried the following tooo...and it gives the same "Application error" upon execution (but no compilation error)

int & assoc::operator[] (char *name)
{
    int buffer = getAlias(name);  // get the index
    if (buffer == -1)  // if alias does not exist
    {
     for (int x = 0; x < 100; x++)
     {
          if (strlen(allAliases[x].name) == 0)
          {
             strcpy(allAliases[x].name, name);
             buffer = x;
             break; // VERY important!!!
          }
     }
    }
    return(  itsData[   allAliases[buffer].associatedNumber   ] ) ;
}
0
Why You Should Analyze Threat Actor TTPs

After years of analyzing threat actor behavior, it’s become clear that at any given time there are specific tactics, techniques, and procedures (TTPs) that are particularly prevalent. By analyzing and understanding these TTPs, you can dramatically enhance your security program.

 
LVL 11

Expert Comment

by:bcladd
Comment Utility
Do you know where the run-time error is occurring? When you run the program from inside the IDE if you ask to break the program it should show you the line that was execting when the error happened.
That information would make it easier for me to help.

-bcl
0
 

Author Comment

by:anushan
Comment Utility
Hi bcl,

At last...the code works when I plug it into the Unix environment (I can also connect through secure shell to our Unix server)....and I used g++ to compile the code.
When I run this code from the unix environment, I have no problem...only when I plug the same code into VC++ 6.0, I get the run time application error.  VC++ is only available in my work area, so I will go to work tomorrow, compile it and send the exact line that was executing when the error happened.

Once I get that resolved, I should be able to close this issue...thanks for patiently listening to my queries

cheers
0
 
LVL 4

Expert Comment

by:void_main
Comment Utility
int & assoc::operator[] (char *name)
{
    int buffer = getAlias(name);  // get the index

    if (buffer == -1)  // if alias does not exist
    {
        return -1;
    }

    return(  itsData[   allAliases[buffer].associatedNumber   ] ) ;
}

you missused the source. This code should do the job.
The old source (look in the other post) was used to create a new index. The "strcpy" overwrites the alias.

best regards
0
 
LVL 2

Expert Comment

by:colmcc
Comment Utility
>The old source (look in the other post) was used to create a new index.
Which is exactly what the spec says operator[]() is supposed to do, if one does not already exist.  Anushan is on the right track, I think.

I don't want to cause offence, but I must re-iterate that I think it is a big mistake to copy masses of code from someone offering help.  Any programmer, no matter how experienced, will make a number of mistakes when writing that much code without testing it.  A student is far better employed finding and fixing their own mistakes than somebody else's!

In this particular case, I have doubts about the use of the itsData array.  I really can't see what its purpose is.  In fact, I'm pretty sure you need to get rid of it to make the code work as you intend.
So, for example,
return(  itsData[   allAliases[buffer].associatedNumber   ] ) ;
becomes
return(  allAliases[buffer].associatedNumber) ;

Regards,
Colin
0
 
LVL 11

Expert Comment

by:bcladd
Comment Utility
I wasn't paying attention to where itsData was, instead focusing on details. I must agree with Colin. Get rid of it. His suggested change is correct and will make the code much, much easier to understand.

-bcl
0
 

Author Comment

by:anushan
Comment Utility
Hi Colin & bcl,

When I do not use the itsData array for return (to return a  reference to the integer) , I do
not get the correct ouput when I create and initialize a NEW element. It works ok when it re-intializes an already existing element...
Once I return using the itsData array to return a reference to the integer, the output is as expected for this particular main( ).

Here is the operator [] function without using the itsData array for return:

int & assoc::operator[] (char *name)
{
    int buffer = getAlias(name);  // get the index
    if (buffer == -1)  // if alias does not exist
    {
     for (int x = 0; x < 100; x++)
     {
          if (strlen(allAliases[x].name) == 0)
          {
             strcpy(allAliases[x].name, name);
             buffer = x;
             break; // VERY important!!!
          }
     }
    }
    return(  allAliases[buffer].associatedNumber  ) ;    // changed as per your suggestion
}


The main program :
----------------------


void main()
{
      assoc B(20);      // alloc 20 elements
      B["element1"]=111;

       cout << "Element \"element1\" is " << B["element1"] << endl;
      B["element2"]=113;
      B["element1"]=114;
      int k = B["element1"];
      cout << "Element \"element1\" is now " <<  k << endl;
}


The output
-------------

Element "element1" is 1966029422    // this should actually be 111
Element "element1" is now 114


** As I mentioned above, if I use the itsData array to return a reference to the integer the output is as expected !!
0
 
LVL 11

Expert Comment

by:bcladd
Comment Utility
Could you post all of the code? It would be easier for me to compile if I had the whole package. I want to know what is happening because it LOOKS like it should work.

Thanks, -bcl
0
 

Author Comment

by:anushan
Comment Utility
Here's the complete code :


#include <iostream>
#include <string>

using namespace std;

typedef struct alias_s           // Using a structure called alias...
{
   char name[25];
   int associatedNumber;
} alias_t;

class assoc
{
     public:
        assoc(int);
          assoc();
        ~assoc() {delete[] itsData;}
         int &operator[] (char *);
        int getAlias(char *name);

    private:
        alias_t allAliases[100];
        int *itsData;
};

assoc::assoc(int elements)
{
    itsData = new int[elements];      // alloc new memory
    memset(allAliases, 0, 100 * sizeof(alias_s));     // clear alias table
}

assoc::assoc()
{
     itsData = new int;
}


int & assoc::operator[] (char *name)
{
    int buffer = getAlias(name);  // get the index
    if (buffer == -1)  // if alias does not exist
    {
     for (int x = 0; x < 100; x++)
     {
          if (strlen(allAliases[x].name) == 0)
          {
             strcpy(allAliases[x].name, name);
             buffer = x;
             break; // VERY important!!!
          }
     }
    }
    return( allAliases[buffer].associatedNumber ) ;
}





int assoc::getAlias(char *name)
{
    for (int x = 0; x < 100; x++)
    {
          if (strcmp(allAliases[x].name, name) == 0)
          {
               // Found alias and return associated number

                  return allAliases[x].associatedNumber;

          }
    }
    // Not found
    return -1;
}


void main()
{
      assoc B(20);      // alloc 20 elements
      B["element1"]=111;

       cout << "Element \"element1\" is " << B["element1"] << endl;
      B["element2"]=113;
      B["element1"]=114;
      int k = B["element1"];
      cout << "Element \"element1\" is now " <<  k << endl;
}

0
 
LVL 11

Expert Comment

by:bcladd
Comment Utility
getAlias should return x, not allAliases[x].associatedNumber.

-bcl
0
 
LVL 2

Expert Comment

by:colmcc
Comment Utility
Hi

I think GetAlias() also needs changing.

return allAliases[x].associatedNumber;

becomes

return x;

The point that we were making was not that you should just change one line of code because we say so.  Rather, you should think about how it is you think YOUR program is working.  If you include the itsData array, it must be because you think there is a reason for it, not because it was in somebody else's attempt at a solution.  But our advice is that we can't see any reason for it, and so wonder why you would want it.

Regards,
Colin.

0
 
LVL 2

Expert Comment

by:colmcc
Comment Utility
Ah, I see bcl won the race :-)
0
 
LVL 11

Expert Comment

by:bcladd
Comment Utility
I agree with Colin: The point is to understand why getAlias should return the index of the matching alias rather than the value stored IN the matching alias. If you understand WHY the code needs changing then you will have less trouble writing different code in the future AND will be better able to evaluate answers you get to questions you ask.

-bcl
0
 
LVL 4

Expert Comment

by:void_main
Comment Utility
@Colin: exactly!!!!
but why this:
----------------------------------
So, for example,
return(  itsData[   allAliases[buffer].associatedNumber   ] ) ;
becomes
return(  allAliases[buffer].associatedNumber) ;
----------------------------------
operator[] should not return the index of the object to be returned, but instead a reference to the object itself!
I don't understand why (...) becomes (...).

The statement with the code-copy-paste is a very important! But it is (partly) my own fault because I cant stop programming when I started with solving one problem.

regards
0
 
LVL 11

Expert Comment

by:bcladd
Comment Utility
void_main:

What is the object referred to? The objects managed inside this associative array are the string/int pairs and operator[] should return a reference to the integer associated with the given string.

itsData was a redundant array; why associate a string with an index into an array of integers when you can associate it directly with an integer value?

You might argue that itsData works better in a templated context but you can change the pair structure to be string/T and the direct storage scheme continues to work.

Hope this makes sense.

-bcl
0
 

Author Comment

by:anushan
Comment Utility
It's now very clear why itsData integer array is not required. I have removed it completely from the code and changed it to accomodate by main program. I've removed the constructor assoc(int elements) from my code as it is not required. Also, I've changed the datatype of the structure from char[25] to char *, as I do not know the length of the string as per the question.

This compiles and runs well, but I would like to verify the final with the experts :

#include <iostream>
#include <string>

using namespace std;

typedef struct alias_s           // Using a structure called alias...
{
   char * name;
   int associatedNumber;
} alias_t;

class assoc
{
     public:
          assoc();
         ~assoc();
         int &operator[] (char *);
        int getAlias(char *name);

    private:
        alias_t allAliases[100];
};

assoc::assoc()
{
      memset(allAliases, 0, 100 * sizeof(alias_s));   // clear alias table
      for (int x=0; x < 100 ; x++)
      {
            allAliases[x].name = new char;
      }
     
}

assoc::~assoc()
{
      for (int x=0; x < 100; x++)
      {
            delete [] allAliases[x].name;
      }
}
             
int & assoc::operator[] (char * name)
{
    int buffer = getAlias(name);  // get the index
    if (buffer == -1)  // if alias does not exist
    {
     for (int x = 0; x < 100; x++)
     {
          if (strlen(allAliases[x].name) == 0)
          {
                   allAliases[x].name = new char[strlen(name) + 1];
             strcpy(allAliases[x].name, name);
             buffer = x;
             break; // VERY important!!!
          }
     }
    }
    return( allAliases[buffer].associatedNumber ) ;
}





int assoc::getAlias(char * name)
{
    for (int x = 0; x < 100; x++)
    {
          if (strcmp(allAliases[x].name, name) == 0)
          {
               // Found alias and return associated number

                  return x;
          }
    }
    // Not found
    return -1;
}


void main()
{
      assoc B;      // alloc 20 elements
      B["element1"]=111;

       cout << "Element \"element1\" is " << B["element1"] << endl;
      B["element2"]=113;
      B["element1"]=114;
      int k = B["element1"];
      cout << "Element \"element1\" is now " <<  k << endl;
}
0
 

Author Comment

by:anushan
Comment Utility
And one more question??..In the help FAQ's, it says that I can split the points for the experts with the help of a button that is located at the bottom of the question ...I dont' see it..where can I find it??
0
 
LVL 11

Accepted Solution

by:
bcladd earned 100 total points
Comment Utility
In the constructor: Rather than
    allAliases[x].name = new char;

use

  allAliases[x].name = new char[1];
    allAliases[x].name[0] = '\0';

Why: So delete[] works right with all of the names and so strcmp works (note that you have no way of knowing the value of the characters returned from new so things could be bad for strcmp).

Also, in operator[], right before you assign a new value to allAliases[x].name release the old memory:

  delete[] allAliases[x].name;
  allAliases[x].name = new char[strlen(name)+1];

avoids a memory leak.

-bcl
0
 
LVL 2

Assisted Solution

by:colmcc
colmcc earned 100 total points
Comment Utility
Hi anushan,

That is looking pretty good now, but I can see 1 or 2 remaining issues.

In the constructor you have the loop -

for (int x=0; x < 100 ; x++)
     {
          allAliases[x].name = new char;
     }

This allocates one byte of dynamic memory for each alias.  But why? It never gets used; and it never gets deleted.  Later in operator[](), you have -

allAliases[x].name = new char[strlen(name) + 1];

This allocates the memory which you actually use (and later delete in the destructor).

So, I think your constructor loop should just set each of these name pointers to NULL.  You must not pass a NULL pointer to strlen(), so where you currently check for an unused entry in allAliases using -

if (strlen(allAliases[x].name) == 0)
 
this can become

if (allAliases[x].name == NULL)

-Colin
0
 

Author Comment

by:anushan
Comment Utility
Thanks for all your help..I really did learn something here :)..Next time I will make sure I start from scratch...I do agree that it is easier to work with your own code than to debug somebody else's :) thanks again for all your advices ....I'm closing this question now...
0

Featured Post

Better Security Awareness With Threat Intelligence

See how one of the leading financial services organizations uses Recorded Future as part of a holistic threat intelligence program to promote security awareness and proactively and efficiently identify threats.

Join & Write a Comment

What is C++ STL?: STL stands for Standard Template Library and is a part of standard C++ libraries. It contains many useful data structures (containers) and algorithms, which can spare you a lot of the time. Today we will look at the STL Vector. …
How to install Selenium IDE and loops for quick automated testing. Get Selenium IDE from http://seleniumhq.org (http://seleniumhq.org) Go to that link and select download selenium in the right hand columnThat will then direct you to their downlo…
The viewer will learn how to use the return statement in functions in C++. The video will also teach the user how to pass data to a function and have the function return data back for further processing.
The viewer will be introduced to the technique of using vectors in C++. The video will cover how to define a vector, store values in the vector and retrieve data from the values stored in the vector.

762 members asked questions and received personalized solutions in the past 7 days.

Join the community of 500,000 technology professionals and ask your questions.

Join & Ask a Question

Need Help in Real-Time?

Connect with top rated Experts

11 Experts available now in Live!

Get 1:1 Help Now