[Solved] Return instanced class in base class

Max13

[quote author="MuldeR" date="1399036954"]Well, the caller of find() necessarily needs to know what kind of object the function returns, right?[/quote]
Actually, it's find() itself which has to return an instance of it's own class (the derived class). And find() will only return derived class of BaseClass. If by that, you mean the same thing, yes.

[quote author="MuldeR" date="1399036954"]So it could simply use a dynamic_cast to cast the received object from the base-class to the desired sub-class.[/quote]
Yes, it could be a way. Unfortunately, returning the BaseClass isn't doable because it is abstract, and a dynamic_cast won't call the derived class constructor, which means that derived-class' specificities (like internal specific pointers) won't be initialized, would they?
I keep the template approach aside in case of I find something else more "user-friendly"

Chris_Kawa: You're right, I want to make a factory base class but the id isn't the derived class, but the "ID" of the calling object in a database. For instance:

@class Vehicle
{
public:
static Vehicle find(int ID)
{
// SELECT FROM ... WHERE id = ID;
return Vehicle(...);
}
}

class Car : public Vehicle {...}

int main()
{
Car mercedes = Car::find(5);
}@

Is it clearer ?

MuldeR

Why not something like this:
@class Vehicle
{
virtual void loadFromDB(const int &id) = 0;
};

class Car : public Vehicle
{
virtual void loadFromDB(const int &id)
{
/* a car knows how to load its own DB entry */
}
};

class Database /* The "Factory" */
{
static Vehicle *lookup(const int &id)
{
Vehicle *ret = NULL;
const int type = lookupTypeInDB(id);
switch(type)
{
case DB_TYPE_CAR:
ret = new Car();

     [...]

  }
  if(ret)
     ret->loadFromDB(id);
  return ret;

}
}

int main()
{
Vehicle *vehicle = Database::lookup(5);
if(vehicle)
{
Car car = dynamic_cast<Car>(vehicle);
if(car)
{
printf("%d", car->horsepower());
}
else
{
printf("Not a car!");
}
}
else
{
printf("Not found in DB!");
}
}@

Chris Kawa

So basically my code becomes:
@
class Factory {
public:
static Base& find(int ID) {
auto type = getItFromDB(ID); //that's new
switch(type)
{
case 0:
{
static Derived1 obj;
return obj;
}
case 1:
{
static Derived2 obj;
return obj;
}
}
}
}@
which is the same as MuldeR's except for dynamic vs static allocation.

Max13

I can't do that (talking about conditional allocation with a switch) because I'm trying to do something which will allow me to create derived class without having to modify the Base class or the Database. Maybe templates are the only way, but I don't like the repetitive syntax:

@Car mecedes = Car::find<Car>(4);@

MuldeR

Addendum:
This might also be a use case for the Vistor Pattern:
http://en.wikipedia.org/wiki/Visitor_pattern

MuldeR

[quote author="Chris Kawa" date="1399060171"]
which is the same as MuldeR's except for dynamic vs static allocation.[/quote]

Returning a reference to a static local variable?

What if I call this function twice? Or even multi-threading is used :-O

Very dangerous!

[quote author="Max13" date="1399060384"]I can't do that (talking about conditional allocation with a switch) because I'm trying to do something which will allow me to create derived class without having to modify the Base class or the Database.[/quote]

--> Visitor Pattern <--

[quote author="Max13" date="1399060384"]Maybe templates are the only way, but I don't like the repetitive syntax:
@Car mecedes = Car::find<Car>(4);@
[/quote]

Use a macro like:
@#define FIND(CLASS, VAR,ID)
CLASS VAR = CLASS::find<CLASS>(ID);

main()
{
FIND(Car, mercedes, 42);
}@

Also "auto" type could simplify things, if C++11 is okay.

MuldeR

Visitor-style solution:
@class Vehicle
{
virtual void load(Database *db, const int &id) = 0;
};

class Car : public Vehicle
{
virtual void load(Database *db, const int &id)
{
db->load(this, id);
}
};

class Bike : public Vehicle
{
virtual void load(Database *db, const int &id)
{
db->load(this, id);
}
};

class Database
{
void lookup(Vehicle *vehicle, const int &id)
{
vehicle->load(this, id);
}

void load(Car *car, const int &id)
{
/load car from DB/
}

void load(Bike *car, const int &id)
{
/load bike from DB/
}
}

int main()
{
Database db;
Car *mercedes = new Car();
db.lookup(mercedes, 42);
}@

Yes, there's still a load() function for each type in the Database class, but somewhere that code has to go unavoidably!

Chris Kawa

You've lost me again :(
Do you mean something like this?

@
class Vehicle {
whatever someCommonDBStuff(int ID) { ... }
};

class Car1 : public Vehicle {
static Car1 find(int ID)
{
auto foo = someCommonDBStuff(ID);
return Car1();
}
};

class Car2 : public Vehicle {
static Car2 find(int ID)
{
auto foo = someCommonDBStuff(ID);
return Car2();
}
};
@

Where does template fit here? I can't see it. Why do you put find in the base? From what I see you want it to return different types in derived classes. In that case Base class shouldn't know anything about derived and you can't overload find on just the return type.

The example code Car::find<Car>(4); you gave is basically equivalent to:
@
class Vehicle {};

class Car1 : public Vehicle {
static Car1 findCar1(int ID) { ... }
};

class Car2 : public Vehicle {
static Car2 findCar2(int ID) { ... }
};
@
just without template syntax. You create unrelated methods in derived classes this way.

Chris Kawa

MuldeR - what's wrong with returning reference to local static object?
If you call the function twice you will get two references to it. Nothing wrong or dangerous there.
Returning a pointer is no different (single or multithreading wise).

In C++11 There's even a 1-line thread-safe singleton pattern:
@
Singleton& getIt () { static Singleton s; return s; }
//thread safe because of the C++11 "magic-statics" guarantee
@

MuldeR

[quote author="Chris Kawa" date="1399061766"]
MuldeR - what's wrong with returning reference to local static object?[/quote]

If you call the function repeatedly, you are getting another reference - to the very same statically allocated object! The same object which is now again loaded from the Database! Witch your code, this is not possible:

@Base &car1 = find(42);
Base &car2 = find(666);@

Now car1 and car2 are pointing to the very same object, which has been loaded with the data for entry 666, while the previously loaded data for entry 42 is lost. Almost certainly not what the caller what have expected.

Even worse, imagine there's another thread that also makes use of your Factory class. It certainly would end up in a mess...

[quote author="Chris Kawa" date="1399061766"]Returning a pointer is no different (single or multithreading wise).[/quote]

It is! Each call would be returning a pointer to a completely new object, allocated on the heap via new operator. So it's perfectly safe to call it repeatedly. It's even re-entrant, i.e. safe to use with multiple threads.

Max13

[quote author="MuldeR" date="1399061131"]Yes, there's still a load() function for each type in the Database class, but somewhere that code has to go unavoidably![/quote]

This is close to what I want, but when I say I don't need to reimplement "load/find" in subclass, it's because the implementation would be a copy-paste of the base class one:

@class Vehicle
{
public:
static QVariantMap loadFromDb(QString table, int id); // To make it simple
static CLASS find(int id) // Static or pointer, not important
{
QVariantMap data = Base::loadFromDb(CLASS::staticMetaObject.className(), id); // Way to get the table for i.e.
return CLASS(data);
}
}

class Car
{
public:
static Car find(int id)
{
QVariantMap data = Car::loadFromDb(Car::staticMetaObject.className(), id);
return Car(data);
}
}@

Here is an example of what I think I get if I have to reimplement "find()" or make a conditional allocation. As we can see, for Car, Bike, Moto, ... The code inside "find()" will be exactly the same except for the types, that's why I was trying to find a way to reuse base class code without switch or many if/else.

Chris Kawa

[quote author="MuldeR" date="1399062117"]
Yes, you are getting another reference - to the very same statically allocated object! The same object which is now again loaded from the Database! [/quote]

My impression was that was the intent. OP never said returned object should be different depending on the content returned from DB. I read it as a function that looks something up in DB and then returns a type-related object. Name "find" doesn't really suggest construction of any object copies.

But of course if it's actualy a "createInstanceBasedOnSmthn()" just named "find()" then you're 100% right. I don't seem to get what you guys are talking about here so I'll better leave it to you to hammer out. I need my coffee anyway :P

EDIT: Ok, that last OP entry confirms you DO need a separate instance (because it puts retrieved data into the constructor)

MuldeR

[quote author="Max13" date="1399062356"][quote author="MuldeR" date="1399061131"]Yes, there's still a load() function for each type in the Database class, but somewhere that code has to go unavoidably![/quote]

This is close to what I want, but when I say I don't need to reimplement "load/find" in subclass, it's because the implementation would be a copy-paste of the base class one:[/quote]

If the load() from DB code is the same (copy&paste) for each sub-class type anyway, where is the problem? Why not do this right away:

@class Vehicle {...};
class Car : public Vehicle {...};
class Bike : public Vehicle {...};

class Database
{
static load(Vehicle *vehicle, const int &id)
{
/Loading code here/
}
}

main()
{
Car *car = new Car();
Database::load(car, 42);

Bike *bike = new Bike();
Database::load(bike, 666);
}@

Yes, you now have to create an instance of the proper type before calling the load function. But that is no different with your Template approach, where you need to know the proper type of the loaded object just as well...

Max13

[quote author="Chris Kawa" date="1399062679"]OP never said returned object should be different depending on the content returned from DB.[/quote]
You were half right, it's not depending on the content returned by the DB, the return will depend on the calling class of find() (find() would return an instance of the calling class).

MuldeR: This is a good way to do it, but in my current class tree, a database instance exists in BaseClass. So if I could return the object, it's better.
Honestly, I've already implemented this possibility this way:

@Car car = new Car();
car.fill(Database::get(car.metaObject()->className(), 42);@

But my needs are precisely a way to do it in one line, because I also have an "all()" method which returns a QList<CLASS> for example if I need all the registered cars.

EDIT: "This":http://stackoverflow.com/a/15777222/640164 is also a way to use template pattern without repeating the templated class

MuldeR

[quote author="Max13" date="1399063674"]But my needs are precisely a way to do it in one line[/quote]

If that really is the problem, you could use a macro like:
@#define LOAD_VEHICLE(TYPE, NAME, ID)
TYPE *NAME;
Database::load(NAME, ID);

main()
{
LOAD_VEHICLE(Car, myCar, 42)
LOAD_VEHICLE(Bike, myBike, 666)
}@

Or similarly with the Template-based approach:
@#define LOAD_VEHICLE(TYPE, NAME, ID)
TYPE *NAME = Database::load<TYPE>(ID);

main()
{
LOAD_VEHICLE(Car, myCar, 42);
LOAD_VEHICLE(Bike, myBike, 666);
}@

Max13

Thank you for all your suggestions.

After thinking about it, I realized that it's a design problem.
I won't use a template model Factory, but as every model is constructed with QVariantMap, I just have to make find() and all() return QVariantMap and simply construct my models with it:

@class Model : public DatabaseInterface
{
public:
static QVariantMap find(const int id, const QString table);
};
class Car : public Model
{
public:
static Car find(const int id)
{
return Car(Model::find(42, "Car"));
}
};

int main()
{
Car mercedes(Car::find(42));
}@

Your answer are still interesting for similar cases, but for mine it's just a design error (because I will never need to construct multiple object). I'd rather make my DatabaseInterface return QSqlQueryModel and QSqlTableModel to use them in my views.

Thanks for your answers