Smalltalk › Instantiations

Best practices question

_‹ Previous Topic Next Topic _›

Classic

List

Threaded

7 messages Options

Louis LaBrunda

Best practices question

HI,

I'm about to write a method where I need to test whether to do something or just get out. What is the best practices way to do this? For example:

(a = b) ifTrue: [^nil].

*The code that does the work*

or:

(a = b) ifFalse: [

*The code that does the work*

I think the second is better style but I have used both. Generally using the first when *The code that does the work* is long and the second when it is short.

Is there any speed difference between them? I doubt it but I thought I would ask anyway.

Lou

--
You received this message because you are subscribed to the Google Groups "VA Smalltalk" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
Visit this group at http://groups.google.com/group/va-smalltalk.
For more options, visit https://groups.google.com/groups/opt_out.

Dusty-2

Re: Best practices question

My 2c

I prefer the first method because you know you're getting nil back, and can test ifNil, ifNotNil, ifNilDo: etc...

The second returns the class that executes the code.

EG:

Object subclass: #BestPractice
    instanceVariableNames: ''
    classVariableNames: ''
    poolDictionaries: ''
    category: 'Examples'!

!BestPractice methodsFor: 'as yet unclassified' stamp: 'dusty 10/24/2013 12:22'!
return
    (1 = 1) ifFalse: [^'This method returns me']! !

!BestPractice methodsFor: 'as yet unclassified' stamp: 'dusty 10/24/2013 12:22'!
returnNil
    (1 = 1) ifTrue: [^nil].
    ^'This method returns nil'! !

Then in a workspace.

|anObject|
    anObject := BestPractice new.
    (anObject returnNil) inspect. "Returns nil"
    (anObject return) inspect. "Returns an instance of BestPractice"

So in the first instance, you can say, (anObject returnNil) isNil ifTrue: [do something else].

On Wed, Oct 23, 2013 at 4:10 PM, Louis LaBrunda <[hidden email]> wrote:

HI,

I'm about to write a method where I need to test whether to do something or just get out. What is the best practices way to do this? For example:

(a = b) ifTrue: [^nil].
*The code that does the work*

or:

(a = b) ifFalse: [

*The code that does the work*
].

I think the second is better style but I have used both. Generally using the first when *The code that does the work* is long and the second when it is short.

Is there any speed difference between them? I doubt it but I thought I would ask anyway.

Lou

--
You received this message because you are subscribed to the Google Groups "VA Smalltalk" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
Visit this group at http://groups.google.com/group/va-smalltalk.
For more options, visit https://groups.google.com/groups/opt_out.

Wayne Johnston

Re: Best practices question

In reply to this post by Louis LaBrunda

Louis, I agree with your conclusion. Of course if the return value matters as Dusty mentions, both forms can be made equivalent.

I thought someone might bring up 'Guarding clauses' as found by Smalllint. It looks like it checks for methods with an ifTrue or ifFalse, with two or more statements inside, and that's the end of the method. Personally I don't think it's worth pointing out code with just two statements inside, but just a data point for you.

John O'Keefe-3

Re: Best practices question

In reply to this post by Louis LaBrunda

Lou -

There are speed differences, but you would probably never notice them (and they depend on the specifics of your method).

The second piece of code will have fewer bytecodes since it doesn't have bytecodes for ^ nil. But it may have more bytes if the block is large since a 3 or 4-byte bytecode is used to jump around larger blocks rather than the 2-byte bytecode used for smaller blocks.

Answering self at the end of the method (either explicitly or implicitly) is preferred since it saves one push bytecode (self is pushed onto the stack as part of calling the method).

Of course, the 2 examples are not identical in semantics since the first answers either nil or self while the second always answers self.

So, I think it just comes down to style. If you are writing the code for yourself, use whatever style make you more comfortable.

John

On Wednesday, October 23, 2013 10:10:30 AM UTC-4, Louis LaBrunda wrote:

HI,

I'm about to write a method where I need to test whether to do something or just get out. What is the best practices way to do this? For example:

(a = b) ifTrue: [^nil].
*The code that does the work*

or:

(a = b) ifFalse: [

*The code that does the work*
].

I think the second is better style but I have used both. Generally using the first when *The code that does the work* is long and the second when it is short.

Is there any speed difference between them? I doubt it but I thought I would ask anyway.

Lou

Louis LaBrunda

Re: Best practices question

Thanks John and everyone for your answers.

I think I will stick with what I have been doing except I will return ^self unless I want to test for nil. My question really had little to do with the return value and more to do with should I get out right away or wrap the meat of the code in a block but it is good to know about the bytecodes generated for returns of nil and self.

As for the speed question, I was thinking of cases that might be handling many events, like mouse over stuff. Again wondering if it was faster to get out right away (not that getting out right away is really the case, just that it looks like it is) or fall into the block.

Lou

On Thursday, October 24, 2013 8:17:52 AM UTC-4, John O'Keefe wrote:

Lou -

There are speed differences, but you would probably never notice them (and they depend on the specifics of your method).

The second piece of code will have fewer bytecodes since it doesn't have bytecodes for ^ nil. But it may have more bytes if the block is large since a 3 or 4-byte bytecode is used to jump around larger blocks rather than the 2-byte bytecode used for smaller blocks.

Answering self at the end of the method (either explicitly or implicitly) is preferred since it saves one push bytecode (self is pushed onto the stack as part of calling the method).

Of course, the 2 examples are not identical in semantics since the first answers either nil or self while the second always answers self.

So, I think it just comes down to style. If you are writing the code for yourself, use whatever style make you more comfortable.

John

On Wednesday, October 23, 2013 10:10:30 AM UTC-4, Louis LaBrunda wrote:
HI,

I'm about to write a method where I need to test whether to do something or just get out. What is the best practices way to do this? For example:

(a = b) ifTrue: [^nil].
*The code that does the work*

or:

(a = b) ifFalse: [

*The code that does the work*
].

I think the second is better style but I have used both. Generally using the first when *The code that does the work* is long and the second when it is short.

Is there any speed difference between them? I doubt it but I thought I would ask anyway.

Lou

Richard Sargent

Re: Best practices question

Administrator

On Thursday, October 24, 2013 9:21:20 AM UTC-7, Louis LaBrunda wrote:

I think I will stick with what I have been doing except I will return ^self unless I want to test for nil. My question really had little to do with the return value and more to do with should I get out right away or wrap the meat of the code in a block but it is good to know about the bytecodes generated for returns of nil and self.

Don't return and test for nil. If you want to handle the case of the method actually doing what it is intended to do versus not doing it, follow the pattern of #at:ifAbsent:, #detect:ifNone:, etc. e.g. implement #doSomethingOr: in which the Block argument is evaluated in the case the method is skipped.

Passing around nil might be justified when absolute performance is critical, but I advocate against sacrificing readability before it is necessary. (And having a clean value set for the return from a method is also very important: adding nil to the mix makes everyone's life worse.)

Thomas Koschate-2

Re: Best practices question

In reply to this post by Louis LaBrunda

On Wednesday, October 23, 2013 10:10:30 AM UTC-4, Louis LaBrunda wrote:

(a = b) ifTrue: [^nil].*The code that does the work*

or:

(a = b) ifFalse: [

*The code that does the work*
].

If you run the various lint and code quality tests, you'll find they prefer the first form rather than the second.

And, just as a general rule, I avoid the second form like crazy because of the ifFalse:. I'd much prefer to see something like

(a ~= b) ifTrue: [...

I'm just not wired to try to figure out if something is true and then reverse it - it gets really crazy if you throw an or: into the mix!

If your code truly cares about the result of the method execution, then you might want to think about what Richard has to say.

Tom