Hi Jonathan,
as always, a deeper review of your patch will have to wait ('til next
week), but here are some short comments already ...
> Fixed. Though one thing I don't quite understand is why
> ORadioButtonModel::_propertyChanged() can't call
> OReferenceValueComponent::_propertyChanged() when the
> PROPERTY_GROUP_NAME property is changed. If I do call the
> OReferenceValueComponent version, I get a spew of warnings beginning
> with:
>
> Error:
> File
> /home/jon/Development/OpenOffice.org/dev300-m10/forms/source/component/FormComponent.cxx,
> Line 1404: OBoundControlModel::_propertyChanged: where did this come from
> (1)?
Well, the base class code is not intended to be fed with properties it
cannot handle itself. I am not really sure if this pattern is a good
thing all the time (there are counter-measures), but I use it from time
to time. This ensures that half-done changes in the code (i.e. adding as
listener to a new property, in either the base or a derived class, but
not handling it in _propertyChanged) are recognized early.
Just don't call the base class in the GroupName case, and everything
should be fine.
>> - UnoControlDialogModel::AddRadioButtonToGroup
>> - pGroups is superfluous
>> + the method name is misleading - it suggests the radio button is
>> actually added, which is not the case.
>> - pNamedGroups and pCurGroup should be reference parameters, not
>> pointers, since they cannot be NULL
>
> Fixed? It's a large-ish refactor, so opinions may vary.
Sure - that's why I put a "-" in front of this item :)
Personally, I learned in the past (by getting issues for crash reports
sent by users) that there is no such code where "this pointer is never
NULL". You always will have such a case, and be it because there's a
refactoring two years later which silently changes some invariant of the
class.
So, a habit of mine is to not use pointers, but references, when "it
cannot be NULL". It's just a matter of robust code - whoever changes the
thing so it *can* be NULL will be forced by the compiler to do the
complete fix.
(well - I saw code in OOo
void foo( Bar& bar ) { if ( &bar == NULL ) ... }
but that's definitely something the developer should be shot for.)
>> + there's stil no code for *reading* the GroupName property of a radio
>> button in a form
>
> Is this the "need to save/load the GroupName form property into the ODF
> file" issue? As this still needs to be implemented...
You already save it, don't you? Reading shouldn't be too difficult, IIRC
I pointed you to the respective code in one of my previous mails.
Ciao
Frank
--
- Frank Schönheit, Software Engineer [EMAIL PROTECTED] -
- Sun Microsystems http://www.sun.com/staroffice -
- OpenOffice.org Base http://dba.openoffice.org -
- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
