[C++] Return by const ref, pointer, of value

[EddoH]

Soms blijf ik wel eens hangen op de simpelste dingen. Een dilemma wat ik vaak tegenkom en ik elk project weer mijn hoofd over breek over wat nou het 'mooist' is, het meest performant, en/of future-proof met betrekking tot het returnen van data uit een klasse.

Stel een class met private data collectie:

C++:

class Bar;
class Foo
{
  public:
     Bar* getItem(int id ); //returns NULL if id out of range
  private:
     List<Bar> m_barItems;
}

Hoe zouden jullie getItem(..) definieren als de gegeven index ook out of range kan zijn. Nu return ik vaak het gevraagde object als pointer, maar NULL als niet gevonden. Echter gaat dit een beetje tegen de regels van encapsulatie in: ik geef een pointer naar interne data aan de buitenwereld.
Return by value kan ook. Maar dan moet Bar weer een property als .valid() of .isNull() oid bevatten. Idem voor const ref (welke ik dan prefereer). Een alternatief is om een pointer naar een dest Bar object mee te geven welke false returned als niet gevonden:

C++:

bool getItem(int id, Bar * dest); 

Met het risico een ongeldige dest ptr aangereikt te kregen.

Mijn hele simpele vraag is dus: wat heeft jullie voorkeur en waarom (Exceptions geen optie)?

[PrisonerOfPain]

Lekker de hele collectie terug geven en de client laten bepalen hoe 'ie er mee interact. Of een functie toevoegen aan Foo die doet wat de client van getItem() zou doen. Zo heb je je data access lekker bij elkaar en is het makkelijker om performant te maken

[Bob]

Valt imo niet op te antwoorden zonder context.

Om toch maar iets te antwoorden: als Bar een type is dat evenveel of niet relevant meer plaats inneemt dan een pointer, return ik dikwijls Bar by value. Handig voor de caller en kan je direct de method const maken.

En los daarvan ivm out of range: als je de caller de vrijheid geeft van een index te gebruiken dan moet de caller er ook voor zorgen dat het een zinnige index is. Dus voorzie een getNrElements(). Maar nu ben je eigenlijk al een vector aan het wrappen, waarmee we weer bij het begin zijn: waarom doe je dit? Of ook: context

[ Voor 39% gewijzigd door Bob op 04-03-2014 16:47 ]

[EddoH]

PrisonerOfPain schreef op dinsdag 04 maart 2014 @ 16:42:
Lekker de hele collectie terug geven en de client laten bepalen hoe 'ie er mee interact. Of een functie toevoegen aan Foo die doet wat de client van getItem() zou doen. Zo heb je je data access lekker bij elkaar en is het makkelijker om performant te maken

Dat van de hele collectie teruggeven Is niet eens zo'n gek idee inderdaad. De interface van Foo 'vervuilen' met methods voor bar ben ik minder fan van..

Bob schreef op dinsdag 04 maart 2014 @ 16:43:
En in dat geval ivm out of range: als je de caller de vrijheid geeft van een index te gebruiken dan moet die er caller ook voor zorgen dat het een zinnige index is. Dus voorzie een getNrElements(). Maar nu ben je eigenlijk al een vector aan het wrappen.

Uiteraard is de size op te vragen, maar een fout is zo gemaakt en als je dan bijvoorbeeld een default constructed object returned is dat soms best lastig debuggen. Dat is wat me het meest tegenstaat aan return by value.

[Bob]

EddoH schreef op dinsdag 04 maart 2014 @ 16:49:
[...]


Dat van de hele collectie teruggeven Is niet eens zo'n gek idee inderdaad. De interface van Foo 'vervuilen' met methods voor bar ben ik minder fan van..


[...]


Uiteraard is de size op te vragen, maar een fout is zo gemaakt en als je dan bijvoorbeeld een default constructed object returned is dat soms best lastig debuggen. Dat is wat me het meest tegenstaat aan return by value.

Als de caller een invalid index opvraagt is het sowieso undefined behaviour en laat ik de boel crashen, of dat nu met een exception is of iets anders maakt niet uit. Een default construted Bar returnen is al een stap te ver op dat moment en gaat idd het begin zijn van veel meer problemen.

[Soultaker]

Is een const-pointer geen optie? (Is nog steeds een pointer naar interne data, maar deelt het voordeel van de const-reference dat 'ie in ieder geval niet gebruikt kan worden om interene data te wijzigen.)

[PrisonerOfPain]

EddoH schreef op dinsdag 04 maart 2014 @ 16:49:
[...]


Dat van de hele collectie teruggeven Is niet eens zo'n gek idee inderdaad. De interface van Foo 'vervuilen' met methods voor bar ben ik minder fan van..

Hangt er een beetje vanaf met hoe je de ownership van je Bar* wilt verdelen natuurlijk. De reden is zo: meestal wil je iets doen met een collectie van Bar's (zo niet, collectie van size = 1). De vraag is wie de baas is van die operatie.

[EddoH]

Soultaker schreef op dinsdag 04 maart 2014 @ 17:20:
Is een const-pointer geen optie? (Is nog steeds een pointer naar interne data, maar deelt het voordeel van de const-reference dat 'ie in ieder geval niet gebruikt kan worden om interene data te wijzigen.)

Const pointer heeft inderdaad de voorkeur en dat gebruik ik dan ook vaak. In mijn voorbeeld de const eigenlijk vergeten. Het zit me dan alleen een beetje dwars dat het nog steeds een pointer naar interne data is, maargoed, de const moet genoeg bellen laten rinkelen bij de gebruiker van de interface.

Ik was meer op zoek naar een soort rule of thumb, een methode die in 90% van de gevallen de beste is omdat x en y. Ik zie vaak verschillende manier gebruikt worden voor dit probleem end weet van alle wel de voor en nadelen, maar toch blijf ik vaak hangen in dit dilemma. Soms omdat ik de ownership nog niet goed gedefinieerd heb, of opties voor uitbreidingen in de toekomst open wil houden. Het heeft ook invloed op de code van de caller.

PrisonerOfPain schreef op dinsdag 04 maart 2014 @ 17:56:
[...]


Hangt er een beetje vanaf met hoe je de ownership van je Bar* wilt verdelen natuurlijk. De reden is zo: meestal wil je iets doen met een collectie van Bar's (zo niet, collectie van size = 1). De vraag is wie de baas is van die operatie.

Wat me het meest tegenstaat is dat ik dan alle methodes van Bar aan het wrappen ben in de owner. Als ik Bar uitbreid dan moet ik gelijk de interface van Foo uitbreiden. Als de Foo meerdere collecties beheerd wordt dat een draak van een klasse.

[ Voor 22% gewijzigd door EddoH op 04-03-2014 18:06 ]

[PrisonerOfPain]

EddoH schreef op dinsdag 04 maart 2014 @ 18:01:
Wat me het meest tegenstaat is dat ik dan alle methodes van Bar aan het wrappen ben in de owner. Als ik Bar uitbreid dan moet ik gelijk de interface van Foo uitbreiden. Als de Foo meerdere collecties beheerd wordt dat een draak van een klasse.

Dan zou je altijd Bar nog kunnen modelen als een collectie ipv als een enkel object

[EddoH]

PrisonerOfPain schreef op dinsdag 04 maart 2014 @ 18:09:
[...]

Dan zou je altijd Bar nog kunnen modelen als een collectie ipv als een enkel object

Die ik dan return als..?

In het geval waar ik nu mee bezig ben ga ik inderdaad maar voor de Bars collectie als geheel als const ref returnen, is het meest logisch in dit geval (een grid-achtige container Foo waar ik de kolom properties Bar beschikbaar wil maken).

Maar het is me wel duidelijk geworden dat het zoals altijd weer in gevalletje context is. En misschien moet ik niet altijd proberen een gouden oplossing die voor alle cases de beste is te zoeken...

[H!GHGuY]

Bob schreef op dinsdag 04 maart 2014 @ 17:11:
[...]

Als de caller een invalid index opvraagt is het sowieso undefined behaviour en laat ik de boel crashen, of dat nu met een exception is of iets anders maakt niet uit. Een default construted Bar returnen is al een stap te ver op dat moment en gaat idd het begin zijn van veel meer problemen.

Dat, of:

C++:

class Bar;
class Foo
{
  typedef List<Bar> BarList;
public:
  typedef BarList::iterator iterator;
  typeder BarList::const_iterator const_iterator;

  iterator getItem(int id) { return std::advance(m_Bars.begin(), id); }
  iterator begin()  { ... }
  iterator end() { ... }
private:
  BarList m_Bars;
};

Iets in die trant - en dan pass je maar de end iterator, of laat je BarList het probleem opvangen.

Maar ik vraag me af waarom je dit probleem zoveel tegenkomt... Iets zegt me dat je ergens anders met een probleem zit waardoor je dit als gevolg hebt. Sowieso moet je je geen zorgen maken om zaken by-value te returnen, dat lost return-value-optimization wel voor je op.

[PrisonerOfPain]

H!GHGuY schreef op dinsdag 04 maart 2014 @ 22:00:
[...]


Dat, of:

C++:

class Bar;
class Foo
{
  typedef List<Bar> BarList;
public:
  typedef BarList::iterator iterator;
  typeder BarList::const_iterator const_iterator;

  iterator getItem(int id) { return std::advance(m_Bars.begin(), id); }
  iterator begin()  { ... }
  iterator end() { ... }
private:
  BarList m_Bars;
};

Iets in die trant - en dan pass je maar de end iterator, of laat je BarList het probleem opvangen.

Ik vind dit altijd maar een beetje een suffe oplossing, je hebt nu een container gemaakt die minder kan dan de container die ge-encapsulate wordt. Zelf zou ik die iterators, begin, end en getItem in dit geval nooit exposen (tenzij ik natuurlijk een bepaald datatype implementeer).

EddoH schreef op dinsdag 04 maart 2014 @ 19:41:
[...]
Die ik dan return als..?

(const) ref, of smart pointer.

[ Voor 9% gewijzigd door PrisonerOfPain op 05-03-2014 00:29 ]

[Zoijar]

PrisonerOfPain schreef op woensdag 05 maart 2014 @ 00:24:
of smart pointer.

- Nee... alleen als je ownership transferred. Dus een unique_ptr is ok als je hem uit je eigen lijst moved, een shared_ptr is behoorlijk dubieus (en traag: mem lock). Gewoon een raw pointer als je object ownership heeft.

- Dat met die iterators vind ik ook niks. Dan kan je gewoon een const ref naar je vector doen.

- Zo maar tussendoor: waarom eigenlijk een list<Bar> (List zelfs?); is die echt nodig? Ik gebruik zelden tot nooit lists. vector<Bar>?

- const ptr return is het meest gebruikelijke. Kan je ook if ((Bar* b = getItem(5))) {b.foo();} doen (of een non-const ptr natuurlijk, als je mag wijzigen)

- Alleen als getItem ook echt iets te doen heeft, error checking/reporting, index conversie, dynamische objecten aanmaken, etc. Anders een const ref naar je container.

H!GHGuY schreef op dinsdag 04 maart 2014 @ 22:00:
Sowieso moet je je geen zorgen maken om zaken by-value te returnen, dat lost return-value-optimization wel voor je op.

True, maar dat helpt je alleen als je sowieso een kopie gaat maken. RVO/copy elision helpt je geen nutteloze tussentijdse kopieen te maken, maar een kopie wordt er gemaakt. Dat is meer nuttig als je een functie hebt die een object maakt en daarna niet owned maar returned-by-value; dan maakt de caller hem feitelijk in-place aan vanwege rvo.

[ Voor 70% gewijzigd door Zoijar op 05-03-2014 08:28 ]

[EddoH]

H!GHGuY schreef op dinsdag 04 maart 2014 @ 22:00:

Maar ik vraag me af waarom je dit probleem zoveel tegenkomt... Iets zegt me dat je ergens anders met een probleem zit waardoor je dit als gevolg hebt. Sowieso moet je je geen zorgen maken om zaken by-value te returnen, dat lost return-value-optimization wel voor je op.

'Vaak' is relatief. Het is niet zo dat ik al mijn klasses ontwerp als een soort container waar iedereen maar data uit kan halen, maar soms heb je nu eenmaal in een programma een centrale datastore, die datasets beschikbaar maakt aan verschillende andere componenten. De vraag is dan inderdaad wel of ik mijn ownership model goed heb doordacht.

Zoijar schreef op woensdag 05 maart 2014 @ 08:11:
[...]

- Nee... alleen als je ownership transferred. Dus een unique_ptr is ok als je hem uit je eigen lijst moved, een shared_ptr is behoorlijk dubieus (en traag: mem lock). Gewoon een raw pointer als je object ownership heeft.

- Zo maar tussendoor: waarom eigenlijk een list<Bar> (List zelfs?); is die echt nodig? Ik gebruik zelden tot nooit lists. vector<Bar>?

List is geen std::list maar een QList (Qt). Zie bijvoorbeeld http://stackoverflow.com/questions/6602036/qvector-vs-qlist
In mijn voorbeeldje zou een Vector waarschijnlijk efficienter zijn inderdaad,

- const ptr return is het meest gebruikelijke. Kan je ook if ((Bar* b = getItem(5))) {b.foo();} doen (of een non-const ptr natuurlijk, als je mag wijzigen)

Maar dan wringt het weer bij me. Waarom zou ik een pointer naar interne data vrijgeven. Hoewel generieke containers (zoals een QList) hier ook in voorzien, dus misschien moet ik niet zo zeiken.

- Alleen als getItem ook echt iets te doen heeft, error checking/reporting, index conversie, dynamische objecten aanmaken, etc. Anders een const ref naar je container.

Vaak is dit inderdaad het geval. Mijn voorbeeld is nogal versimpeld. Er wordt bijvoorbeeld dynamisch een lijst met relevante objecten gemaakt, of deels uit een cache geladen. Voor die eerste kan ik by value returnen, omdat de ownership overgaat.

Ofwel, verschillend toepassen bij verschillende situaties. Wat ik nu wel vaker zal doen is gewoon een const ref naar de container returnen, ipv die weer gaan wrappen met een getItem(id) achtige constructie. Nutteloze overhead en voegt niets toe.

[H!GHGuY]

PrisonerOfPain schreef op woensdag 05 maart 2014 @ 00:24:
[...]
Ik vind dit altijd maar een beetje een suffe oplossing, je hebt nu een container gemaakt die minder kan dan de container die ge-encapsulate wordt. Zelf zou ik die iterators, begin, end en getItem in dit geval nooit exposen (tenzij ik natuurlijk een bepaald datatype implementeer).

Die bedenking maakt ik me ook. Daarom dat ik ook aangaf dat het probleem volgens mij elders lag. Anders vervang je die klasse gewoon door een container en basta.


Uit de verder beschrijving van EddoH, leid ik af dat hij soms iets "datastore"-achtig nodig heeft.
In dat geval lijkt me ownership transferren van de volledige container een goeie kanshebber wat correctheid betreft (een kopie of const ref).

Anderzijds ook opletten met het creëren van God-objecten aka "CManagerVanAllesEnNiets"... Misschien kan de functionaliteit op een andere manier mooi weggestopt worden:

C++:

class DBHandle;

template<class T>
class DBContainer
{
  DBContainer(DBHandle& handle) { ... }
  Container<T> GetObjects(...) { ... T::Deserialize(handle.GetDBData(...)); ... }
};

DBHandle handle = CreateDBHandle();

DBContainer<ClassX> ctr(handle);

SomeClass c;
c.DoSomethingWith(c.GetObjects(...));

Zo heb je typesafety, 1 container class per type, loose coupling, etc.

[PrisonerOfPain]

Zoijar schreef op woensdag 05 maart 2014 @ 08:11:
[...]

- Nee... alleen als je ownership transferred. Dus een unique_ptr is ok als je hem uit je eigen lijst moved, een shared_ptr is behoorlijk dubieus (en traag: mem lock). Gewoon een raw pointer als je object ownership heeft.

Dat realiseer ik me, maar aangezien EddoH het had over het feit dat 'ie z'n ownership nog niet goed gedefineerd had leek het me nuttig om te vermelden

Verder is er natuurlijk altijd een alternatief in de vorm van handles.