This is what they call : peer review...
1. GetUDTs : sorry about that. Slipped away from my attention. I'll try to fix that.
2. TZInterbase6DatabaseMetadata.UncachedGetCrossReference : Seems to be an unimplemented function. So I should have removed it, I think.
3. TZMsSqlDatabaseMetadata.UncachedGetImportedKeys : This is a difficult one. Changing this would change current behaviour by including a potential performance degradation. As long as those Uncached* functions are protected, it doesn't really make sense to change this. Of course, when we further enable an uncached TZDatabaseMetadata feature (eg. by adding a property 'TZAbstractDatabaseMetadata.Cached' property), this should be handled well. If I'm not mistaken, this can be done in the ancestor class's GetXXX functions. If this property is false, the caching mechanism should be short-circuited there, immediately turning all general GetXXX functions into uncached versions .
Your idea about number 3?
Mark
Metadata cache key retrieval API
Moderators: gto, cipto_kh, EgonHugeist, mdaems
2. You could, yes. Or just call "inherited UncachedGetCrossReference". (notice the "Uncached" part)
3. Hmm... this is indeed a tricky one. On the one hand, as far as the imported keys are concerned, the returned entry should not be cached (the user is expecting uncached keys). This means that calling the cached version of GetCrossReference from UncachedGetImportedKeys is not correct. On the other hand, if the UncachedGetImportedKeys is itself called from the cached GetImportedKeys, then both the imported keys and the cross-reference entries should be cached. In this case, calling the cached version of GetCrossReference is correct.
I think it may be wiser to just not mix functions like this.... if it's imported keys we're talking about, then leave the cross-references alone. But on the other hand, code reuse is a good thing, and code duplication is not... It's a tough call...
On third thought, calling UncachedGetCrossReference is practically equivalent to leaving the cross-references alone (you don't touch the cache - just get fresh values), and provides code reuse.
Yes, a "Cached" property in TZAbstractDatabaseMetadata will solve this. Question is, do you want to do that now?
Btw, are properties allowed in delphi interfaces? Because you'd probably want to expose the Cached property in TZSQLMetadata, and it only uses the interface (IZDatabaseMetadata)..
A small tip: if/when you decide to implemnet a Cached property, I would opt to do it using method pointers. Instead of adding ifs to the Getxxx methods, rename the Getxxx to CachedGetxxx, add method pointer properties called Getxxx (same as events, but read-only), and just switch the pointers whenever the Cached property is modified (which shouldn't be often).
2,3. These aren't the only two places. Have a look around, there are more places like this.
4. virtual/override - see edit in my previous post.
3. Hmm... this is indeed a tricky one. On the one hand, as far as the imported keys are concerned, the returned entry should not be cached (the user is expecting uncached keys). This means that calling the cached version of GetCrossReference from UncachedGetImportedKeys is not correct. On the other hand, if the UncachedGetImportedKeys is itself called from the cached GetImportedKeys, then both the imported keys and the cross-reference entries should be cached. In this case, calling the cached version of GetCrossReference is correct.
I think it may be wiser to just not mix functions like this.... if it's imported keys we're talking about, then leave the cross-references alone. But on the other hand, code reuse is a good thing, and code duplication is not... It's a tough call...
On third thought, calling UncachedGetCrossReference is practically equivalent to leaving the cross-references alone (you don't touch the cache - just get fresh values), and provides code reuse.
Yes, a "Cached" property in TZAbstractDatabaseMetadata will solve this. Question is, do you want to do that now?
Btw, are properties allowed in delphi interfaces? Because you'd probably want to expose the Cached property in TZSQLMetadata, and it only uses the interface (IZDatabaseMetadata)..
A small tip: if/when you decide to implemnet a Cached property, I would opt to do it using method pointers. Instead of adding ifs to the Getxxx methods, rename the Getxxx to CachedGetxxx, add method pointer properties called Getxxx (same as events, but read-only), and just switch the pointers whenever the Cached property is modified (which shouldn't be often).
2,3. These aren't the only two places. Have a look around, there are more places like this.
4. virtual/override - see edit in my previous post.
- mdaems
- Zeos Project Manager
- Posts: 2766
- Joined: 20.09.2005, 15:28
- Location: Brussels, Belgium
- Contact:
2. removed all other similar cases... so I'll just be consistent ad remove it.
3. searched a little for these cases. Found this :
- TZPostgreSQLDatabaseMetadata.UncachedGetImportedKeys
- TZPostgreSQLDatabaseMetadata.UncachedGetExportedKeys
- TZAdoDatabaseMetadata.UncachedGetImportedKeys
- TZAdoDatabaseMetadata.UncachedGetExportedKeys
- TZMsSqlDatabaseMetadata.UncachedGetImportedKeys
- TZMsSqlDatabaseMetadata.UncachedGetExportedKeys
--> Done. (not commited yet) A question: was it just double caching or was there also reuse of the crossref cache? I don't think reuse was possible anyway using the current caching implementation. Can you confirm that?
- Properties are possible for interfaces, but only using Get / Set methods.
- Your tip : is that possible when using interfaces? No risk of getting interfaces pointing to the wrong function? Other risks? Is it possible to do this on the Abstract Metadata class level?
4. I hope I didn't miss one... Please check after the commit.
Mark
(PS. I'll be away for I week. I'll catch up afterwards.)
3. searched a little for these cases. Found this :
- TZPostgreSQLDatabaseMetadata.UncachedGetImportedKeys
- TZPostgreSQLDatabaseMetadata.UncachedGetExportedKeys
- TZAdoDatabaseMetadata.UncachedGetImportedKeys
- TZAdoDatabaseMetadata.UncachedGetExportedKeys
- TZMsSqlDatabaseMetadata.UncachedGetImportedKeys
- TZMsSqlDatabaseMetadata.UncachedGetExportedKeys
--> Done. (not commited yet) A question: was it just double caching or was there also reuse of the crossref cache? I don't think reuse was possible anyway using the current caching implementation. Can you confirm that?
- Properties are possible for interfaces, but only using Get / Set methods.
- Your tip : is that possible when using interfaces? No risk of getting interfaces pointing to the wrong function? Other risks? Is it possible to do this on the Abstract Metadata class level?
4. I hope I didn't miss one... Please check after the commit.
Mark
(PS. I'll be away for I week. I'll catch up afterwards.)
Whenever you call a cached method such as GetCrossReferences, it first tries to fetch the cached entry, if there is one. So if you call GetCrossReferences from UncachedGetImportedKeys, it will try to use the cross-references cache entry.
Re tip: I think so. Something like this:
Re tip: I think so. Something like this:
Code: Select all
TTablesGetter = function (...): ResultType of object;
IMyInterface = interface(...)
property GetTables: TTablesGetter read GetTablesGetter; // or maybe GetGetTables...
property Cached: Boolean read GetCached write SetCached;
...
end;
TAbstractImplementer = class(..., IMyInterface)
private
FCached: Boolean;
FTableGetter: TTablesGetter;
protected
function CachedGetTables(...): ResultType; virtual; // cached tables getter
function UncachedGetTables(...): ResultType; virtual; // uncached tables getter
protected
function GetCached: Boolean;
procedure SetCached(const Value: Boolean);
function GetTablesGetter: TTablesGetter;
public
property GetTables: TTablesGetter read GetTablesGetter; // or GetGetTables
property Cached: Boolean read GetCached write SetCached;
end;
function TAbstractImplementer.GetCached: Boolean;
begin
Result := FCached;
end;
procedure TAbstractImplementer.SetCached(const Value: Boolean);
begin
FCached := Value;
if (Value) then
FTableGetter := CachedGetTables
else
FTableGetter := UncachedGetTables;
end;
function TAbstractImplementer.GetTablesGetter: TTablesGetter;
begin
Result := FTableGetter;
end;