Trouble assigning value to boolean in the struct

[Christian Ehrlicher]

@artwaw said in Trouble assigning value to boolean in the struct:

Now, this one works:

It may compile but it for sure does not work as expected: https://doc.qt.io/qt-5/qvector.html#value

[artwaw]

@Christian-Ehrlicher Range of the index is strictly controlled, simply can't be out of bounds. I took care of that.
My problem is that it does not compile.

[hskoglund]

Hi, perhaps value() is read-only, what happens if you try:

current.fields[ui->editSectionNum->currentIndex()].permanent=(state>0);

[aha_1980]

@hskoglund

Hi, perhaps value() is read-only

Not perhaps, it is const.

That's what Christian wanted to say with his link.

Regards

[Christian Ehrlicher]

@hskoglund said in Trouble assigning value to boolean in the struct:

perhaps value() is read-only

It's even worse - it returns a copy.

[JonB]

@artwaw said in Trouble assigning value to boolean in the struct:
If const is the cause of the error (and I don't doubt it is) for

current.fields.value(ui->editSectionNum->currentIndex()).permanent=(state>0);

why is there no error for

current.fields.value(ui->editSectionNum->currentIndex()).pattern=ui->sectionEdit->text();

? That was the OP's query. That's what I don't see, it should produce the same error.

[hskoglund]

I suspect it's because the const is only 1 layer deep, i.e. it protects integers and booleans in that struct from being reassigned, but since QStrings are pointers, that const only protects reassignment of the pointer, not what the pointer points to, i.e. QStrings data().

[Christian Ehrlicher]

@JonB said in Trouble assigning value to boolean in the struct:

it should produce the same error.

In theory it's a temporary so it should compile. In practice the compiler prevents you from shooting into your foot - at least when the left hand side is a POD. As soon as it's a class the compiler will not complain - blame the compiler :)

struct blub
{
    int a = 0;
    int b = 0;
    blub(int _a, int _b) : a(_a), b(_b) {}
    blub() = default;
};
struct foo
{
    std::string stdStr;
    QString str;
    QPointF p;
    blub myBlub;
    int myInt = -1;
    bool myBool = false;
};

QVector<foo> fooVec;

void doSomething()
{
    int i = 5;
    fooVec.value(i).stdStr = "str";
    fooVec.value(i).str = "str";
    fooVec.value(i).p = QPointF(1,2);
    fooVec.value(i).myBlub = blub(1,2);
    fooVec.value(i).myInt = 2;  // gives compiler error
    fooVec.value(i).myBool = true;  // dito
}

[Christian Ehrlicher]

@hskoglund said in Trouble assigning value to boolean in the struct:

it's because the const

QVector::value() does not return a const object!

[hskoglund]

@Christian-Ehrlicher Hmmm, but the docs says that API is const:

T QVector::value(int i) const

[kshegunov]

@Christian-Ehrlicher said in Trouble assigning value to boolean in the struct:

As soon as it's a class the compiler will not complain - blame the compiler

But don't rush to. The compiler has to generate the assignment for the trivial bool, hence it can see that the written is nonsense. On the other hand QString has both assignment operators defined already, so the compiler has to be really, really, really clever to understand that the written assignment is not what was intended. (for example the compiler has to verify that the destructor of mentioned temporary produces no side effects, which isn't trivial to begin with, to be able to then to infer that the assignment is wrong)

[kshegunov]

@hskoglund said in Trouble assigning value to boolean in the struct:

but the docs says that API is const

The point, Henry, is not that the method is non-mutating, which is without a doubt correct, but that it returns a copy of the element, which copy is then modified.

[hskoglund]

@kshegunov Aha, you're right! If the method had a const at the beginning things would be different :-)

[Christian Ehrlicher]

@kshegunov I thought this too, but even when the compiler generates all the operators (see my struct blub) it will not complain. So I think they simply did not implement it for classes.

[kshegunov]

@Christian-Ehrlicher said in Trouble assigning value to boolean in the struct:

@kshegunov I thought this too, but even when the compiler generates all the operators (see my struct blub) it will not complain. So I think they simply did not implement it for classes.

Yes probably, and probably because it's nontrivial in the general case. While at the same time for an intrinsic type it's plain and simple to deduce.

[Christian Ehrlicher]

@hskoglund said in Trouble assigning value to boolean in the struct:

had a const at the beginning

But then the copy elision would not work afair.

[hskoglund]

Thinking about this, if the intent is to modify the elements of the QVector, then using value() is a bad idea since you're working with a copy (even though it happens to work for QStrings in that struct).

But if the API was changed to have another const at the beginning, say:

const T QVector::value(int i) const

I'm too lazy to test it, but wouldn't that leading const cause compilation errors also for QStrings assignments? If so that would help dispel confusion about the usage of value().

[kshegunov]

@hskoglund said in Trouble assigning value to boolean in the struct:

I'm too lazy to test it, but wouldn't that leading const cause compilation errors also for QStrings assignments?

It would. I'm not sure if the compiler can RVO it correctly if you assign to a non-const however ... that needs actual looking at the asm output.

[Christian Ehrlicher]

From my pov QVector::value() should be deprecated - it's a really weird function which returns a copy just for the reason that the caller does not do the bounds checking.
I don't see a difference in the asm output in a single testcase.

[artwaw]

Thank you for the explanation - indeed, I saw in docs that value() is const but since it worked for the QString I just... assumed it works anyway. Silly of me.

So, to make it work as intended I should use [index] notation, like @hskoglund said (I am somehow opposed to using it in the favour of methods).

Since I already got everyones involved attention - the QVector (in this case) is supposed to hold less than 20 of items, set once and not being manipulated further. Is the choice of QVector over other container types a valid one? I read about container classes and differences between them and after that I rather tend to avoid QList.