pointers and copy on write

The copy on write has to be triggered when you access a non-const member of an instance that is currently a shallow copy.

In the context of your example b and c should be marked as shallow copies. Then when modifying the coeficient, the copy on write should be triggered.

>> In the context of your example b and c should be marked as shallow copies.

It's imo easier to mark all three as shallow copies, that way, a modification to one object doesn't have an impact on other objects.

If you create a new instance (not by copy or assignment) then there is no point in marking it as shallow. Actually that would be wrong, because there is no source for the shallow copy.

>> Actually that would be wrong, because there is no source for the shallow copy.

There is if you make one. Using a shared_ptr eg.

okay, thanks for suggestions

Can you post your code ?

here's my code so far, if you can comment on it, it would be great

>>>> const void PolyClass::initPolynomial(int index, int c)

What do you want achieve using "const void"?

>>>>   (pol_s->pol)[index] = c;

Before doing that the initPolynomial function should make sure that it has a refcount of 1 or make a copy.

Same applies for clearPoly.

>>>> if (pol_s->refCount > 1){
>>>>    std::map<int, int>::iterator iter;
>>>>    Poly new_map;
>>>>    for (iter = (this->pol_s)->pol.begin(); iter != (this->pol_s)->pol.end(); ++iter)
>>>>       new_map[iter->first] = iter->second;
>>>>    this->pol_s->refCount--;
>>>>    this->pol_s = new PolyStruct();
>>>>    this->pol_s->refCount = 1;
>>>>    this->pol_s->pol = new_map;
>>>>}

put the code above in a member function 'makeSafeCopy' and call it in all non-const member functions and operators before writing to the pol_s member.

>>>>    std::map<int, int>::iterator iter;
>>>>    Poly new_map;
>>>>    for (iter = (this->pol_s)->pol.begin(); iter != (this->pol_s)->pol.end(); ++iter)
>>>>       new_map[iter->first] = iter->second;
>>>>    this->pol_s->refCount--;
>>>>    this->pol_s = new PolyStruct();
>>>>    this->pol_s->refCount = temp;
>>>>    this->pol_s->pol = new_map;

Instead do:

  // decrement the current ref counter
  this->pol_s->refCount--;
  // create a new struct
  PolyStruct * pol_s_new = new PolyStruct();
  // set the counter to 1 (if the constructor didn't do that already)
  pol_s_new->refCount = 1;
  // make a copy of the old map
  pol_s_new->pol          = this->pol_s->pol;
  // Finally take the new pointer for pol_s  (the old pol_s was used in at least one other polynomial)
  this->pol_s = pol_s_new;

you mean after calling the makeSafeCopy, I need to do the following?

// decrement the current ref counter
  this->pol_s->refCount--;
  // create a new struct
  PolyStruct * pol_s_new = new PolyStruct();
  // set the counter to 1 (if the constructor didn't do that already)
  pol_s_new->refCount = 1;
  // make a copy of the old map
  pol_s_new->pol          = this->pol_s->pol;
  // Finally take the new pointer for pol_s  (the old pol_s was used in at least one other polynomial)
  this->pol_s = pol_s_new;

                                              

1:
2:
3:
4:
5:
6:
7:
8:
9:
10:

>>Before doing that the initPolynomial function should make sure that it has a refcount of 1 or make a copy.

what if has a refCount > 1? is that what you mean?

>>>> what if has a refCount > 1? is that what you mean?

Yes, but you also can put the if into the makeSafeCopy like

void Polynomial::makeSafeCopy()
{
   if (pol_s->refcount > 1)
   {
       // decrement the current ref counter
       this->pol_s->refCount--;
       // create a new struct
       PolyStruct * pol_s_new = new PolyStruct();
       // set the counter to 1 (if the constructor didn't do that already)
       pol_s_new->refCount = 1;
      // make a copy of the old map
      pol_s_new->pol          = this->pol_s->pol;
      // Finally take the new pointer for pol_s  (the old pol_s was used in at least one other polynomial)
      this->pol_s = pol_s_new;
   }
   // else do nothing
   
}

That would allow you to simply call makeSafeCopy(); in any of the writing functions and operators to assure you have a pol_s member with refcount == 1.

but even if I don't put that into a makeSafeCopy() method, it would still be fine right?

>>>> but even if I don't put that into a makeSafeCopy() method, it would still be fine right?

Only if you have

    if (this->pol_s->refCount > 1)  
        makeSafeCopy()

in each of those functions (where the if statement seems some unnecessary overhead for me).

so I do two comparsion in the code above, first is:

this->pol_s->refCount > 1 and next I do if pol_s->refcount > 1

this->pol_s->refCount > 1 is identical to pol_s->refcount > 1

The "this->" is redundant code though it may increase the readability.

If you put the if statement (with or without this->) into the makeSafeCopy you need not repeat it in each member function or operator.

>>>> this->pol_s->refCount > 1 is identical to pol_s->refcount > 1

Of course only if using "refCount"  with a capital C for both.

I don't see how that will make a difference in the code, as what you're doing is just moving the whole block of code into a method called makesafecopy and then call that everytime we need to do a shallow copy... I know this is better design and yes I do admit it.. but can you please the purpose other than that?

Here's my updated one, see if it suffices what you suggested me and I am getting what you explained correctly

>>>> I know this is better design and yes I do admit it.. but can you please the purpose other than that?
A better design should be enough reason ;-)

But any redundant code is error-prone and is contrary to object-oriented principles. Assume you want to make the reference counting thread-safe. Then, decrementing and incrementing the reference counter must be made an exclusive operation. Do you want to add this additional code to hundreds of statements in your implementation or only to a few statements?

Look at your implementation posted last. You added the code for checking the reference counter only in a few functions. The majority of operators and functions also changing the member data didn't have the needed code. You may ask yourself why you didn't add the code. Because you didn't know of that the code was needed there as well? Rarely. It is much more likely that you didn't want to copy the same codelines into ten or more functions as you thought that there might be a more elegant solution. You were right.

other than that it's all good?

and by the way changing it to:

make the unary operation not to work

iter->second = -iter->second;

                                              

1:

>>>> make the unary operation not to work
Really? Hmmm. The iter-> would/should operate on a reference to the pair stored in the map.

Alternatively try

   std::pair<int, int> & current = *iter;
   current.second = -current.second;

but actually it shouldn't make a difference.

As last resort do

   this->pol_s[iter->first] = -iter->second;

>>>> other than that it's all good?  

Don't know. I didn't have time to make a full check. But you should make some own unit tests so that *all* functions and operators were tested.

okay, will do... if you have time, do you mind looking at my code again

>>>> bool operator<(const PolyClass& p1, const PolyClass& p2){
>>>>  bool result = false;
>>>>  std::map<int, int>::reverse_iterator i = (p1.pol_s)->pol.rbegin();
>>>>  std::map<int, int>::reverse_iterator j = (p2.pol_s)->pol.rbegin();
>>>>  if (i->first < j->first)
>>>>       result = true;
>>>>  else
>>>>       result = false;
>>>> 
>>>>  return result;
>>>> }


The operators <, <= , >, >=  are wrong. You neither consider empty PolyClass objects nor polynomials where the 'first' coefficient is equal but higher coefficients are different.

bool operator<(const PolyClass& p1, const PolyClass& p2)
{
  // first check for different sizes
  // a *shorter* polynomial should be called the *smaller* one
  if (p1.size() != p2.size())
     return (p1.size() < p2.size());

 std::map<int, int>::reverse_iterator i = (p1.pol_s)->pol.rbegin();
 std::map<int, int>::reverse_iterator j = (p2.pol_s)->pol.rbegin();

 // as the sizes were equal we only need to check for i
 for (; i != p1.pol_s->pol.end(); ++i, ++j)
 {
     if (i->second!= j->second)
         return (i->second < j->second);
 }
 return false;  // all coefficients have been equal
}

Note, with operator== and operator< you can implement all the other comparision operators.

One more note: a for loop is better than a while loop as the incrementation is part of the for statement and it is much more difficult to have an infinite loop.

thanks for pointing that out, that seems to make more sense

>>>> // first check for different sizes
I reflected about that part and came to a different result. First, the size probably is not a function of PolyClass but of the map in the PolyStruct. Second, if the polynomials have different dimensions then the coefficient of the highest power compared to 0 would determine the comparision between the polynomials. So, we would have the following code:

bool operator<(const PolyClass& p1, const PolyClass& p2)
{
 std::map<int, int>::reverse_iterator i = (p1.pol_s)->pol.rbegin();
 std::map<int, int>::reverse_iterator j = (p2.pol_s)->pol.rbegin();
 
 // we need to check for i and j as any of them could end
 // a new cycle only will happen if the dimensions were equal
 for (;i != p1.pol_s->pol.end() || j != p2.pol_s->pol.end(); ++i, ++j) 
 {
     int d1 = -1;
     int d2 = -1;
     if (i != p1.pol_s->pol.end())
         d1 = i->first;
     if (j != p2.pol_s->pol.end())
         d2 = j->first;
     if (d1 < d2)
     {  // p2 is greater if the coefficient of the higher power >= 0
        return (j->second >= 0); 
     }
     else if (d2 > d1)
     {  // p1 is less if the coefficient of the higher power >= 0
        return (i->second >= 0); 
     }
     // now we have equal dimension and comapre the coefficients
     if (i->second!= j->second)
         return (i->second < j->second);
 }
 return false;  // all coefficients have been equal
}

                                              

1:
2:
3:
4:
5:
6:
7:
8:
9:
10:
11:
12:
13:
14:
15:
16:
17:
18:
19:
20:
21:
22:
23:
24:
25:
26:
27:
28:
29:

Silly me. I thought the question was :

>> After I do a[2] then b and c should also be 120x^2 + 10x + 90 right?

lol

Silly you  ;-)

The question was 'pointers and copy on write' and actually the

   a[2] = 120;

should NOT lead to b and c  is  "120x^2 + 10x + 90"  but they keep on "2x^2 + 10x + 90" what was their initial value.

Come on Alex, no word games ... The only question was the one that ended with a question mark. Everything else after that question was answered was follow-up discussion. What you quote is the question title, and maybe you're psychic, but I can't spot a question in there - just a description of the topic.


>> should NOT lead to b and c  is  "120x^2 + 10x + 90"  but they keep on "2x^2 + 10x + 90" what was their initial value.

Which is what I said :

>> as soon as you modify one of them, you'd have to implement your "copy-on-write" logic.

"one of them" includes 'a', so modifying 'a' implies that a "copy-on-write" has to happen.