2

I have implemented the FlyWeight pattern in my Delphi application. Everything has worked great, everything is a lot faster and takes less memory, but there is one thing I am worried about.

My implementation will only work as long as client code never calls Free() on the shared objects. In the Flyweight pattern, the FlyweightFactory itself is supposed to "maintain a reference to flyweights" i.e. to the shared objects.

My problem is that there is no (obvious) way to stop other code from destroying the objects once they have a reference. I could live with this, but it would be a "big win" if I could pass these objects round freely without worrying about accidental freeing.

To show a (contrived) example:

flyweight1:=FlyweightFactory.GetFlyweight(42); 
WriteLn('Description is '+flyweight.Description); 
flyweight1.Free;

flyweight2:=FlyweightFactory.GetFlyweight(42); 
WriteLn('Description is '+flyweight.Description); 
// Object has already been Freed!; behaviour is undefined

I have considered overriding the destructor as shown here to stop the flyweight object being freed altogether. This is not an option in my case as

a) I only want to stop cached objects from being Freed, not objects that aren't part of the cache. There is a lot of legacy code that doesn't use the cache; they still need to create and free objects manually.

b) I do want the FlyweightFactory to Free the objects during finalization; I agree with Warren P that a "zero leaked memory" policy is best.

I'll leave with a quote from the Flyweight chapter of GoF

Sharability implies some form of reference counting or garbage collection to reclaim storage when it's no longer needed. However, neither is necessary if the number of flyweights is fixed and small. In that case, the flyweights are worth keeping around permanently.

In my case the flyweights are "fixed" and (sufficiently) small.

[UPDATE See my answer for details of how I solved this problem]

Community
  • 1
  • 1
awmross
  • 3,789
  • 3
  • 38
  • 51

5 Answers5

2

My answer to the question you link to still applies. The objects must know by means of a private boolean flag that they are cached objects. Then they can elect not to destroy themselves in Destroy and FreeInstance. There really is no alternative if you want to allow Free to be called.

To deal with finalization you would want to add the cached objects to a list of cached objects. That list of objects can be freed at finalization time. Of course the flag to disable freeing would have to be reset whilst you walked the list.

Having made this point regarding finalization, I would advise you to register an expected memory leak and just leak this memory. It makes the code much simpler and there's nothing to lose. Any memory you don't free will be reclaimed by the OS as soon as your executable closes. One word of caution: if your code is compiled into a DLL then leaking could be troublesome if your DLL is loaded, unloaded, loaded again etc.

What all this is telling you is that you are swimming against the current. Is it possible that you could achieve your goals with a different solution that fitted better with the way Delphi is steering you?

Community
  • 1
  • 1
David Heffernan
  • 601,492
  • 42
  • 1,072
  • 1,490
  • @David: link? I assume you also ruled out interfaces? –  May 04 '11 at 03:05
  • @moz OP is clear that objects should be used. I suspect that there may be a more natural approach to solving the problem than subverting object destruction. – David Heffernan May 04 '11 at 03:07
  • @David: I hope so. I was asking for a link to the previous question - there are afew from that user and none leap out at me. –  May 04 '11 at 03:20
  • @moz Sorry I misunderstood your comment. The link is in the OP's question. It's this one: http://stackoverflow.com/questions/5640199/how-to-create-an-instance-that-can-not-be-destroyed – David Heffernan May 04 '11 at 03:22
  • @David: Are you referring to the link I posted in the question? That question was asked by somebody else. And I didn't see any mention of using a flag in your answer there. Using a flag is an interesting idea though. Slightly convoluted, but it might be the only way. And yes I realise this is not the Delphi way. – awmross May 04 '11 at 03:25
  • @awmross Sorry, I didn't realise the linked Q was asked by someone else. Whilst that question doesn't talk about optionally bypassing object destruction, the principle is the same. There can really be no alternative but to override `Destroy` and `FreeInstance`. You'll have to put some other logic in if you wish to pursue this approach. – David Heffernan May 04 '11 at 03:29
  • @David Perhaps a more Delphi way would be to return a new copy from the cache each time. The client would then be responsible for Freeing that instance. That way the client would never see any of the cached instances (and so couldn't accidently free them). – awmross May 04 '11 at 04:05
  • @awmross that could work if you are prepared to pay the object creation overhead – David Heffernan May 04 '11 at 04:08
  • @David Actually the real Delphi way is to pass in an object you created earlier, and copy the values into that. – awmross May 04 '11 at 04:19
  • @awmross It's very hard to know what's right for you since we don't know all your design constraints. I think only you can take those decisions. I still don't see another way to answer you original question. – David Heffernan May 04 '11 at 04:22
  • Your answer is great. I'm just pondering (out loud) your point about "swimming against the Delphi current". – awmross May 04 '11 at 04:31
  • 1
    @awmross I can think of a number of approaches that are more "with the flow" but I'm reluctant to suggest any with a lack of knowledge of your problem. – David Heffernan May 04 '11 at 04:33
  • @downvoter would you care to offer a reason why this answer doesn't address the question? – David Heffernan May 04 '11 at 14:09
  • @awmross: said "Perhaps a more Delphi way would be to return a new copy from the cache each time". Doesn't that take you full circle and defeat the purpose of the flyweight objects in the first place? – Disillusioned May 04 '11 at 14:20
  • @awmross: said "Actually the real Delphi way is to pass in an object you created earlier, and copy the values into that." Aren't your flyweight objects immutable? Flyweight pattern is perfectly valid in a Delphi context. Usually people wouldn't worry about the flyweights being incorrectly destroyed: `Get...` implies I don't have to destroy it. Whereas `Create...` implies I do. The problem is, you have two paradigms mixed, which makes a incorrect use (aka destruction) of flyweight instances quite possible. – Disillusioned May 04 '11 at 14:22
  • @Craig We don't know why OP is implementing flyweights. Perhaps object creation is expensive, but copying is cheap. – David Heffernan May 04 '11 at 14:22
  • @David: True, but it certainly does seem to defeat the point. Also, another comment of his hinted that his objects might be mutable. In which case flyweight might not have been his best option after all. – Disillusioned May 04 '11 at 14:31
  • @craig all this is why I have refrained from offering advice beyond a direct answer to the question. – David Heffernan May 04 '11 at 14:36
  • @Craig Yes the flyweights are mutable. Even if I had immutable objects and called my method 'GetX' the original problem would remain. i.e. clients can destroy the flyweights. And yes you are both right; the client can alter these objects freely which is another problem :-( – awmross May 04 '11 at 19:21
  • @craig: said "Doesn't that take you full circle and defeat the purpose of the flyweight objects in the first place? " I said that was the *Delphi* way. I didn't say it was a *good* way :-) – awmross May 08 '11 at 23:55
0

You could also hide a destructor by making it protected or private. Programmers won't see it outside the scope of the unit in which it is declared in.

But I am posting this answer more like a curiosity because this will not prevent freeing an object by using FreeAndNil or by using a "Protected Hack"

Wodzu
  • 6,932
  • 10
  • 65
  • 105
0

I suggest to add a reference count in order to known if your shared object is still used. Every client should use the pattern AddRef / Release (AddRef increases the count; Release decrements it; if count reaches zero Free is called)

The AddRef may be called directly by your GetFlyweight method; Release has to be used instead of Free.

If you refactor your class and extract an interface from it the AddRef/Release pattern in naturally implemented in then interface implementation. (You could derive from TInterfacedObject or implement IInterface by your self)

Stefano Moratto
  • 161
  • 2
0

Ideally you seldom want 2 ways of using the same things. It just complicates matters in the long run. In 6 months time, you might not be sure whether a particular piece of code is using the new flyweight paradigm or the old paradigm.

The best way to prevent someone calling Free or Destroy is to make sure it's not even there. And within the Delphi world, the only way to do that is to use interfaces.

To expand on your contrived example:

type
  TFlyweightObject = class
  public
    constructor Create(ANumber: Integer);
    function Description: string;
  end;

  TFlyweightFactory = class
  public
    function GetFlyweight(ANumber: Integer): TFlyweightObject;
  end;

This being an object can easily be destoyed by a rogue client. You could make the following changes:

type
  IFlyweight = interface
  //place guid here
    function Description: string;
  end;

  TFlyweightObject = class(TInterfacedObject, IFlyweight)
  public
    constructor Create(ANumber: Integer);
    function Description: string;
  end;

  TFlyweightFactory = class
  public
    function GetFlyweight(ANumber: Integer): IFlyweight;
  end;

Now any code that is updated to use the flyweight paradigm is forced to use it as intended. It's also easier to recognise the old code that still needs to be refactored because it doesn't use the interface. Old code would still construct the "flyweight" object directly.

Disillusioned
  • 14,635
  • 3
  • 43
  • 77
  • You say that this is the only way. There is always records. – David Heffernan May 04 '11 at 12:05
  • @David: You're right - records can't be destroyed. And you _could_ use them, provided you're willing to forego certain concepts applicable to flyweight ___objects___. ;) – Disillusioned May 04 '11 at 14:12
  • @craig interfaces aren't objects either. Having said all that I don't really want to recommend anything to OP who clearly has lots of existing code that uses object references. – David Heffernan May 04 '11 at 14:17
  • 1
    @David: Do you like being difficult just for the sake of it? You know as well as I do that if you have a valid interface reference, _you have an underlying object_. Not to mention the fact that from an OO perspective, interfaces tend to produce a "purer" mechanism to abstract object interfaces from their implementations. – Disillusioned May 04 '11 at 15:40
  • @Craig I'm not trying to be difficult. That's never my intention. But OP has stated that there is lots of code that uses object references that he/she does not want to change. Switching to interfaces goes against this desire. It might still be the best thing to do, but that's the point I'm trying to make. – David Heffernan May 04 '11 at 15:49
  • @David: And my point is that he should avoid mixing paradigms. New and refactored code can use the flyweights via _interface references_. Older code can use _object references_ to **different instances** of the same kinds of objects. In this way there is something to clearly distinguish old vs. new code, and the old object reference code **won't be able to** make the mistake of accidentally destroying flyweight instances. – Disillusioned May 04 '11 at 15:59
  • 1
    @David: When you 1) Propose records (which are very different from objects) then 2) Claim that interfaces aren't objects when you know perfectly well that interface references are "handles" to objects, then in my book: you're just being argumentative/difficult for the sake of it. .... Are you that bored? – Disillusioned May 04 '11 at 16:06
  • I didn't propose records to OP. It was a comment on your assertion that interfaces are the only way. – David Heffernan May 04 '11 at 16:20
  • @Craig I am hesitant to use Delphi interfaces as our team doesn't have much experience with using them. Also David is right when he says I have a lot of existing code I don't want to touch. I think it would be better to be able to use the flyweights and the 'old style' objects interchangeably; which adding an Interface would not allow? If I was starting from scratch I would look into interfaces. – awmross May 04 '11 at 19:30
0

I managed to get around the problems I cited in my original question using the following techniques, suggested by David Heffernan in his answer.

a) I only want to stop cached objects from being Freed, not objects that aren't part of the cache. There is a lot of legacy code that doesn't use the cache; they still need to create and free objects manually.

I fixed this by subclassing the Flyweight class and overriding destroy, BeforeDestruction and FreeInstance in the subclass only. This left the parent class as is. The cache contains instances of the subclass (which can't be freed), whereas objects outside the cache can be freed as per usual.

b) I do want the FlyweightFactory to Free the objects during finalization; I agree with Warren P that a "zero leaked memory" policy is best.

To solve this, I added a private boolean flag that has to be set to true before the object can be freed. This flag can only be set from the Cache Unit, it is not visible to other code. This means that the flag cannot be set outside by code outside the cache.

The destructor just looks like this:

destructor TCachedItem.destroy;
begin
    if destroyAllowed then
        inherited;
end;

If client code trys to Free a cached object, the call will have no effect.

awmross
  • 3,789
  • 3
  • 38
  • 51