Re: comtctl32 : updown : Getter and Setter Tests for Updown control

[James Hawkins]

On 2/10/07, Leslie Choong  wrote:
> Hi there, My name is Leslie Choong and I am currently finishing up my
> under graduate work at UCLA. This patch is being sent as part of the
> coursework for CS 130 : Software Engineering. Please take a look and
> let me know if you have any suggestions or comments for change.
> Thanks!
> -Leslie Choong

+    /* Set Range from 0 to 100 */
+    r = SendMessage(updown, UDM_SETRANGE, 0 , MAKELONG(100,0) );
+    r= SendMessage(updown, UDM_GETRANGE, 0,0);

If you're not going to check the first return value, then take out the
'r =', else it seems like you're missing a test.  On the other hand,
why don't you test the return value?

comtctl32 : updown : Getter and Setter Tests for Updown control

[Leslie Choong]

Thanks for the input. UDM_SETRANGE has no return value according to
MSDN so I'll remove those assignments.

On 2/11/07, James Hawkins  wrote:
> On 2/10/07, Leslie Choong  wrote:
> > Hi there, My name is Leslie Choong and I am currently finishing up my
> > under graduate work at UCLA. This patch is being sent as part of the
> > coursework for CS 130 : Software Engineering. Please take a look and
> > let me know if you have any suggestions or comments for change.
> > Thanks!
> > -Leslie Choong
> - /* Set Range from 0 to 100 */
> - r = SendMessage(updown, UDM_SETRANGE, 0 , MAKELONG(100,0) );
> - r= SendMessage(updown, UDM_GETRANGE, 0,0);
>
> If you're not going to check the first return value, then take out the
> 'r =', else it seems like you're missing a test.  On the other hand,
> why don't you test the return value?
>
> --
> James Hawkins

comtctl32 : updown : Getter and Setter Tests for Updown control

[Leslie Choong]

Hey there, I've made the changes so that the return value is not
stored unnecessarily. I've also added comments explaining why. Let me
know what you think.
-Leslie

On 2/11/07, Leslie Choong  wrote:
> Thanks for the input. UDM_SETRANGE has no return value according to
> MSDN so I'll remove those assignments.
>
> On 2/11/07, James Hawkins  wrote:
> > On 2/10/07, Leslie Choong  wrote:
> > > Hi there, My name is Leslie Choong and I am currently finishing up my
> > > under graduate work at UCLA. This patch is being sent as part of the
> > > coursework for CS 130 : Software Engineering. Please take a look and
> > > let me know if you have any suggestions or comments for change.
> > > Thanks!
> > > -Leslie Choong
> > - /* Set Range from 0 to 100 */
> > - r = SendMessage(updown, UDM_SETRANGE, 0 , MAKELONG(100,0) );
> > - r= SendMessage(updown, UDM_GETRANGE, 0,0);
> >
> > If you're not going to check the first return value, then take out the
> > 'r =', else it seems like you're missing a test.  On the other hand,
> > why don't you test the return value?
> >
> > --
> > James Hawkins

comtctl32 : updown : Getter and Setter Tests for Updown control

[Dan Kegel]

I haven't looked at what James was talking about, but I noticed three things:

You have a typo 'sohuld'.

You should use the same filename as before, but increment the number
in the filename.

You might consider using the expect() macro I've been encouraging
the other cs130 students to use, it makes the repeated ok() calls a bit
more readable (IMHO, we'll see if others agree).
- Dan

On 2/11/07, Leslie Choong  wrote:
> Hey there, I've made the changes so that the return value is not
> stored unnecessarily. I've also added comments explaining why. Let me
> know what you think.
> -Leslie
>
> On 2/11/07, Leslie Choong  wrote:
> > Thanks for the input. UDM_SETRANGE has no return value according to
> > MSDN so I'll remove those assignments.
> >
> > On 2/11/07, James Hawkins  wrote:
> > > On 2/10/07, Leslie Choong  wrote:
> > > > Hi there, My name is Leslie Choong and I am currently finishing up my
> > > > under graduate work at UCLA. This patch is being sent as part of the
> > > > coursework for CS 130 : Software Engineering. Please take a look and
> > > > let me know if you have any suggestions or comments for change.
> > > > Thanks!
> > > > -Leslie Choong
> > > - /* Set Range from 0 to 100 */
> > > - r = SendMessage(updown, UDM_SETRANGE, 0 , MAKELONG(100,0) );
> > > - r= SendMessage(updown, UDM_GETRANGE, 0,0);
> > >
> > > If you're not going to check the first return value, then take out the
> > > 'r =', else it seems like you're missing a test.  On the other hand,
> > > why don't you test the return value?
> > >
> > > --
> > > James Hawkins

comtctl32 : updown : Getter and Setter Tests for Updown control

[Leslie Choong]

Thanks for the input, I've added the expect() macro, corrected some
spelling mistakes. Let me know what else I can do.
-Leslie

On 2/11/07, Dan Kegel  wrote:
> I haven't looked at what James was talking about, but I noticed three things:
>
> You have a typo 'sohuld'.
>
> You should use the same filename as before, but increment the number
> in the filename.
>
> You might consider using the expect() macro I've been encouraging
> the other cs130 students to use, it makes the repeated ok() calls a bit
> more readable (IMHO, we'll see if others agree).
> - Dan
>
> On 2/11/07, Leslie Choong  wrote:
> > Hey there, I've made the changes so that the return value is not
> > stored unnecessarily. I've also added comments explaining why. Let me
> > know what you think.
> > -Leslie
> >
> > On 2/11/07, Leslie Choong  wrote:
> > > Thanks for the input. UDM_SETRANGE has no return value according to
> > > MSDN so I'll remove those assignments.
> > >
> > > On 2/11/07, James Hawkins  wrote:
> > > > On 2/10/07, Leslie Choong  wrote:
> > > > > Hi there, My name is Leslie Choong and I am currently finishing up my
> > > > > under graduate work at UCLA. This patch is being sent as part of the
> > > > > coursework for CS 130 : Software Engineering. Please take a look and
> > > > > let me know if you have any suggestions or comments for change.
> > > > > Thanks!
> > > > > -Leslie Choong
> > > > - /* Set Range from 0 to 100 */
> > > > - r = SendMessage(updown, UDM_SETRANGE, 0 , MAKELONG(100,0) );
> > > > - r= SendMessage(updown, UDM_GETRANGE, 0,0);
> > > >
> > > > If you're not going to check the first return value, then take out the
> > > > 'r =', else it seems like you're missing a test.  On the other hand,
> > > > why don't you test the return value?
> > > >
> > > > --
> > > > James Hawkins
>
> --
> Wine for Windows ISVs: http://kegel.com/wine/isv