[C++] Problemino con i puntatori

[shang84]

Ciao,
vorrei che nel main dopo aver chamato la funzione get_p(p) il valore stampato sia quelle a cui nella funzione l'ho inizializzato anziche' un valore casuale.

Come posso cambiare la funzione?


using namespace std;
#include <iostream>

void get_p(double *p);

int main(){
double *p;
get_p(p);
cout << p[0] << endl; // non displaya 99!
return 0;
}

void get_p(double *p){
p = new double[20];
p[0]=99;
}

[bottomap]

Ciao,
Il problema è di indirezione... Ti spiego meglio...

Quando passi un puntatore ad una funzione, puoi modificarne gli elementi, ma ovviamente non puoi modificare il valore del puntatore... in pratica la variabile p che arriva alla funzione è un numero e non ha alcuna relazione col chiamante... non è possibile quindi modificarla.
Quando gli assegni un nuovo valore (con la new) quella modifica ha effetto solo all'interno della funzione... nella main il puntatore rimane non inizializzato.

Per risolvere questo problema puoi scegliere tre alternative:
1) Brutta... mantieni il puntatore globale (sarà quindi visibile ovunque - inutile pertanto passarlo alla funzione).

2) Allochi la memoria nella main e poi passi il puntatore alla funzione (che non dovrà riallocarlo, solo riempirlo)

3) Passi, invece del puntatore, l'indirizzo del puntatore... lo puoi fare in due modi:
a) Per puntatore... la funzione avrà come parametro un double**p... allocherai il puntatore con *p=new... ed accederai agli elementi con (*p)[ i ]. Nella main passerai &p invece che p...
b) Per reference(solo C++)... la funzione avrà come parametro un double* &p... il resto del codice non cambia

Per il 3.b le cose in gioco sono un concetto piuttosto importante se vuoi impararne effettivamente il comportamento... il passaggio per reference è qualcosa di abbastanza semplice, ma che merita un esame attento. In caso riguarda un po'di documentazione relativa...

Ciaociao :)

[shang84]

Chiaro ed esauriente!

Grazie mille :)

[fek]

Ciao,
b) Per reference(solo C++)... la funzione avrà come parametro un double* &p... il resto del codice non cambia


Tutto molto chiaro, solo un piccolo appunto di carattere stilistico :)

In C/C++ e' convenzione che le funzioni restituiscano valori attraverso puntatori e non reference.

Questo no:

void get_p(double*& p)
{
p = ...;
}


Questo si':

void get_p(double** p)
{
*p = ...;
}


Guarda le due chiamate a funzione:

double *p;
get_p(p);

double *p;
get_p(&p);

Nel secondo caso e' piu' chiaro che il puntatore e' restituito come output e non passato come input.
Seguire questa convenzione diventa ancora piu' importante quando la funzione accetta piu' parametri.

Guarda in questo caso:
Multiply(a, b, c);

E' a = b * c oppure c = a * b?

Riscritto cosi' e' subito chiaro:
Multiply(a, b, &c).

Il passaggio per reference dovrebbe avvenire sempre con il const davanti per impedire che l'oggetto venga modificato.

[bottomap]

Ciao,

Si... il discorso non fa una grinza... il fatto di utilizzare il reference con disinvoltura fa parte del bagaglio di un programmatore già esperto... ad ogni modo se un reference non può essere modificato (dici che i risultati non si ritornano con i reference) a cosa serve un reference? Unicamente a togliere di mezzo il -> e sostituirlo con .?

La scrittura è comunque corretta e rappresenta una possibilità se si sta bene attenti a come si usa... Se poi nel codice si ha chiaro quello che succede non è importante la leggibilità cristallina, sempre che il programma rimanga tale anche per eventuali altri sviluppatori.

Ad esempio le funzioni d3dx che operano su matrici in DirectX utilizzano tutti i parametri per reference... il primo è sempre il risultato (che viene comunque anche ritornato dalla funzione).

Ovviamente col passaggio per reference si perde in leggibilità quello che si guadagna in comodità... va solo valutata la cosa sulla base delle esigenze e delle preferenze personali.
L'idea è che quando l'impiego è chiaro non si dovrebbe aver timore ad utilizzarlo (io personalmente non uso un granché questo passaggio, preferisco il classico puntatore, ma in certi casi può tornare comodo - vedi una classettina chiusa in sé stessa con un interfaccia limitata).

PS: Ci siamo scordati entrambi, tra l'altro, dell'ultima (e più semplice) possibilità in materia di puntatori... far tornare alla funzione il puntatore modificato:

4) In pratica la get_p non prende parametri, alloca un double* e lo ritorna al chiamante. La main associa il ritorno di get_p alla variabile p con un semplice assegnamento p=get_p();

Ciaociao :)

[fek]

Ciao,

Si... il discorso non fa una grinza... il fatto di utilizzare il reference con disinvoltura fa parte del bagaglio di un programmatore già esperto... ad ogni modo se un reference non può essere modificato (dici che i risultati non si ritornano con i reference) a cosa serve un reference? Unicamente a togliere di mezzo il -> e sostituirlo con .?

Reference (via const) si usa principalmente per passare oggetti per i quali non vuoi che sia create una copia temporanea, con invocazione del costruttore di copia:

No:

void DoStuffOnBigObject(BigObject bigObject)
{
...
}


Si':

void DoStuffOnBigObject(BigObject const& bigObject)
{
...
}


Nel secondo caso non viene creata una copia di bigObject (perche' viene passato un puntatore internamente) ed e' garantito che l'oggetto non sia modificabile. Inoltre per la semantica dei reference e' garantito che il puntatore che c'e' sotto non puo' essere spostato a puntare ad un altro oggetto. Ultimo vantaggio, l'uso di un reference e' piu' naturale e permette di invocare gli operatori ridefiniti sull'oggetto stesso.

Sull'ultimo vantaggio non tutti sono d'accordo, primo perche' ridefinire gli operatori e' raramente una buona idea, secondo perche' questa versione di DoStufOnBigObject() e' altrettanto corretta:


void DoStuffOnBigObject(const BigObject const* bigObject)
{
...
}


Io preferisco la prima perche' e' piu' concisa. Gusto personale.


La scrittura è comunque corretta e rappresenta una possibilità se si sta bene attenti a come si usa... Se poi nel codice si ha chiaro quello che succede non è importante la leggibilità cristallina, sempre che il programma rimanga tale anche per eventuali altri sviluppatori.

Quello che hai scritto tu e' perfettamente corretto. Ho solo fatto un'aggiunta di carattere stilistico :)


Ad esempio le funzioni d3dx che operano su matrici in DirectX utilizzano tutti i parametri per reference... il primo è sempre il risultato (che viene comunque anche ritornato dalla funzione).

Le funzioni D3DX passano tutto per puntatore e usano la convenzione matematica per l'ordine dei parametri (prima il risultato e poi gli argomenti):

D3DXMatrixMultiply(D3XMATRIX* result, const D3DXMATRIX* left, const D3DXMATRIX* right);

Io avrei preferito questa versione perche' secondo me e' piu' corretta:

D3DXMatrixMultiply(D3DXMATRIX const& left, D3DXMATRIX const& right, D3XMATRIX* result);


L'idea è che quando l'impiego è chiaro non si dovrebbe aver timore ad utilizzarlo (io personalmente non uso un granché questo passaggio, preferisco il classico puntatore, ma in certi casi può tornare comodo - vedi una classettina chiusa in sé stessa con un interfaccia limitata).

Secondo me l'output dovrebbe essere sempre fatto via puntatore e non reference per chiarezza.


PS: Ci siamo scordati entrambi, tra l'altro, dell'ultima (e più semplice) possibilità in materia di puntatori... far tornare alla funzione il puntatore modificato

Si', e' vero :)
Anche se spesso sempre per chiarezza io passo comunque anche un solo parametro di output invece di usara una funzione. Qui dipende dallo stile personale e dalle situazioni pero'.

[bottomap]

Ciao,

Si, hai perfettametne ragione per dx... si è trattato di una svista citando "a memoria" (cercavo casi in cui il reference veniva utilizzato "impropriamente" secondo il tuo punto di vista, a questo punto non mi vengono in mente ulteriori esempi, comincio a notare che in effetti hai più che ragione nella tua considerazione - non si finisce mai di imparare ;) )

Personalmente non passo praticamente mai oggetti... preferisco, come detto, ricorrere nel 90% del codice a puntatori (anche trattandosi di oggetti), per cui utilizzo poco anche il const... passando un puntatore in genere non ho problemi di costruttori di copia lanciati indiscriminatamente...

Sicuramente qualcuno che proviene da Java fa bene a tener presente quanto hai sottolineato (spesso e volentieri i newbie Java che passano a C++ si scontrano con la mancanza di RTTI e con una gestione delle cose spesso differente, distruttore a parte).

Mi è balenato un caso in cui ho utilizzato il reference (in realtà sarebbe andato bene un puntatore) per una cosa un po'bislacca... dimmi un po'cosa ne pensi, alla luce anche di quanto abbiamo detto finora...

In pratica avevo una classe che, ad un dato momento restituiva un puntatore a memoria (LPVOID).
Tale memoria veniva liberata dalla classe (al momento della sua distruzione).
La classe la fornivo assieme ad altre cose in una libreria (quindi per lo sviluppatore interfaccia e niente codice).

La prima implementazione fu una banale LPVOID GetPtr()... subito però mi sono chiesto cosa sarebbe successo se l'utente avesse assegnato il risultato ad una variabile, l'oggetto fosse morto per un qualsiasi motivo (anche una delete) e l'utente avesse cercato di accedere al puntatore... un bel crash...

Volendo agevolare la cosa il più possibile pensai a dichiarare GetPtr(LPVOID*)... internamente alla classe ovviamente tenevo traccia dei puntatori così assegnati in modo da liberarli alla morte della classe (il puntatore sarebbe diventato automaticamente NULL non appena la classe si fosse distrutta)... il problema è che l'utente era costretto a dichiarare un puntatore e a richiamare GetPtr(&puntatore).
Nell'interfaccia si notava qualcosa che per l'utilizzatore non era chiarissimo... un paio di persone (magari inesperte, per carità) che mi hanno contattato dichiaravano un LPVOID* e lo passavano alla funzione...

In quel momento sono ricorso al reference... GetPtr(LPVOID&)... a quel punto l'utente non poteva fare altro che dichiarare un LPVOID e passarlo alla funzione... niente alternative... e il codice è rimasto decisamente più pulito...

Ovviamente, questa liberazione di memoria da parte mia comportò un chiaro commento di "Attenzione... una volta chiusa la classe ogni puntatore viene reso nullo..." negli esempi che corredavano la soluzione, ma questo sarebbe stato necessario in qualsiasi implementazione dell'idea di liberare la memoria...

Quindi probabilmente può essere questionabile questa mia attenzione verso l'utilizzatore (che la liberi lui la memoria! :D ), ma preferivo limitare il più possibile i compiti di colui che avrebbe dovuto usare la classe...

Ciaociao :)

[fek]

Quindi probabilmente può essere questionabile questa mia attenzione verso l'utilizzatore (che la liberi lui la memoria! :D ), ma preferivo limitare il più possibile i compiti di colui che avrebbe dovuto usare la classe...

Ciaociao :)

Se proprio devo essere sincero, non e' questionabile, e' proprio sbagliato :p

Stai violando un principio fondamentale nel C++ che si chiama RAII: Resource Acquisition Is Initialisation.

In italiano significa che chi inizializza una risorsa (chi crea un oggetto nel tuo caso) ne e' anche il possessore ed e' colui che deve essere incaricato di rilasciare la risorsa (distruggere l'oggetto nel tuo caso).

Nel tuo esempio quindi, se ho capito bene quel GetPtr e' un Factory, piu' correttamente dovrebbe essere chiamato qualcosa tipo CreateObject e restituisce un puntatore ad un oggetto appena creato.

Ora, la cosa corretta da fare e' che la libreria che ha creato l'oggetto sia anche responsabile della sua distruzione, quindi devi provvedere una funzione parallela DestroyObject che chiami il delete.

Per impedire all'utente di distruggere l'oggetto, dichiari il distruttore della classe privato.

Ora, il secondo errore e' usare LPVOID per passare il puntatore ad un oggetto che poi magari l'utente dovra' castare con un reintepret_cast al tipo corretto. Facendo cosi' perdi tutto i vantaggi nell'usare un linguaggio fortemente tipizzato come il C++ ed esponi il tuo utente a svariati tipi di errori possibili.

Se ho capito bene il tuo design, un modo piu' corretto di implementarlo sarebbe dichiarando un'interfaccia al tipo di oggetto con metodi virtuali puri che esporrai in un header file:


class MyInterface
{
public:
virtual void MyMethod() = 0;

static MyInterface* Create(); // Factory Method pattern
static void Destroy(MyInterface** obj);

protected:
virtual ~MyInterface() = 0;; // protetto, il cliente non puoi invocarlo
};


Puoi implementare il tuo oggetto concreto che non esporrai in nessun header file al cliente:


class MyConcreteObject
{
public:
virtual MyMethod() { // do stuff; }
virtual ~MyConcreteObject() { }
};


Ed infine implementi il factory method:


MyInterface* MyInterface::Create()
{
return new MyConcreteObject();
}

void MyInterface::Destroy(MyInterface** obj)
{
delete *obj;
*obj = 0;
}


Codice cliente:



MyInterface* obj = MyInterface::Create();

// ...

MyInterface::Destroy(&obj);


Se non ho scritto qualche scemenza grossa, l'idea dovrebbe essere piu' o meno questa :)

Ci sono parecchie altre varianti, puoi implementare il Factory Method pattern con tutti i crismi, ma questo dipende dal tuo problema particolare.

I punti chiave sono:
- chi crea un oggetto lo distrugge
- non si passano e restituiscono MAI oggetti via LPVOID

La tua attenzione a limitare l'uso di una classe da parte dell'utente e' invece perfettamente corretta. Devi solo porre piu' attenzione a come lo implementi. Il C++ e' una brutta bestia.

[bottomap]

Ciao,

Beh... non pensavo fosse del tutto scorretto...

Ci tuffiamo nei gusti personali, ma la OOP mi sembra un'ottima scelta quando non ti costringe a dei "giri di parole" per non violarne la filosofia... la forza del C++ è proprio la possibilità di programmare in maniera "mista" a seconda delle esigenze... altrimenti avrei usato Java e buonanotte (li si che dovevo ricorrere al pattern Factory).

L'LPVOID tornato non è un oggetto di nessun genere... è semplicemente memoria (di un'immagine, un suono o qualsiasi altro tipo di file) e ho preferito lasciarlo void per poter castare sia ad LPBYTE che ad RGBQUAD*, a WAVEFORMATHDR* (o altre strutture, puntatori comunque e non ad oggetti)... in alternativa potevo tranquillamente tornare, di default, un char* e lasciare all'utente il cast.

Oltre che con una delete esplicita la classe se ne può andare uscendo semplicemente dallo scope... il distruttore può anche restare privato ma non impedisce che un puntatore a memoria liberata se ne vada a zonzo per il resto del codice...

Il discorso del pattern factory mi torna... ma l'oggetto che istanzio non crea un'altro oggetto, semplicemente ritorna un puntatore a della memoria che l'oggetto stesso contiene - è il risultato di un metodo ed in questo senso la classe ne è il possessore - il programmatore che usa la classe non è il proprietario della memoria che ottiene...
Si può pensare alla GetPtr che istanzia un oggetto di una nuova classe (class_memoria)... ma mi sembrava un peso in più (tocca a dichiarare n oggetti diversi) per l'utilizzatore rispetto alla semplicità che volevo ne venisse fuori.

Inoltre volevo evitare che l'utente fosse costretto ad esplicitare la distruzione (MyInterface:: Destroy(&obj))... è previsto un rilascio esplicito della memoria, ma volevo evitare memory leaks in caso che tale distruttore non venga chiamato (per sbaglio, per una return preventiva o per qualsiasi altro problema).

Ciaociao :)

[fek]

Ciao,

Beh... non pensavo fosse del tutto scorretto...

Ci tuffiamo nei gusti personali, ma la OOP mi sembra un'ottima scelta quando non ti costringe a dei "giri di parole" per non violarne la filosofia... la forza del C++ è proprio la possibilità di programmare in maniera "mista" a seconda delle esigenze... altrimenti avrei usato Java e buonanotte (li si che dovevo ricorrere al pattern Factory).

L'LPVOID tornato non è un oggetto di nessun genere... è semplicemente memoria (di un'immagine, un suono o qualsiasi altro tipo di file) e ho preferito lasciarlo void per poter castare sia ad LPBYTE che ad RGBQUAD*, a WAVEFORMATHDR* (o altre strutture, puntatori comunque e non ad oggetti)... in alternativa potevo tranquillamente tornare, di default, un char* e lasciare all'utente il cast.


No, qui non e' una questione di gusti personali, e' una questione di correttezza o meno dell'uso del linguaggio. Anche se il linguaggio ti permette qualcosa, questo non significa che tu debba usarlo e in un linguaggio fortemente tipizzato tornare un'area di memoria che l'utente puo' castare a quello che gli pare e' esattamente quello che non va fatto. Si chiama polimorfismo ed esistono pattern semplici e corretti per implementare questo concetto. Come lo hai implementato tu esponi il cliente a tutta una serie di errori e problemi che dovresti evitargli.

In sintesi: non e' affatto un buon design.

Oltre che con una delete esplicita la classe se ne può andare uscendo semplicemente dallo scope... il distruttore può anche restare privato ma non impedisce che un puntatore a memoria liberata se ne vada a zonzo per il resto del codice...

No, non puo', perche' impedisci la creazione dell'oggetto sullo stack e tieni sotto controllo l'intero ciclo di vita dell'oggetto attraverso i metodi Create/Destroy. L'oggetto puo' essere creato e distrutto solo quando vuoi e con gli strumenti che metti a disposizione. Quando il puntatore esce dallo scope, l'oggetto resta allocato.

Per evitare eventuali memory leak, puoi implementare un semplice garbage collector tenendo una lista di oggetti creati che se non sono distrutti espliticamente vengono distrutti dal gestore alla fine dell'applicazione.

Un'altra soluzione e' la creazione di un wrapper attorno al puntatore sotto forma di smart pointer che invoca automaticamente Destory all'uscita dello scope e impedisce fisicamente il memory leak, ma porta dietro altre complicazioni.

Il discorso del pattern factory mi torna... ma l'oggetto che istanzio non crea un'altro oggetto, semplicemente ritorna un puntatore a della memoria che l'oggetto stesso contiene - è il risultato di un metodo ed in questo senso la classe ne è il possessore - il programmatore che usa la classe non è il proprietario della memoria che ottiene...
Si può pensare alla GetPtr che istanzia un oggetto di una nuova classe (class_memoria)... ma mi sembrava un peso in più (tocca a dichiarare n oggetti diversi) per l'utilizzatore rispetto alla semplicità che volevo ne venisse fuori.

Sostanzialmente stai restituendo una rappresentazione interna dell'oggetto che dipende dall'implementazione. In C++ ci sono poche cose piu' pericolose e errate di questo :)
Ti consiglio di rivedere pesantemente il design.

E' molto piu' corretto (in termini di possibilita' di introdurre difetti) fornire un'API di gestione dell'oggetto piuttosto che riportare la sua rappresentazione interna. Infine, scrivere una classe in piu' quando la logica del problema lo richiede e' una buona norma.

Inoltre volevo evitare che l'utente fosse costretto ad esplicitare la distruzione (MyInterface:: Destroy(&obj))... è previsto un rilascio esplicito della memoria, ma volevo evitare memory leaks in caso che tale distruttore non venga chiamato (per sbaglio, per una return preventiva o per qualsiasi altro problema).


Garbage collection o smart pointer.

[bottomap]

Ciao,

Continuiamo a vedere la cosa sotto due luci differenti. La tua è esclusivamente orientata all'OO... non ho intenzione di aggiungere un overhead enorme ad una classe che, in sostanza, è in sé un contenitore. L'utente tra l'altro sa a priori quali sono gli elementi di questo contenitore... forse non era chiaro dai post il senso della cosa, più sotto la descrivo più dettagliatamente...

Sono d'accordo, in caso, per gli smart pointer... al momento dell'implementazione non li avevo ancora sentiti nominare (si parla di qualche annetto fa). Tornare un'area di memoria da castare è esattamente l'utilizzo principale di questa classe...

Probabilmente descrivendo meglio la classe è più chiaro quello che cercavo di ottenere...
In sostanza non è nient'altro che un interfaccia d'accesso ad un file (compresso) contenente n files (grafica, audio et similari - a piacere dell'utente). Ognuno può essere caricato e scaricato dalla memoria in un momento qualsiasi (video di qualche mega è bene che vengano playati e tolti di mezzo quanto prima) con le debite funzioni.

I files contenuti (e dei quali ritorno il famigerato puntatore) sono stati messi li dall'utente, che è quindi pienamente consapevole di cosa contiene l'oggetto e di come castare i puntatori restituiti.. utilizza la classe esclusivamente per ottenere puntatori da castare velocemente... l'accesso risulta molto veloce proprio perché richiede praticamente solo il tempo di decomprimere i dati. Visto poi che i files in genere vengono passati ad API e funzioni che si aspettano aree di memoria o, al limite, scritti su disco è ridondante incapsularli in una struttura troppo elaborata.

L'utilizzo dei reference è stata un'aggiunta in più (come ti dicevo inizialmente tornavo il puntatore e stop) che garantisce un minimo di garbage collection.
Se correttamente usata la classe prevede la distruzione esplicita della memoria allocata, tramite una funzione apposita (o implicita quando l'oggetto esce dallo scopo o viene invocata una delete)... ed in effetti finora non mi sono giunte lamentele in merito alla poca chiarezza o ad usi errati.

Ciaociao :)

[fek]

Ciao,

Continuiamo a vedere la cosa sotto due luci differenti. La tua è esclusivamente orientata all'OO... non ho intenzione di aggiungere un overhead enorme ad una classe che, in sostanza, è in sé un contenitore. L'utente tra l'altro sa a priori quali sono gli elementi di questo contenitore... forse non era chiaro dai post il senso della cosa, più sotto la descrivo più dettagliatamente...

Ti ripeto di no, non e' una questione di vedere la cosa sotto due luci differenti, ma di riconoscere che un design e' scorretto e propende agli errori, mentre un altro non lo e'.

Usare LPVOID per descrivere un oggetto in C++ e' sempre un design scorretto, da qualunque punto di vista, perche' il C++ mette a disposizione strumenti migliori e piu' sicuri per risolvere lo stesso problema.

Probabilmente descrivendo meglio la classe è più chiaro quello che cercavo di ottenere...
In sostanza non è nient'altro che un interfaccia d'accesso ad un file (compresso) contenente n files (grafica, audio et similari - a piacere dell'utente). Ognuno può essere caricato e scaricato dalla memoria in un momento qualsiasi (video di qualche mega è bene che vengano playati e tolti di mezzo quanto prima) con le debite funzioni.

E' del tutto indifferente che cosa rappresenti l'oggetto in questo contesto, come lo e' se l'oggetto e' inserito dall'utente o meno. Qualunque cosa rappresenti quell'oggetto lo puoi modellare con un'interfaccia piu' o meno generica. Modellarlo con LPVOID e' un errore :)

L'utilizzo dei reference è stata un'aggiunta in più (come ti dicevo inizialmente tornavo il puntatore e stop) che garantisce un minimo di garbage collection.
Se correttamente usata la classe prevede la distruzione esplicita della memoria allocata, tramite una funzione apposita (o implicita quando l'oggetto esce dallo scopo o viene invocata una delete)... ed in effetti finora non mi sono giunte lamentele in merito alla poca chiarezza o ad usi errati.


Questo e' un altro errore che viola il principio RAII e apre la porta ad ogni tipo di problema. Se crei la memoria con un allocatore e lo distruggi con un'altra? Undefined behaviour.

Io ti consiglio vivamente di rivedere fortemente tutto il design e di non rimanere ancorato ad una sola soluzione: cio' che contraddistingue un buon programmatore e' la capacita' di capire gli errori nel proprio design (che ci sono sempre) e di saperli correggere e semplificarlo. Difendere a spada tratta il proprio design non e' mai una buona idea, bisogna essere molto critici.

[bottomap]

Ciao,

Non dico che si tratti di un buon design, tutt'altro... è che non lo vedo un design così pessimo... non sono certo così esperto in materia, ma a questo punto ti chiederei come avresti implementato tu questo genere di comportamento senza appesantire troppo la classe... Smart pointers a parte

Parlo quindi della specifica situazione che ho descritto...
-Tramite un altro programma (alla winzip/winrar) l'utilizzatore crea un grosso file compresso contenente un insieme di numerosi files.
-Tramite la classe che ho descritto, esportata da una DLL o in un .lib statico, l'utente vuole accedere nella maniera più indolore possibile (anche in termini di tempo) alle risorse presenti in questo file. L'ideale sarebbe dover invocare il minimo numero di metodi con il minimo numero di parametri. Attualmente qualcosa del tipo: Apri_il_pack, Carica_uno_o_piu_file (in mem), Chiudi_uno_o_piu_file, Chiudi_il_pack.
I files presenti nel pack (lo chiamo così d'ora in poi) sono assolutamente eterogenei ed io non posso fare assunzioni di nessun tipo su questi files.
Il contenuto dei files viene presumibilmente utilizzato nel programma che usa questa dll ed in genere passato ad API o funzioni apposite, come detto.

Sinceramente non riesco a vedere un'implementazione altrettanto minimale (il discorso del reference, concordo, può essere questionabile).
Il ritorno, comunque, di un puntatore a memoria è semplicemente necessario, a mio avviso... Qualunque implementazione posso pensare deve dare all'utente l'accesso alle risorse presenti nel pack... risorse del tutto eterogeneee (non posso certo riscrivermi un handler per ogni possibile tipo di file presente nel pack!). Qualunque interfaccia generica ad un dato momento deve dare all'utente l'accesso alla memoria del file in questione (lo vorrà usare prima o poi).

L'unica alternativa era ottenere, invece che memoria, un file temporaneo ma mi pareva peggiore in caso di crash e meno performante, saturando la temp di files grossotti... forse con i MemoryMappedFiles si poteva riottenere qualcosa, ma in sostanza la cosa cambiava poco... invece che un puntatore avrei dovuto tornare un handle con le stesse problematiche. Solo con gli Smart pointers avrei potuto effettivamente migliorare qualcosa.

Anche utilizzare più di una classe non mi sembrava (e non mi sembra tuttora)una buona soluzione.
Continuo ad avere l'impressione (non me ne volere :) ) che vedi la cosa da un punto di vista esclusivamente OO, che, in questo caso specifico, non rappresenta la soluzione migliore...

Insomma, se ti do l'impressione di "difendere a spada tratta" mi scuso...
Il fatto che la discussione si sia prolungata, da una spiegazione veloce sui puntatori a questo insieme interminabile di post :D , indica che sono molto interessato a quello che mi stai dicendo... pur non condividendolo appieno (probabilmente io penso al problema specifico e tu invece stai facendo una - giustissima - digressione sul design in generale).

Se crei la memoria con un allocatore e lo distruggi con un'altra? Undefined behaviour.
Questa mi è un po'meno chiara... creo la memoria internamente con una new e la tolgo di mezzo con una delete. L'interfaccia alloca ed ottiene il puntatore con una Load e libera la memoria con una Purge. Se l'utente non chiama la Purge è l'oggetto stesso, al momento della sua distruzione, a liberare la memoria.

Grazie, sempre e comunque, delle ottime delucidazioni
Ciaociao :)

[fek]

-Tramite la classe che ho descritto, esportata da una DLL o in un .lib statico, l'utente vuole accedere nella maniera più indolore possibile (anche in termini di tempo) alle risorse presenti in questo file. L'ideale sarebbe dover invocare il minimo numero di metodi con il minimo numero di parametri. Attualmente qualcosa del tipo: Apri_il_pack, Carica_uno_o_piu_file (in mem), Chiudi_uno_o_piu_file, Chiudi_il_pack.
I files presenti nel pack (lo chiamo così d'ora in poi) sono assolutamente eterogenei ed io non posso fare assunzioni di nessun tipo su questi files.
Il contenuto dei files viene presumibilmente utilizzato nel programma che usa questa dll ed in genere passato ad API o funzioni apposite, come detto.

Ti posso dire come abbiamo risolto noi esattamente questo problema :)

La risorsa nel PackFile e' rappresentata da una classe astratta LHFile con i metodi di lettura del file classici (Read/ReadWholeFile/Etc).

Un Factory method static di nome LHFile:: Open cerca il file nel pack e crea un oggetto LHPackedFile che implementa l'interfaccia LHFile. L'equivalente LHFile::Close chiude il file e libera le risorse associate.
Come aggiunta, LHFile:: Open puo' cercare il file prima nel file system e poi nel packed file; se il file e' nel file system, crea e ritorna un oggetto di tipo LHOSFile che implementa la stessa interfaccia, quindi il cliente e' disaccoppiato dall'effettiva implementazione del file e dalla sua posizione, ma lo accede sempre attraverso quell'interfaccia comune.
Fra le altre cose, LHFile:: Open puo' tornare oggetti di tipo LHMemoryMappedFile, sempre con la medesima interfaccia.

Un esempio d'uso:


LHFile* myFile = LHFile::Open("myzipfile.zip");

myFile->Read(buffer, 0, myFile->Length(), bufferSize);

LHFile::Close(&myFile);


Come vedi non c'e' nessun LPVOID di mezzo, il principio RAII e' rispettato, uno smart pointer puo' proteggere dal memory leak, il file nel packed file e' astratto per mezzo dell'interfaccia LHFile. Nulla vieta a LHFile:: Open di tornare una versione memory mapped di quel file, oppure una versione sul file system nativo.

L'utente accede al contenuto del file per mezzo dei metodi Read ai quali e' abituato.

Piccola nota: se allochi memoria in una DLL e la liberi da chi usa la DLL, il minimo che puoi aspettarti e' un crash dell'applicazione.

Un'ultima nota: stai facendo scelte di design basandoti su considerazioni di carattere prestazionale (minimo overhead possibile). Questo e' sbagliato, perche' tu non conosci il profilo prestazionale del tuo sistema e non hai i dati per prendere questa decisione. Chi ti dice che i metodi di gestione del packed file saranno i colli di bottiglia? Nessuno. Allora questo aspetto non deve decidere il tuo design.

[bottomap]

Ciao,

Adesso la cosa è parecchio più chiara... un'ottima implementazione che non viola i principi che hai esposto...
Purtroppo continua a non piacermi il dover ricorrere al myfile->Read, ad una classe aggiuntiva, insomma... ma potrei forse ricorrere anche a qualcosa di alternativo dalla classe originale...

Risposta (con domanda) alla piccola nota: In che senso alloco da dll e dealloco da utente? La memoria viene allocata dall'oggetto istanziato dall'utente al momento della chiamata a "load_file"... quindi nello spazio del processo... e liberata dallo stesso spazio con la "purge". Alla morte dell'oggetto viene forzata una sorta di "purge_all" su quello che eventualmente è rimasto allocato. Non mi sono giunte voci di crash finora (e tutti i test che ho potuto fare non hanno prodotto problemi simili)...

Ciaociao :)

[fek]

Ciao,

Adesso la cosa è parecchio più chiara... un'ottima implementazione che non viola i principi che hai esposto...
Purtroppo continua a non piacermi il dover ricorrere al myfile->Read, ad una classe aggiuntiva, insomma... ma potrei forse ricorrere anche a qualcosa di alternativo dalla classe originale...

In realta' con la Read non fai altro che mantenere un'astrazione al quale ogni programmatore C++ e' abituato (fopen, fread). E' decisamente piu' naturale da usare di un puntatore LPVOID.

Comunque questo e' il meno, il concetto importante e' l'esporre la tua risorsa con un'interfaccia che alza il livello di astrazione. Piu' e' alto il livello di astrazione (interfaccia contro puntatore), piu' e' semplice da usare e, di solito, piu' overhead prestazionale introduci, ma questo e' un altro discorso.


Risposta (con domanda) alla piccola nota: In che senso alloco da dll e dealloco da utente? La memoria viene allocata dall'oggetto istanziato dall'utente al momento della chiamata a "load_file"... quindi nello spazio del processo... e liberata dallo stesso spazio con la "purge". Alla morte dell'oggetto viene forzata una sorta di "purge_all" su quello che eventualmente è rimasto allocato. Non mi sono giunte voci di crash finora (e tutti i test che ho potuto fare non hanno prodotto problemi simili)...


Non so se succede nel tuo caso, ma se fai una cosa del genere:


void* data = CreateData(size);

free(data);


Se CreateData e' in un'altra DLL aspettati un crash perche' potenzialmente stai allocando in un heap e deallocando in un altro. E tutte le varianti del caso, ad esempio una new in una DLL e la delete nel chiamante.

In questi casi seguire il principio RAII e' fondamentale perche' ti assicura che new e delete sono chiamati nello stesso heap.

[bottomap]

Ciao,

No... niente di tutto questo... le funzioni di creazione e deallocazione stanno tutte nell'oggetto... un utilizzo banale è:
PackFile pf;
pf.Open(nomefilecompresso)
pf.Load(nomefile,p) //qui alloca mem
...
pf.Purge(...) //qui la libera
pf.Close()

Avviene tutto dall'oggetto pf... una sola DLL, niente funzioni. La load alloca e la purge dealloca... tutto dallo heap del processo che linka la dll... nella ~PackFile c'è una semplice for(..) pf.Purge.
Certo l'utente può avere l'idea malsana di chiamare una delete sul puntatore che gli ritorno, ma questo è un altro discorso...

Ciaociao :)

[fek]

Avviene tutto dall'oggetto pf... una sola DLL, niente funzioni. La load alloca e la purge dealloca... tutto dallo heap del processo che linka la dll... nella ~PackFile c'è una semplice for(..) pf.Purge.
Certo l'utente può avere l'idea malsana di chiamare una delete sul puntatore che gli ritorno, ma questo è un altro discorso...

Perfetto allora. Devi solo impedirgli di chiamare delete (intercettarlo a compile time con un errore) come ti ho mostrato e sei a cavallo.

Un piccolissimo dettaglio, hai tre metodi Purge, Close, delete (distruttore) che fanno essenzialmente parti della stessa cosa e devono essere chiamati in una certa sequenza.
Ti consiglio di toglierne due dei tre, o dichiararli privati e creare tu il Destoyer. Semplifichi il design e l'uso.

[bottomap]

Ciao,

Pare che siamo arrivati in fondo alla discussione :) :)

Ad ogni modo, visto che un re-engineering del pacchetto era in previsione non tanto per i problemi che sono emersi adesso, quanto per aggiungere un paio di funzionalità (crittazione del pack con pwd e conseguente validazione nell'apertura ed un paio di altri ammennicoli) volevo a questo punto aggiungere ai ToDo quello che è stato in sostanza il risultato di questa discussione...

Il prodotto è completamente freeware, pertanto non posso far altro che aggiungere dei ringraziamenti nell'header a corredo della libreria... Preferisci una frase in particolare, un semplice "thanks to fek", qualcos'altro (email e similari) o niente del tutto? :)

PS: Vedo dalla firma che fai parte del comitato contro l'uso sconsiderato delle k... dove devo firmare? ;) ;)

Ciaociao :)

[fek]

Non c'e' bisogno di alcun ringraziamento :D