U++ framework: U++ Core » why not "T & Add(const T & x)" in all containers

Home » U++ Library support » U++ Core » why not "T & Add(const T & x)" in all containers

Show: Today's Messages :: Polls :: Message Navigator
E-mail to friend

why not "T & Add(const T & x)" in all containers [message #27700]

Thu, 29 July 2010 22:17

kohait00
Messages: 939
Registered: July 2009
Location: Germany

Experienced Contributor

just a short question:

i noticed that all containers dont return a ref on the item created inside a container, when a copy Add is used: (here Array is example)

	T&       Add();
	void     Add(const T& x); //why void?
	void     AddPick(pick_ T& x); //why void?
	T&       Add(T *newt);

so why not having

	T&       Add();
	T&       Add(const T& x);
	T&       AddPick(pick_ T& x);
	T&       Add(T *newt);

the added elements in any case end up in the container, so their ref could be returned...

Report message to a moderator

Re: why not "T & Add(const T & x)" in all containers [message #27779 is a reply to message #27700]

Tue, 03 August 2010 11:15

kohait00
Messages: 939
Registered: July 2009
Location: Germany

Experienced Contributor

any comments on this?

Report message to a moderator

Re: why not "T & Add(const T & x)" in all containers [message #27787 is a reply to message #27779]

Tue, 03 August 2010 12:53

andrei_natanael
Messages: 262
Registered: January 2009

Experienced Member

Hi,

That would be possible but we will lose some performance for nothing(or less). Taking the following code from Array implementation:

	void     Add(const T& x)            { vector.Add(DeepCopyNew(x)); }
	void     AddPick(pick_ T& x)        { vector.Add(new T(x)); }

to return the reference to added element, we have to find the element in vector, and that will take some time.

Andrei

Report message to a moderator

Re: why not "T & Add(const T & x)" in all containers [message #27793 is a reply to message #27787]

Tue, 03 August 2010 16:06

kohait00
Messages: 939
Registered: July 2009
Location: Germany

Experienced Contributor

why then, not do the same as in

	T&       Add()                      { T *q = new T; vector.Add(q); return *q; }
//like this
	void     Add(const T& x)            { T *q = DeepCopyNew(x); vector.Add(q); return *q; }
	void     AddPick(pick_ T& x)        { T *q = new T(x); vector.Add(q); return *q; }

the reference does not change, it's stored on heap, there should be no performance hit on that i think.

Report message to a moderator

Re: why not "T & Add(const T & x)" in all containers [message #27798 is a reply to message #27793]

Tue, 03 August 2010 22:44

andrei_natanael
Messages: 262
Registered: January 2009

Experienced Member

That seems ok.

Report message to a moderator

Re: why not "T & Add(const T & x)" in all containers [message #27801 is a reply to message #27798]

Wed, 04 August 2010 09:57

kohait00
Messages: 939
Registered: July 2009
Location: Germany

Experienced Contributor

i changed things for Add semantic, take a look at it. it was no much work. maybe mirek has had something special in mind with void return.. i'll ask him.

another thing to be changed is maybe the void Set(int i, const T& x, int count = 1); behaviour. which also could be split up just like Insert..

EDIT: no one has downloaded previous version, so i just change it for one, where Insert and Set behaviour is already changed accordingly. plrese review it. maybe it can go upstream if nothing stands in way.

Attachment: Core.rar
(Size: 12.62KB, Downloaded 334 times)

[Updated on: Wed, 04 August 2010 11:23]

Report message to a moderator

Re: why not "T & Add(const T & x)" in all containers [message #27810 is a reply to message #27801]

Thu, 05 August 2010 08:35

kohait00
Messages: 939
Registered: July 2009
Location: Germany

Experienced Contributor

any comments on that one? (need it for my app Smile

, like always...)

Report message to a moderator

Re: why not "T & Add(const T & x)" in all containers [message #27812 is a reply to message #27810]

Thu, 05 August 2010 09:55

mrjt
Messages: 705
Registered: March 2007
Location: London

Contributor

I'd like to hear Mirek's reasons for doing it the old way because there is usually a good reason, especially for the NTL stuff.

Report message to a moderator

Re: why not "T & Add(const T & x)" in all containers [message #27814 is a reply to message #27812]

Thu, 05 August 2010 10:10

kohait00
Messages: 939
Registered: July 2009
Location: Germany

Experienced Contributor

ofcorse. i also was trying to imagine what problems there might be to have prevented this stuff to already be there. nevertheless wanted to provide already a solution to think visually about Smile

Report message to a moderator

Re: why not "T & Add(const T & x)" in all containers [message #27844 is a reply to message #27700]

Fri, 06 August 2010 11:27

mirek
Messages: 14291
Registered: November 2005

Ultimate Member

kohait00 wrote on Thu, 29 July 2010 16:17

just a short question:

i noticed that all containers dont return a ref on the item created inside a container, when a copy Add is used: (here Array is example)

	T&       Add();
	void     Add(const T& x); //why void?
	void     AddPick(pick_ T& x); //why void?
	T&       Add(T *newt);

Mostly because of standard usage pattern...

It might be a little bit confusing as those variants that are taking parameter make a copy of this parameter (and would return a reference to this copy).

Also note the existence of Top() - only one more line...

But I am not strongly opposed to changing this either....

[Updated on: Fri, 06 August 2010 11:30]

Report message to a moderator

Re: why not "T & Add(const T & x)" in all containers [message #27847 is a reply to message #27844]

Fri, 06 August 2010 11:48

kohait00
Messages: 939
Registered: July 2009
Location: Germany

Experienced Contributor

the usage pattern is not changed too much in fact. all the functions changed (in code provided) returned void by before. so it's more of an extension..

the 'new' usage pattern would be, get direct access to the object, that 'somehow' has been added / replaced (Set) / or inserted. because it exists in container after the call, beeing it the same or a copy, the user wouldnt need to care. it is the one that remains in the container (maybe could even be more logic).

this would spare the usage of Top() or even operator[](GetCount()-1) like in some places, after having void Add(const & T x). this by the way uses the rather 'implicit' assumption that an added item is always placed last...

Report message to a moderator

Re: why not "T & Add(const T & x)" in all containers [message #27878 is a reply to message #27847]

Mon, 09 August 2010 08:50

kohait00
Messages: 939
Registered: July 2009
Location: Germany

Experienced Contributor

to which conclusion have you guys come? should it be extended?

Report message to a moderator

Re: why not "T & Add(const T & x)" in all containers [message #27998 is a reply to message #27878]

Fri, 13 August 2010 09:38

mirek
Messages: 14291
Registered: November 2005

Ultimate Member

kohait00 wrote on Mon, 09 August 2010 02:50

to which conclusion have you guys come? should it be extended?

I am barely positive about this, but before I waste time fixing documentation, I still cannot imagine usage scenario which I would like - I think that you either use Vector to store "values" and then you do not need those references as return values, or to create objects inside.

I mean, just tell me little where it does help Smile

Practical example is most welcome.

Report message to a moderator

Re: why not "T & Add(const T & x)" in all containers [message #28101 is a reply to message #27998]

Mon, 16 August 2010 09:06

kohait00
Messages: 939
Registered: July 2009
Location: Germany

Experienced Contributor

it's actually the same behaviour as with T& Array::Attach(T* newt);
and merly a logical unification of interface, that anything that ends beeing an object, no matter newly added or as copy, should be instantly available, without the need to again access the container to get the same. here, it actually doesnt matter the container type, it's same situation for Vector and Array.

a practical use is this:

crating new container objects, based on some 'template' objects, and remodifying stuff that is actually different, on the new created object, pushing it somewhere to do something. this would use in case of Array:
void Array::Add(cibst T&),
then
T& Array::operator[](int i) with Array::GetCount()-1.
actually 3 invokations, that could be done in one.

it's maybe more of estetic use Smile

but could again add to Ultimate's short and reading friendly code

Report message to a moderator

Re: why not "T & Add(const T & x)" in all containers [message #28347 is a reply to message #28101]

Mon, 30 August 2010 10:46

kohait00
Messages: 939
Registered: July 2009
Location: Germany

Experienced Contributor

hey mirek, what is your final decision on this one?
sorry if disturbing in vacations Smile

Report message to a moderator

Re: why not "T & Add(const T & x)" in all containers [message #28533 is a reply to message #28101]

Mon, 06 September 2010 10:53

mirek
Messages: 14291
Registered: November 2005

Ultimate Member

kohait00 wrote on Mon, 16 August 2010 03:06

then
T& Array::operator[](int i) with Array::GetCount()-1.
actually 3 invokations, that could be done in one.

Could be done with Top().

I am likely to make this change, but I am still waiting for 3rd party opinion...

Report message to a moderator

Re: why not "T & Add(const T & x)" in all containers [message #28536 is a reply to message #28533]

Mon, 06 September 2010 11:52

kohait00
Messages: 939
Registered: July 2009
Location: Germany

Experienced Contributor

who is 3rd party? Cool

Report message to a moderator

Re: why not "T & Add(const T & x)" in all containers [message #28588 is a reply to message #28536]

Wed, 08 September 2010 09:00

mirek
Messages: 14291
Registered: November 2005

Ultimate Member

Not me and not you Smile

(That makes it 3rd, right?)

Report message to a moderator

Re: why not "T & Add(const T & x)" in all containers [message #28605 is a reply to message #28588]

Wed, 08 September 2010 10:24

rylek
Messages: 79
Registered: November 2005

Member

Hi there!

It is a pleasure to play the third party, though the semantics of the term are highly disputable here. I think it's possible Mirek originally didn't include the return reference in Vector::Add to emphasize the fact that such references into Vectors are volatile in principle (specifically they are periodically invalidated by the Add function itself while reallocating the physical Vector data). But, of course, the same argument holds for Add() which does return the reference. Moreover, Array::Add(T *newobj) also returns the reference, albeit for different reasons.

U++ also decidedly avoids returning references in pick assignment operators, which is natural because a "chain" assignment (a = b = c) in such cases exhibits undesirable behaviour (by destroying b). But Vector::Add(const T&) doesn't have this problem; the only thing that has to be avoided is rather artificial constructs of the form

vector.Add(vector.Add(obj))

exactly because of the periodical Vector reference invalidation. But then again you can run into exactly the same problems by writing, e.g.

vector.Add(vector[5]);

so that this is no specific of Vector::Add(const T&) either. To sum it all up, I currently see no practical reasons against modifying Vector::Add to include the return reference. I would rather say that it's like updating old code to match interface standards adopted / developed later on, in fact I believe Vector is one of the very oldest things in U++ (although it's been rewritten quite a few times since its inception).

Regards

Tomas

Report message to a moderator

Re: why not "T & Add(const T & x)" in all containers [message #28611 is a reply to message #28605]

Wed, 08 September 2010 10:54

kohait00
Messages: 939
Registered: July 2009
Location: Germany

Experienced Contributor

thanks, nice outline

Report message to a moderator

Re: why not "T & Add(const T & x)" in all containers [message #28619 is a reply to message #28605]

Wed, 08 September 2010 11:30

mirek
Messages: 14291
Registered: November 2005

Ultimate Member

rylek wrote on Wed, 08 September 2010 04:24

exactly because of the periodical Vector reference invalidation. But then again you can run into exactly the same problems by writing, e.g.

vector.Add(vector[5]);

Actually, above code is OK, this was already improved (because improvement is possible by changing the order of operations in implementation).

The only last one in this zone is Insert... (which right now ASSERTs, but I guess it should be fixed too).

Mirek

Report message to a moderator

Re: why not "T & Add(const T & x)" in all containers [message #28792 is a reply to message #28619]

Fri, 17 September 2010 08:11

mirek
Messages: 14291
Registered: November 2005

Ultimate Member

OK, it now returns T&...

Report message to a moderator

Re: why not "T & Add(const T & x)" in all containers [message #28794 is a reply to message #28792]

Fri, 17 September 2010 09:15

kohait00
Messages: 939
Registered: July 2009
Location: Germany

Experienced Contributor

hi mirek, thanks.
just a question aside: why does DeepCopyConstryct return T& instead of T*? it kinda differs from the other 'patterns' DeepCopyNew, ::new(p) T() etc..which all return T* at that level.

EDIT: i think i understand why..there occure strange compile errors for ArrayCtrl Vector< Vector< Value > > things. and i kind of cant figure out why and how to potentially fix it.

nonetheless, Add is done, what about Insert and Set behaviour? should do the same..

i've added the current proposal, based on your changes.

Attachment: Core.rar
(Size: 310.15KB, Downloaded 274 times)

[Updated on: Fri, 17 September 2010 10:33]

Report message to a moderator

Re: why not "T & Add(const T & x)" in all containers [message #29447 is a reply to message #28794]

Thu, 21 October 2010 09:02

kohait00
Messages: 939
Registered: July 2009
Location: Germany

Experienced Contributor

kohait00 wrote on Fri, 17 September 2010 09:15

hi what about Insert and Set behaviour? should do the same..

just a reminder, since i've still got the changes around in my code, just to know if they once might become upstream. or what your position is..

Report message to a moderator

Re: why not "T & Add(const T & x)" in all containers [message #29449 is a reply to message #29447]

Thu, 21 October 2010 11:10

mirek
Messages: 14291
Registered: November 2005

Ultimate Member

kohait00 wrote on Thu, 21 October 2010 03:02

kohait00 wrote on Fri, 17 September 2010 09:15

hi what about Insert and Set behaviour? should do the same..

just a reminder, since i've still got the changes around in my code, just to know if they once might become upstream. or what your position is..

Insert/Set has sort of problem as it can insert more than single element, and even worse, it can insert ZERO elements. (Note that for single element it already returns T&).

Report message to a moderator

Re: why not "T & Add(const T & x)" in all containers [message #29450 is a reply to message #29449]

Thu, 21 October 2010 11:54

kohait00
Messages: 939
Registered: July 2009
Location: Germany

Experienced Contributor

i thought of it. and there is a solution in the code.
i just split the function, i.e. Set:

original:

	void     Set(int i, const T& x, int count = 1);

split:

	T&       Set(int i, const T& x);
	void     Set(int i, const T& x, int count);

which is syntactically the same, the omited count defaults to setting only one element and returns the ref..no changes in user code. when using count, the other function evaluates.

insert is same:

	void     Insert(int i, const T& x, int count = 1);

split:

	T&       Insert(int i, const T& x);
	void     Insert(int i, const T& x, int count);

i'm actually working with the code for some while now, and haven't noticed any misbehavior, so the sub layers of upp deal well with it. consider it again. i think it can contribute to the comfort using upp containers, and of corse again a little bit of speed optimization at user level..and shorten the code a bit.

attached the current source based snapshot..

Attachment: containers_Core.rar
(Size: 12.56KB, Downloaded 285 times)

Report message to a moderator

Re: why not "T & Add(const T & x)" in all containers [message #31830 is a reply to message #29450]

Wed, 30 March 2011 10:54

kohait00
Messages: 939
Registered: July 2009
Location: Germany

Experienced Contributor

just wanted to poke gently w.r.t that issue..
i've got some fixes in my local tree and am cleaning up Smile

is there interest/chance for this to be upstream sometime or should i consider it lost? (future coding needs to take account of it)

Report message to a moderator

Re: why not "T & Add(const T & x)" in all containers [message #32000 is a reply to message #31830]

Sat, 16 April 2011 20:31

mirek
Messages: 14291
Registered: November 2005

Ultimate Member

OK. I really dislike this change, but I cannot bring any rational argument against it.

So it is now in Core Smile

Report message to a moderator

Re: why not "T & Add(const T & x)" in all containers [message #32018 is a reply to message #32000]

Sun, 17 April 2011 14:46

kohait00
Messages: 939
Registered: July 2009
Location: Germany

Experienced Contributor

cool..

just for completion sake..there are those changes for Map and Index, that can expose their elements now as well, just by using the api..was it your intention?

for easy handling, i just packed them, so you can replace them for svn review. to current revision 3346

there is also the Insert(int i, T* newt) that seems to be overseen.
thats why Vcont.h and .hpp are in the rar as well.

it'd be nice to have all together

thanks for your time Smile