[Solved] returning custom Object from function

maximus

Hi, I have a small problem understanding how to return an object from a function in C++,

I'm loosing some of the object attribute while returning it. In the code below, the function "parseSingleWorkoutXml(QString filePath)" return a custom object "Workout", "Workout" contain a QList<Interval> which is another custom object. For some reason, when I exit the function "parseSingleWorkoutXml", the QList<Interval> size is now 0 even if before the return it had element in it.

Both of my custom Object are declared with Q_DECLARE_METATYPE and are coded so that they can be used in QList (copy constructor, etc.) Any help appreciated! thanks

Main function :

@QList<Workout> XmlUtil::getLstWorkoutLocal() {

QStringList lstWorkoutPat = Util::getListWorkoutFiles();

QList<Workout> lstWorkout;
Workout workout;

foreach (QString filePath, lstWorkoutPat)
{
    workout = parseSingleWorkoutXml(filePath);
    if (workout.getId() != -1) {
        lstWorkout.append(workout);
    }
}
return lstWorkout;

}@

Helper function:

@Workout XmlUtil::parseSingleWorkoutXml(QString filePath) {

QList<Interval> lstIntervalSource;
//[... add element to lstIntervalSource, but after the return below, those element are no longer present?]
workout = Workout(666, lstIntervalSource, lstRepeat,
                  name, creator, description, plan, type );
return workout;

}@

SGaist

Hi,

Are you sure your workout object's id is not -1 ?

Jeroentjehome

Hi,
To copy an QObject derived class is "impossible". Think about it, are the children of the first object also copied, if so, who is the parent of those objects, They can't have two parents, now can they?
To do a copy by reference (return value for the getLstWorkoutLocal function) instead of by value for the QList and you should do fine.
Second, returning a function scope variable reference is not so great, so make the QList on the heap (using the new keyword).

maximus

Yes thanks SGaist i'm sure it's not -1, If I put a debug here I see the problem in action:
@//size Source inside function 2
//size Source after return from function 0 @

Jeroenje thanks for your answer, sorry it's a bit hard coming from Java trying to always think of the memory under the hood, I would be happy to code and not always have to think about it! [sad_face]
Joke aside, returning a QList<Workout> from "getLstWorkoutLocal" is not causing problem it seems, but you say it is bad practice to return a local variable, why is that exactly?

My main problem is from the function "parseSingleWorkoutXml", I will try to return a pointer to a Workout instead of a Workout and see. But since I have a QList<Workout> and not QList<Workout*> on the top level, I will have to do some pointer arithmetic to do I guess to change Workout* back to a Workout

If you can highlight the code you think is not so great in red I would be really glad, got dogecoin wallet? thank you!

SGaist

Just to be sure, is your Workout class deriving from QObject ?

maximus

Oh I forgot to mention, Workout and Interval are plain C++ object (not QObject)

Exemple (stripped down a bit)

@class Interval
{

public:

enum StepType
{
    NONE,
    FLAT,
    PROGRESSIVE,
};

Interval();
Interval(int id, QTime duration, QString displayMessage,
         StepType powerStepType, double targetFTP_start, double targetFTP_end, int targetFTP_range, double leftPowerTarget,
         StepType cadenceStepType, int targetCadence_start, int targetCadence_end, int cadence_range,
         StepType hrStepType, double targetHR_start, double targetHR_end, int HR_range);
Interval( const Interval& other );
~Interval();
bool operator==(const Interval &other) const;
friend QDataStream& operator<<(QDataStream& out, const Interval& interval);
friend QDataStream& operator>>(QDataStream& out, Interval& interval);


/// -------------------- setters -----------------------
void setId(int id) {
    this->id = id;
}

[...]
};
Q_DECLARE_METATYPE(Interval)@

andreyc

Do you provide all necessary functions to make your class assignable

"Here's an example custom data type that meets the requirement of an assignable data type:":http://qt-project.org/doc/qt-5/containers.html#the-container-classes
@
class Employee
{
public:
Employee() {}
Employee(const Employee &other);

Employee &operator=(const Employee &other);

private:
QString myName;
QDate myDateOfBirth;
};
@

maximus

ahhhh I found it, my copy constructor what not updated with the new field that I added!

Thank you very much all, i'm starting to get the hang of this language ;)

@/// Copy Constructor
Workout::Workout( const Workout& other ) {

this->id = other.id;
this->lstInterval = other.lstInterval;
this->lstIntervalSource = other.lstIntervalSource;  // Forgot to add!
this->lstRepeat = other.lstRepeat;
this->name = other.name;
this->createdBy = other.createdBy;
this->description = other.description;
this->plan = other.plan;
this->type = other.type;
this->durationMinutes = other.durationMinutes;
this->durationQTime = other.durationQTime;
this->maxPowerPourcent = other.maxPowerPourcent;

}@

andreyc

I would suggest to move this code to a function and call that function from the copy constructor and from an assignment operator. This way you don't need to duplicate your typing and you if you miss some variables then you will miss it in consistent way :-).

maximus

hehe thanks for the tip, yeah it's likely i'll forget this again!
In fact I just have this code in the copy constructor, I didn't provide an assignement operator.

I just read this :
If we don't provide a copy constructor or an assignment operator, C++ provides a default implementation that performs a member-by-member copy.

So i'll try to remove my copy constructor, that way I don't have to code it and the default should do the job!
Thanks!

andreyc

Default copy constructor will work correctly only if all class variables provides their own copy constructors. Standard types like int, bool, etc provide it.
You need to be careful with default copy constructor and default assignment operator.

maximus

Thanks for the heads up,

For now, I guess they all qualify as standard type, QList with custom object seems to work also, my object shouldn't change much for now so being lazy i'll use the default copy constructor ;)

Workout variables :
@ QList<Interval> lstInterval;
QList<Interval> lstIntervalSource;
QList<RepeatWidget*> lstRepeat

int id;
QString name;
QString createdBy;
QString description;
Type type;
QString plan;
double maxPowerPourcent;
double durationMinutes;
QTime durationQTime;@

Interval variables :
@ int id;
QTime duration;
QString displayMessage;

StepType powerStepType;
double targetFTP_start;
double targetFTP_end;
int targetFTP_range;
double leftPowerTarget;

StepType cadenceStepType;
int targetCadence_start;
int targetCadence_end;
int targetCadence_range;

StepType hrStepType;
double targetHR_start;
double targetHR_end;
int targetHR_range;@