What do you think about this idea ?

What do you think about this idea ?

Replace
Control.ParentFont : Boolean

by

Control.ParentFont.Name : Boolean
Control.ParentFont.Size : Boolean
Control.ParentFont.Color: Boolean

with a RecordProperty instead of a new TParentFont = class(TPersistent) of course !

https://github.com/tothpaul/DelphiTips/tree/master/RecordProperty
https://github.com/tothpaul/DelphiTips/tree/master/RecordProperty

Comments

  1. That was what I wanted for like 2 decades.

    ReplyDelete
  2. Bad idea because records are value types and thus assigning anything to Control.ParentFont.Name will not assign to the parentfont record in the control but to the copy being returned by the ParentFont property if its a getter.

    A better approach would be to implement ref returns into the language where you can specifiy that the ParentFont property will not cause a record copy but return a reference without all the nastiness of what a true ^TParentFont pointer would cause - this would also improve code in other areas like enable to directly set record fields in a list (see https://blogs.msdn.microsoft.com/mazhou/2017/12/12/c-7-series-part-7-ref-returns/)

    Also your naming is as misleading as the original one. Why is Name, Size and Color of type Boolean?

    ReplyDelete
  3. Stefan Glienke False, you didn't read the repository link :)

    because the property name is ParentFont and we use it for decades as a boolean.

    you could also define Font.Size := inherited be it's more difficult to implement.

    ReplyDelete
  4. I did read it and I think it's a brittle hack around a shortcoming of the language.

    Yes, the fact that the boolean property was named ParentFont instead of something like UseParentFont is bad.

    But I was commenting on your proposed names: Name, Size and Color (which I would imagine to be string, Integer and TColor). What you mean though is InheritName or UseName or something similar.

    And if you think about that how would you name the aggregation of those properties? ParentFontInheritanceOptions or something like that (I know its ugly and long but it certainly describes it better)

    Anyhow if they are all Boolean we already have a solution for that and even proper support in the Object Inspector: enumset.

    ReplyDelete
  5. Stefan Glienke until I can fix the compiler myself, I only can do brillant hacks :)

    well, In fact I just think faster then I write.

    we have 20 years of projects with a ParentFont: Boolean property, how can we turn this in something more involved without breaking habits and code ?

    replace the existing ParentFont property by something that holds more informations, and define a custom property that handle both cases, old DFM with a Boolean value, new DFM with a complex property.

    a DFM with a ParentFont = True will turn into a component that inherit from all the parent's font properties. when Parent = False it use it's own Font...that is for compatibility...then you can have custom properties to specify that you want change only the size, the attributs (forgot that one), font name or color.

    BTW, I hope I'll never read you a bad day; instead of talking about the possibility to inherited parts of the parent font and how it could be defined, you're just growling agains my code.

    ReplyDelete
  6. Paul TOTH You posted this under "Delphi Feature Requests" with the question what we think about this and so I explained my concerns - this is nothing personal.

    As for this particular problem:

    Add a new property called ParentFontOptions (not perfect but I don't spend more time on thinking a better name right now) of type TParentFontOptions = set of TParentFontOption

    TParentFontOption would have the members InheritName, InheritSize and InheritColor. (again just quickly wrote them down, could put more time into naming)

    That way you get perfect support in the object inspector without any hacking.

    The migration path has to be determined yet (what default values to be chosen when the ParentFont property is True or False and all that)

    ReplyDelete
  7. Stefan Glienke well, you're right on one point, no need of a record property for a set of boolean, and I did not think much on the problem either, that's why it's a post here instead of a QP entry...I've think about a record property because I had in mind the CSS properties, but in fact a set of boolean are sufficient.

    ReplyDelete
  8. Did you enter a feature request for this?

    ReplyDelete
  9. David Millington no, I talked about it here to see if it was a good idea, and I let it go in response

    ReplyDelete

Post a Comment