[SOLVED] How to implement a QList of references.

Chris Kawa

bq. The point of using this is the same point as using QList<T*>, except we are abstracting pointers to references

It's like saying I'm abstracting apples with oranges. This is not abstraction. This is using one mechanism instead of another, but wrong ;)

bq. The only way I could figure out how to do the iterator implementation was to make it so no duplicate pointers could be stored in the list.

You're mixing values with iterators pointing to these values. The T* pointer is a value in this case and you don't touch it at all. A QList is perfectly able to store multiple identical pointers. Just try QList<int*>() << nullptr << nullptr << nullptr; Works as expected - 3 values; An iterator is another, unrelated pointer. In this case you could implement it simply as a index (int) into the list. QList implementation is not important here but it's a sort of amortized linked list - where it doesn't allocate one element at a time but some chunks.

@
if(this != &other) {
clear();
for(T &item : other) append(item);
other.clear();
}
@
That is copying and the slowest possible one. One element at a time.

No, begin and cbegin are not the same thing. One is const, another is not. Just because they happen to seem kinda similar thing in this case doesn't change it. As I said earlier - const is not just a problematic keyword to shuffle away here and there. It's a very important mechanism.

bq. It works exactly the same as QList::takeAt()

No, it doesn't. You see this is the big problem with this reference list type (which really is a PointerWithoutStarList). It hides what it's really doing which will lead to errors. Example:
@
//QList
int foo = 42;
QList<int*> list;
list << &foo; //I'm storing an address of a local variable, I see what I'm doing
int* bar = list.takeFirst();

*bar = 43; //OK, no problem, foo is now 43

//Your implementation
int foo = 42;
ReferenceList<int> list; //it stores *int, not int, how am I to tell that?
list << foo; //is it a copy? a move? no, it's tking an address of a local variable, how would I tell?
int bar = list.takeFirst(); //bar is an int, a copy of what element 0 pointed at
bar = 43; //Does nothing to foo, they are unrelated
@
So you see the way this works you could have a list of copies (QList<int>) just as well, because you don't grant any way to get a reference to foo back from the container.

The reason why C++ doesn't have containers of references is that they would have many pitfalls. For example ownership and object lifetimes wouldn't be clear(as in your implementation). I really can't see any benefit to such container. If you just don't want to use stars then instead of creating QList<Type*>, make a QList<Type> and make Type movable(in the Qt way). That will be much cleaner, shorter and, what's really important, cache friendly as the values will occupy adjacent memory. Your implementation tries to hide the fact that there are pointers in the language and that's just not possible. There are pointers in c++. Many of them in fact and they are there for a reason. Deal with it ;)

I don't want to sound grumpy. It's great that you're experimenting like that. It's just a bad topic ;)

Tory Gaurnier

[quote author="Chris Kawa" date="1413108905"]bq. The point of using this is the same point as using QList<T*>, except we are abstracting pointers to references

It's like saying I'm abstracting apples with oranges. This is not abstraction. This is using one mechanism instead of another, but wrong ;)[/quote]

I disagree on this point, it's not like comparing apples to oranges, but more like comparing an orange to an orange without it's peel. From what I've learned a reference basically IS a pointer, just abstracting away the pointer mechanics to make it more safe, which is exactly what I'm attempting to do (but I know my implementation has lots of problems, I'm working on fixing it).

[quote author="Chris Kawa" date="1413108905"]bq. The only way I could figure out how to do the iterator implementation was to make it so no duplicate pointers could be stored in the list.

You're mixing values with iterators pointing to these values. The T* pointer is a value in this case and you don't touch it at all. A QList is perfectly able to store multiple identical pointers. Just try QList<int*>() << nullptr << nullptr << nullptr; Works as expected - 3 values; An iterator is another, unrelated pointer. In this case you could implement it simply as a index (int) into the list. QList implementation is not important here but it's a sort of amortized linked list - where it doesn't allocate one element at a time but some chunks.

@
if(this != &other) {
clear();
for(T &item : other) append(item);
other.clear();
}
@
That is copying and the slowest possible one. One element at a time. [/quote]

Well, I know my iterator implementation was all jacked up, I realized there's a much better way of dealing with iterator, my iterator can just inherit from QList<T*>::iterator then the only operators I need to reimplement are *, ->, and [], so they will return expected values. I already tested it and it works quite well.

[quote author="Chris Kawa" date="1413108905"]No, begin and cbegin are not the same thing. One is const, another is not. Just because they happen to seem kinda similar thing in this case doesn't change it. As I said earlier - const is not just a problematic keyword to shuffle away here and there. It's a very important mechanism.[/quote]

Not to sound argumentative, but did you notice begin has a const overloaded version? So yeah, 'begin() const' and 'begin()' are two different methods, with that in mind, 'begin() const' and 'cbegin() const' are basically the same thing right? If they are different how so? Anyways, it's not as important because I redid my whole implementation of iterators, so I'm not doing that anymore anyways.

[quote author="Chris Kawa" date="1413108905"]bq. It works exactly the same as QList::takeAt()

No, it doesn't. You see this is the big problem with this reference list type (which really is a PointerWithoutStarList). It hides what it's really doing which will lead to errors. [/quote]

Once again, I don't see how what you are describing is any different from a reference? A reference is supposed to "hide" what it's really doing, and dereference itself "hiding" the star.

[quote author="Chris Kawa" date="1413108905"]Example:
@
//QList
int foo = 42;
QList<int*> list;
list << &foo; //I'm storing an address of a local variable, I see what I'm doing
int* bar = list.takeFirst();

*bar = 43; //OK, no problem, foo is now 43

//Your implementation
int foo = 42;
ReferenceList<int> list; //it stores *int, not int, how am I to tell that?
list << foo; //is it a copy? a move? no, it's tking an address of a local variable, how would I tell?
int bar = list.takeFirst(); //bar is an int, a copy of what element 0 pointed at
bar = 43; //Does nothing to foo, they are unrelated
@
So you see the way this works you could have a list of copies (QList<int>) just as well, because you don't grant any way to get a reference to foo back from the container.

The reason why C++ doesn't have containers of references is that they would have many pitfalls. For example ownership and object lifetimes wouldn't be clear(as in your implementation). I really can't see any benefit to such container. If you just don't want to use stars then instead of creating QList<Type*>, make a QList<Type> and make Type movable(in the Qt way). That will be much cleaner, shorter and, what's really important, cache friendly as the values will occupy adjacent memory. Your implementation tries to hide the fact that there are pointers in the language and that's just not possible. There are pointers in c++. Many of them in fact and they are there for a reason. Deal with it ;)[/quote]

Honestly I think you might be overthinking it a bit, remember that the idea of this is that it's a "list of references", and you'd use it just like you'd use references, in the examples you are showing that would obviously be bad usage of references. Let me give an example of what I'm using it for and maybe it'll clear things up.

I'm working on a library program, I have a library class as such
@class Library : public QList<MetadataObject> {
...
}@

So it's a qlist that stores metadata objects. Now I have a dialog to edit the metadata, and what I'm doing is in the dialog I basically call Library::getReferenceListToSelectedMetadataObjects(), which returns one of my fancy ReferenceLists (that you hate so much :P), and when the info is edited in the dialog it's automatically edited in the library, because each metadata object in the list is a reference, and it works perfect for this.

Now remember, you can't say someone's not going to know what it's doing unless they don't know how references work. When you assign something to a reference, like this:
@Type &ref = other_var;@
You are not passing the address from other_var, the address is implicitly passed because you are using a reference, this is exactly what my ReferenceList is doing. Of course, like I stated earlier, I know I have problems in my implementation and I'm working on fixing them.

Tory Gaurnier

Also, don't think I have anything against pointers, I actually really like pointers and what you can do with them, that's why I prefer C/C++ over other languages, but they created references for a reason, and there are some times where you prefer to use references over pointers.

Chris Kawa

bq. that you hate so much :P

Don't need to get defensive. I don't like most of the code out there and probably you don't either. You'll just continue to use what you like and I will do the same. No hard feelings ;)

My main complaint is that (when you fix the details) in many cases it's not obvious what your type or its methods do. Even in your own last example there's a & there to tell you that addresses are being passed around. In your implementation there's no telling what a method does without looking at the implementation. Imagine you had to do that with Qt. It would be unusable.

I don't think I overthink this. It's called (it's not actually) a reference list. From every list type I know I expect to get back exactly what I put in. And yes, if I put a value type I expect to get a copy (it's called value semantics). In your case I put a reference (or so you tell me, but you do something else actually) and I get a non-reference copy back, which is totally unexpected. I would be somewhat (with a wry face) ok if it at least returned a reference to the original object i.e. T& from methods like takeAt() or value(). I mean it's called value() but does copyOf(). It's called takeAt() and does copyAndForget(). It's called append() and does appendUnique() (ok, you say you fixed that one).

Apart from doing something different than it is named for, have you considered that this will not work if a type is not copyable eg. QObject?

As for the const iterators (I know you say you fixed that) - I'd suggest to look into why there is const overload for begin().
Consider this:
@
auto foo = list.begin();
auto bar = list.cbegin();
const auto baz = list.begin();
const auto bzz = list.cbegin();
@
What are the types of these variables?

EDIT:
Following your last example:
@
int& foo = referenceList.value(idx);
//value() returns a copy so you're taking an address of temporary variable, which is an error
//not much of a reference list is it?
@

Tory Gaurnier

[quote author="Chris Kawa" date="1413147863"]My main complaint is that (when you fix the details) in many cases it's not obvious what your type or its methods do. Even in your own last example there's a & there to tell you that addresses are being passed around. In your implementation there's no telling what a method does without looking at the implementation. Imagine you had to do that with Qt. It would be unusable.[/quote]

I think we can agree to disagree on this point, I think it's pretty clear that you're dealing with references since it's called ReferenceList and the class comment says it stores pointers internally and returns references.

[quote author="Chris Kawa" date="1413147863"]
From every list type I know I expect to get back exactly what I put in. And yes, if I put a value type I expect to get a copy (it's called value semantics). In your case I put a reference (or so you tell me, but you do something else actually) and I get a non-reference copy back, which is totally unexpected. I would be somewhat (with a wry face) ok if it at least returned a reference to the original object i.e. T& from methods like takeAt() or value(). I mean it's called value() but does copyOf(). It's called takeAt() and does copyAndForget(). It's called append() and does appendUnique() (ok, you say you fixed that one).[/quote]

I actually agree on this, I fixed it, and now no methods will return values, only references. Remember I wrote it up pretty quickly (in my free time between work and school), and most of the time I was writing it was 2AM+ ;)

[quote author="Chris Kawa" date="1413147863"]Apart from doing something different than it is named for, have you considered that this will not work if a type is not copyable eg. QObject?

As for the const iterators (I know you say you fixed that) - I'd suggest to look into why there is const overload for begin().
Consider this:
@
auto foo = list.begin();
auto bar = list.cbegin();
const auto baz = list.begin();
const auto bzz = list.cbegin();
@
What are the types of these variables?[/quote]

If I'm not mistaken foo will be iterator, bar will be const_iterator, baz and bzz will both be const_iterator.

[quote author="Chris Kawa" date="1413147863"]EDIT:
Following your last example:
@
int& foo = referenceList.value(idx);
//value() returns a copy so you're taking an address of temporary variable, which is an error
//not much of a reference list is it?
@
[/quote]

Yeah, as I stated above I did notice this issue and fixed it.

I should also note that you were right about the move constructor, I am very new to move semantics and lvalue vs rvalue, I didn't realize you had to wrap a variable in std::move() for it to pass an rvalue (I'm not mistaken on this, am I?).

Thanks for all the feedback! You're feedback has forced me to review several things I've missed.

Tory Gaurnier