This is the mail archive of the java-patches@gcc.gnu.org mailing list for the Java project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Checkbox states


>>>>> "Kim" == Kim Ho <kho@redhat.com> writes:

Kim> Fixed.

Looking good.  A few coding style nits, though...

First, a ChangeLog entry is required.

Kim>    // Group from last time it was set.
Kim>    public GtkCheckboxGroupPeer old_group;
 
Kim> -  public native void nativeCreate (GtkCheckboxGroupPeer group);
Kim> +  public native void nativeCreate (GtkCheckboxGroupPeer group, boolean state);

79 column limit... wrap after the ",".

Kim>    public native void nativeSetCheckboxGroup (GtkCheckboxGroupPeer group);
Kim>    public native void connectHooks ();
Kim> +  private boolean currentState;

Please group new fields with the other fields, and add a comment
explaining what the field is for.  old_group is an ok example to
follow.
 
Kim> +    // A firing of the event is only desired if the state has changed due to a button
Kim> +    // press. The currentCheckBox's state must be different from the one that the 
Kim> +    // stateChange is changing to. 

79 columns.

Kim> +    if ((!currentCheckBox.getState() && (stateChange==1)) || (currentCheckBox.getState() && (stateChange==2)))

Leave out the redundant parens, put whitespace around operators, and
wrap at 79 columns.  So:

  if ((! currentCheckBox.getState() && stateChange == 1)
      || (currentCheckBox.getState() && stateChange == 2))


With these changes, this is ok to commit.

Tom


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]