View Issue Details

IDProjectCategoryView StatusLast Update
0002312JEDI VCL00 JVCL Componentspublic2004-11-23 07:11
ReporterglchapmanAssigned Touser72 
Status resolvedResolutionwon't fix 
Product Version3.00 BETA 2 
Target VersionFixed in Version 
Summary0002312: Some Jvg properties of object-type directly write to fields
DescriptionSome properties in the Jvg family of controls are declared like this:

  property Bevel: TJvgBevelOptions read FBevel write FBevel;

where TJvgBevelOptions is a class inheriting from TPersistent, and FBevel is an instance of the class created and freed by the object declaring the property. I believe directly exposing object-type fields to writes like this is considered bad practice (since it can lead to leaks or access violations). Instead, the property should have a write method which uses an overridden Assign method to assign the passed-in value to the (field) object.

In the attached zip are two diffs showing changes to fix this problem in JvgCommClasses and JvgSpeedButton (also copies of the changed units). I think similar changes will have to be made in the other Jvg units if you agree that this is a problem which should be fixed.
Additional InformationThe Assign methods do not assign the value of the OnChanged event, since this is generally used to inform the owning control owning that some part of it has changed. Also, the write methods do not call Changed. This is because it looks to me that, in the cases here, Changed will be called when the individual properties are changed (during Assign).
TagsNo tags attached.


2004-11-12 07:11 (14,775 bytes)


2004-11-13 14:17


> I think similar changes will have to be made in
> the other Jvg units if you agree that this is
> a problem which should be fixed.
Most definitely. Do you have a list of candidates?


2004-11-14 11:37

reporter   ~0005627

I don't have a list, but after a quick glance through some of the units, it appears that all properties whose type is one of the types in JvgCommClasses are broken. Also, I noted several component-type properties which do not register for FreeNotification with the components they are referencing (e.g., WallpaperImage in TJvgAskListBox).

After looking at this, I realized my initial set of changes to JvgCommClasses was not thorough enough; all the classes need Assign methods in preparation for adding in the property write methods. So attached is a new zip file with a new version of JvgCommClasses (JvgSpeedButton.pas is still included, but it is unchanged). In general, the Assign methods now directly assign to the fields (except where the fields are objects), and then call Changed at the end -- it seemed like it was probably a good idea to limit the number of Changed events. (I also had broken TJvgTwainColors by assigning to the RGB properties rather than FromColor and ToColor; that's now fixed.)

Do you want me to try to work on a more extensive patch?

2004-11-14 11:37 (17,846 bytes)


2004-11-15 11:42


> Do you want me to try to work on a more extensive patch?
That would be greatly appreciated


2004-11-22 11:37

viewer   ~0005688

Well, I started working on a larger patch, but I'm stopping now: there are too many problems with the Jvg units. Aside from the problems mentioned earlier, several components make use of a pattern where they have a private TBitmap field which is sometimes used to refer to Image.Picture.Bitmap, where Image is the value of a TImage property (e.g., TJvgAskListBox, which sets WallpaperBmp to FWallpaperImage.Picture.Bitmap). This means that if anything changes the Image's Picture, the components are left with a stray pointer.

Also, there is frequent use of:


Much of the time, it appears this is used when a string to numeric conversion might fail (e.g., the TJvgPrintCrossTable.CalcResults monstrosity), but of course another exception may have been anticipated. Who knows?

Also, JvgHint seems to have misunderstood the VCL hint system (unless something major changed after D5). It sets up TJvgHint.NewShowHint as the Application.OnShowHint handler, then ends up creating and showing a new hint window every time this is called; as far as I can tell, these hint windows are not freed until application shutdown.

Also, JvgButton uses a cumbersome method of iterating all components owned by a form every time an OnBlinkTimer event occurs. Unfotunately, this won't work if the button is on a frame; then it will be owned by the frame, not the parent form.

Anyway, I've decided not to use the Jvg units anymore, so I won't be submitting a further patch.


2004-11-23 07:11


Fair enough. If anyone is keen on cleaning up the Globus units, you are welcome but I'll leave this issue now as well.

Issue History

Date Modified Username Field Change
2004-11-12 07:11 glchapman New Issue
2004-11-12 07:11 glchapman File Added:
2004-11-13 14:17 user72 Note Added: 0005622
2004-11-13 14:17 user72 Status new => feedback
2004-11-14 11:37 glchapman Note Added: 0005627
2004-11-14 11:37 glchapman File Added:
2004-11-15 11:42 user72 Note Added: 0005632
2004-11-22 11:37 anonymous Note Added: 0005688
2004-11-23 07:11 user72 Status feedback => resolved
2004-11-23 07:11 user72 Resolution open => won't fix
2004-11-23 07:11 user72 Assigned To => user72
2004-11-23 07:11 user72 Note Added: 0005692