ucam-department-of-psychiatry / camcops

Cambridge Cognitive and Psychiatric Test Kit (CamCOPS)
Other
12 stars 8 forks source link

Adult Autism Spectrum Quotient (AQ) task #361

Closed martinburchell closed 2 weeks ago

martinburchell commented 2 weeks ago

@RudolfCardinal which menu(s) do you think this task fits under?

Checklist for new tasks:

Please [X] all the boxes above that apply

RudolfCardinal commented 2 weeks ago

Looks good -- am almost there but not quite. In passing, Qt is these days dumping the moc_*.cpp files and the final binary (plus two other autogenerated C++ files) into my source directory. I see there is a MOC_DIR setting; e.g.

... what's best practice here (given the potential variety of build setups)? Do you have the same problem? Should I be setting an environment variable, or should we encode this into the .pro file?

RudolfCardinal commented 2 weeks ago

Changes so far:

martinburchell commented 2 weeks ago

Looks good -- am almost there but not quite. In passing, Qt is these days dumping the moc_*.cpp files and the final binary (plus two other autogenerated C++ files) into my source directory. I see there is a MOC_DIR setting; e.g.

* https://doc.qt.io/qt-5/qmake-variable-reference.html#moc-dir

* https://stackoverflow.com/questions/6181142

... what's best practice here (given the potential variety of build setups)? Do you have the same problem? Should I be setting an environment variable, or should we encode this into the .pro file?

I've seen some inconsistent behaviour here. Sometimes Qt Creator wants to put the build in a 'build' directory. Sometimes it works as it always has done. Changing the build directory can leave things in a right mess. The 'shadow build' checkbox is meant to separate the source from the binaries.

RudolfCardinal commented 2 weeks ago

Am doing final testing -- having messed up icon transparency and sorted it again... I'll add question numbers as prefixes to match the original. I have also learned std::iota! But is this safe? e.g. const QString& fieldname = Q_PREFIX + QString::number(qnum); -- is the reference not to something transient? I've amended as I was changing this anyway, but if it's safe, I should probably understand this better.

martinburchell commented 2 weeks ago

Am doing final testing -- having messed up icon transparency and sorted it again... I'll add question numbers as prefixes to match the original. I have also learned std::iota! But is this safe? e.g. const QString& fieldname = Q_PREFIX + QString::number(qnum); -- is the reference not to something transient? I've amended as I was changing this anyway, but if it's safe, I should probably understand this better.

I don't know if it's safe! Valgrind didn't complain when I ran it just now. std::iota is meant to fill the QVector with 1,2,3...50. I've not used it before.

RudolfCardinal commented 2 weeks ago

OK! If it was unsafe, I think it would be akin to a "dangling pointer" rather than a memory leak. But I was wrong, as follows: it is close to the C++11 feature of rvalue references (e.g. https://www.cprogramming.com/c++11/rvalue-references-and-move-semantics-in-c++11.html), but more specifically it is the longer-standing concept that a const reference to a temporary rvalue prevents the temporary object from being deleted ("const lvalue references") (e.g. https://pvs-studio.com/en/blog/posts/cpp/1006/). So I was being unnecessarily worried. Sorry! (Separately, there was a memory leak via new NameValueOptions() -- I've put that in a QSharedPointer.)

RudolfCardinal commented 2 weeks ago

And thanks re build options! That seems to have fixed my local mess.