Smalltalk › Pharo › Pharo Smalltalk Developers

Catching Exceptions without any notice

_‹ Previous Topic Next Topic _›

Classic

List

Threaded

35 messages Options

Nicolai Hess-3-2

Catching Exceptions without any notice

Please don't do this:

updateHeight
    "no need to care about height, when it's logic is not customized"
    self layout isHeightCustom ifFalse: [ ^ self ].
    [ self bounds: (self brickBounds withHeight: self customHeight) ]
        on: Exception
        do: [ "just skip and do nothing" ]

This makes debugging GLM/Brick ui/layout code with "self haltOnce" impossible.
see
GLMBrickGeometryTrait>>#updateHeight
GLMBrickGeometryTrait>>#updateWidth

And if you log out the raised exception, you see some calls to

not initialized fonts and a ZeroDevide and some more errors.

The above catch, catches and hides wrong / to late initialized objects.

Nicolas Cellier

Re: Catching Exceptions without any notice

Isn't catching too wide an anti-pattern?

2016-03-30 13:33 GMT+02:00 Nicolai Hess <[hidden email]>:

Please don't do this:

updateHeight
    "no need to care about height, when it's logic is not customized"
    self layout isHeightCustom ifFalse: [ ^ self ].
    [ self bounds: (self brickBounds withHeight: self customHeight) ]
        on: Exception
        do: [ "just skip and do nothing" ]

This makes debugging GLM/Brick ui/layout code with "self haltOnce" impossible.
see
GLMBrickGeometryTrait>>#updateHeight
GLMBrickGeometryTrait>>#updateWidth

And if you log out the raised exception, you see some calls to
not initialized fonts and a ZeroDevide and some more errors.
The above catch, catches and hides wrong / to late initialized objects.

Richard Sargent

Re: Catching Exceptions without any notice

Administrator

Nicolas Cellier wrote

Isn't catching too wide an anti-pattern?

Yes!
Just think of all the Exception subclasses which are not errors. This kind of thing effectively disables them.

2016-03-30 13:33 GMT+02:00 Nicolai Hess <[hidden email]>:

> Please don't do this:
>
> updateHeight
> "no need to care about height, when it's logic is not customized"
> self layout isHeightCustom ifFalse: [ ^ self ].
> [ self bounds: (self brickBounds withHeight: self customHeight) ]
> on: Exception
> do: [ "just skip and do nothing" ]
>
> This makes debugging GLM/Brick ui/layout code with "self haltOnce"
> impossible.
> see
> GLMBrickGeometryTrait>>#updateHeight
> GLMBrickGeometryTrait>>#updateWidth
>
> And if you log out the raised exception, you see some calls to
> not initialized fonts and a ZeroDevide and some more errors.
> The above catch, catches and hides wrong / to late initialized objects.
>

Aliaksei Syrel

Re: Catching Exceptions without any notice

In reply to this post by Nicolai Hess-3-2

Do anyone really use GLM-Morphic-Brick?

Because it is there just for spotter and will be deleted ASAP.

We know that it is bad. Normal fix requires too many changes...

On Mar 30, 2016 1:34 PM, "Nicolai Hess" <[hidden email]> wrote:

Please don't do this:

updateHeight
    "no need to care about height, when it's logic is not customized"
    self layout isHeightCustom ifFalse: [ ^ self ].
    [ self bounds: (self brickBounds withHeight: self customHeight) ]
        on: Exception
        do: [ "just skip and do nothing" ]

This makes debugging GLM/Brick ui/layout code with "self haltOnce" impossible.
see
GLMBrickGeometryTrait>>#updateHeight
GLMBrickGeometryTrait>>#updateWidth

And if you log out the raised exception, you see some calls to
not initialized fonts and a ZeroDevide and some more errors.
The above catch, catches and hides wrong / to late initialized objects.

Nicolai Hess-3-2

Re: Catching Exceptions without any notice

2016-03-30 19:54 GMT+02:00 Aliaksei Syrel <[hidden email]>:

Do anyone really use GLM-Morphic-Brick?

We all "use" it, our tools are build with this. (inspector/debugger/playground/spotter).

Because it is there just for spotter and will be deleted ASAP.

Not only for spotter. And if our tools are build with this, people may try to use this

for their own tools. This is the core image, and the core tools. What should we

say?

Please don't use it, but our core tools are built on this?

We know that it is bad. Normal fix requires too many changes...

No excuse.

On Mar 30, 2016 1:34 PM, "Nicolai Hess" <[hidden email]> wrote:
Please don't do this:

updateHeight
    "no need to care about height, when it's logic is not customized"
    self layout isHeightCustom ifFalse: [ ^ self ].
    [ self bounds: (self brickBounds withHeight: self customHeight) ]
        on: Exception
        do: [ "just skip and do nothing" ]

This makes debugging GLM/Brick ui/layout code with "self haltOnce" impossible.
see
GLMBrickGeometryTrait>>#updateHeight
GLMBrickGeometryTrait>>#updateWidth

And if you log out the raised exception, you see some calls to
not initialized fonts and a ZeroDevide and some more errors.
The above catch, catches and hides wrong / to late initialized objects.

Tudor Girba-2

Re: Catching Exceptions without any notice

Hi Nicolai,

Thank you for the feedback. Just, please let’s adopt a different tone.

Cheers,
Doru

> On Mar 31, 2016, at 8:37 AM, Nicolai Hess <[hidden email]> wrote:
>
>
>
> 2016-03-30 19:54 GMT+02:00 Aliaksei Syrel <[hidden email]>:
> Do anyone really use GLM-Morphic-Brick?
>
> We all "use" it, our tools are build with this. (inspector/debugger/playground/spotter).
>
> Because it is there just for spotter and will be deleted ASAP.
>
>
> Not only for spotter. And if our tools are build with this, people may try to use this
> for their own tools. This is the core image, and the core tools. What should we
> say?
> Please don't use it, but our core tools are built on this?
>
>
> We know that it is bad. Normal fix requires too many changes...
>
>
> No excuse.
>
>
> On Mar 30, 2016 1:34 PM, "Nicolai Hess" <[hidden email]> wrote:
> Please don't do this:
>
> updateHeight
> "no need to care about height, when it's logic is not customized"
> self layout isHeightCustom ifFalse: [ ^ self ].
> [ self bounds: (self brickBounds withHeight: self customHeight) ]
> on: Exception
> do: [ "just skip and do nothing" ]
>
> This makes debugging GLM/Brick ui/layout code with "self haltOnce" impossible.
> see
> GLMBrickGeometryTrait>>#updateHeight
> GLMBrickGeometryTrait>>#updateWidth
>
> And if you log out the raised exception, you see some calls to
> not initialized fonts and a ZeroDevide and some more errors.
> The above catch, catches and hides wrong / to late initialized objects.
>

--
www.tudorgirba.com
www.feenk.com

“Live like you mean it."

Nicolai Hess-3-2

Re: Catching Exceptions without any notice

2016-03-31 10:51 GMT+02:00 Tudor Girba <[hidden email]>:

Hi Nicolai,

Thank you for the feedback. Just, please let’s adopt a different tone.

Cheers,
Doru

I am not sure what is wrong with my tone.

I did not choose a random external library and starting to criticize it.

This is a core component. And

"Because it is there just for spotter and will be deleted ASAP.

We know that it is bad. Normal fix requires too many changes."

is not an excuse for badly debuggable code.
I don't like to have quick-dirty-throwaway-hacks in the core.

There is a real demand for debugging this code.

The attached screenshot shows a common issue with inspector panes.

The available space would suffice for all tabs (secodn window) but the default

size shnrinks nearly all tabs (first window).

The normal way to understand and trying to fix this issue, is to put some

"self halt" at some point for the layout processing.But this does not work here

because *all* exceptions are catched.

And changing the code to actually execute the exception won't work either, becuase this

will throw all other (previously cachted) exceptions (what is wrong with initializing the font property properly or

checking for ZeroDivide?)

And this is frustrating.

And I hope no one sees and adopts this code, just because it is the way our own

core tools are built.

BTW, your LazyTabGroupMorph class is another bad example. It subclasses
TabGroupMorph, it
calls super initialize,

removes all created and added morphs from its super class and

recreates and replaces them with its own.

nicolai

PharoScreenshot.2.png (34K) Download Attachment

EstebanLM

Re: Catching Exceptions without any notice

In reply to this post by Aliaksei Syrel

On 30 Mar 2016, at 19:54, Aliaksei Syrel <[hidden email]> wrote:

Do anyone really use GLM-Morphic-Brick?
Because it is there just for spotter and will be deleted ASAP.
We know that it is bad. Normal fix requires too many changes...

but catching Exception is bad because it will catch all, even Warnings and Halts.

It would be less bad if it catches Error (not Exception). Still bad, but less bad :)

Esteban

On Mar 30, 2016 1:34 PM, "Nicolai Hess" <[hidden email]> wrote:
Please don't do this:

updateHeight
    "no need to care about height, when it's logic is not customized"
    self layout isHeightCustom ifFalse: [ ^ self ].
    [ self bounds: (self brickBounds withHeight: self customHeight) ]
        on: Exception
        do: [ "just skip and do nothing" ]

This makes debugging GLM/Brick ui/layout code with "self haltOnce" impossible.
see
GLMBrickGeometryTrait>>#updateHeight
GLMBrickGeometryTrait>>#updateWidth

And if you log out the raised exception, you see some calls to
not initialized fonts and a ZeroDevide and some more errors.
The above catch, catches and hides wrong / to late initialized objects.

gcotelli

Re: Catching Exceptions without any notice

On the meantime at least change the handler to ignore the halts.

... on: Exception - Halt

Should do the trick

On Mar 31, 2016 06:57, "Esteban Lorenzano" <[hidden email]> wrote:

On 30 Mar 2016, at 19:54, Aliaksei Syrel <[hidden email]> wrote:

Do anyone really use GLM-Morphic-Brick?
Because it is there just for spotter and will be deleted ASAP.
We know that it is bad. Normal fix requires too many changes...
but catching Exception is bad because it will catch all, even Warnings and Halts.
It would be less bad if it catches Error (not Exception). Still bad, but less bad :)

Esteban

On Mar 30, 2016 1:34 PM, "Nicolai Hess" <[hidden email]> wrote:
Please don't do this:

updateHeight
    "no need to care about height, when it's logic is not customized"
    self layout isHeightCustom ifFalse: [ ^ self ].
    [ self bounds: (self brickBounds withHeight: self customHeight) ]
        on: Exception
        do: [ "just skip and do nothing" ]

This makes debugging GLM/Brick ui/layout code with "self haltOnce" impossible.
see
GLMBrickGeometryTrait>>#updateHeight
GLMBrickGeometryTrait>>#updateWidth

And if you log out the raised exception, you see some calls to
not initialized fonts and a ZeroDevide and some more errors.
The above catch, catches and hides wrong / to late initialized objects.

Eliot Miranda-2

Re: Catching Exceptions without any notice

On Mar 31, 2016, at 3:51 AM, Gabriel Cotelli <[hidden email]> wrote:

On the meantime at least change the handler to ignore the halts.

... on: Exception - Halt

Should do the trick

Can someone say why

... on: Error

will not improve things?

Exception - Halt is just /wrong/, less wrong than Exception but still very wrong.

_,,,^..^,,,_ (phone)

On Mar 31, 2016 06:57, "Esteban Lorenzano" <[hidden email]> wrote:

On 30 Mar 2016, at 19:54, Aliaksei Syrel <[hidden email]> wrote:

Do anyone really use GLM-Morphic-Brick?
Because it is there just for spotter and will be deleted ASAP.
We know that it is bad. Normal fix requires too many changes...
but catching Exception is bad because it will catch all, even Warnings and Halts.
It would be less bad if it catches Error (not Exception). Still bad, but less bad :)

Esteban

On Mar 30, 2016 1:34 PM, "Nicolai Hess" <[hidden email]> wrote:
Please don't do this:

updateHeight
    "no need to care about height, when it's logic is not customized"
    self layout isHeightCustom ifFalse: [ ^ self ].
    [ self bounds: (self brickBounds withHeight: self customHeight) ]
        on: Exception
        do: [ "just skip and do nothing" ]

This makes debugging GLM/Brick ui/layout code with "self haltOnce" impossible.
see
GLMBrickGeometryTrait>>#updateHeight
GLMBrickGeometryTrait>>#updateWidth

And if you log out the raised exception, you see some calls to
not initialized fonts and a ZeroDevide and some more errors.
The above catch, catches and hides wrong / to late initialized objects.

gcotelli

Re: Catching Exceptions without any notice

Handling Error will be an improvement. But we don't know the rationale for handling Exception in the first place, if it's for ignoring some Warnings and Errors then changing the handler to Error would break things. I was trying to propose the less risky change, but I'm with you. The right solution is not to make the handler too broad, even handling Error is a bad smell.

On Thu, Mar 31, 2016 at 11:22 AM, Eliot Miranda <[hidden email]> wrote:

On Mar 31, 2016, at 3:51 AM, Gabriel Cotelli <[hidden email]> wrote:

On the meantime at least change the handler to ignore the halts.

... on: Exception - Halt

Should do the trick
Can someone say why
... on: Error
will not improve things?

Exception - Halt is just /wrong/, less wrong than Exception but still very wrong.

_,,,^..^,,,_ (phone)

On Mar 31, 2016 06:57, "Esteban Lorenzano" <[hidden email]> wrote:

On 30 Mar 2016, at 19:54, Aliaksei Syrel <[hidden email]> wrote:

Do anyone really use GLM-Morphic-Brick?
Because it is there just for spotter and will be deleted ASAP.
We know that it is bad. Normal fix requires too many changes...
but catching Exception is bad because it will catch all, even Warnings and Halts.
It would be less bad if it catches Error (not Exception). Still bad, but less bad :)

Esteban

On Mar 30, 2016 1:34 PM, "Nicolai Hess" <[hidden email]> wrote:
Please don't do this:

updateHeight
    "no need to care about height, when it's logic is not customized"
    self layout isHeightCustom ifFalse: [ ^ self ].
    [ self bounds: (self brickBounds withHeight: self customHeight) ]
        on: Exception
        do: [ "just skip and do nothing" ]

This makes debugging GLM/Brick ui/layout code with "self haltOnce" impossible.
see
GLMBrickGeometryTrait>>#updateHeight
GLMBrickGeometryTrait>>#updateWidth

And if you log out the raised exception, you see some calls to
not initialized fonts and a ZeroDevide and some more errors.
The above catch, catches and hides wrong / to late initialized objects.

Igor Stasenko

Re: Catching Exceptions without any notice

In reply to this post by Nicolai Hess-3-2

A perfect example of careless programming.
"I'll do it my way, and if it causing any problems, i don't care and i will just ignore them. And it's not my problem anyways, i went to something else already, since this part is works and DONE"
:)

On 30 March 2016 at 14:33, Nicolai Hess <[hidden email]> wrote:

Please don't do this:

updateHeight
    "no need to care about height, when it's logic is not customized"
    self layout isHeightCustom ifFalse: [ ^ self ].
    [ self bounds: (self brickBounds withHeight: self customHeight) ]
        on: Exception
        do: [ "just skip and do nothing" ]

This makes debugging GLM/Brick ui/layout code with "self haltOnce" impossible.
see
GLMBrickGeometryTrait>>#updateHeight
GLMBrickGeometryTrait>>#updateWidth

And if you log out the raised exception, you see some calls to
not initialized fonts and a ZeroDevide and some more errors.
The above catch, catches and hides wrong / to late initialized objects.

Best regards,
Igor Stasenko.

Tudor Girba-2

Re: Catching Exceptions without any notice

Hi,

Nice to hear from you Igor. I am glad to see you around here.

I do not see how your quote applies to the current case given that the original authors did not leave anywhere, but perhaps it was a joke, and I did not get it.

Cheers,
Doru

> On Apr 1, 2016, at 5:54 AM, Igor Stasenko <[hidden email]> wrote:
>
> A perfect example of careless programming.
> "I'll do it my way, and if it causing any problems, i don't care and i will just ignore them. And it's not my problem anyways, i went to something else already, since this part is works and DONE"
> :)
>
> On 30 March 2016 at 14:33, Nicolai Hess <[hidden email]> wrote:
> Please don't do this:
>
> updateHeight
> "no need to care about height, when it's logic is not customized"
> self layout isHeightCustom ifFalse: [ ^ self ].
> [ self bounds: (self brickBounds withHeight: self customHeight) ]
> on: Exception
> do: [ "just skip and do nothing" ]
>
> This makes debugging GLM/Brick ui/layout code with "self haltOnce" impossible.
> see
> GLMBrickGeometryTrait>>#updateHeight
> GLMBrickGeometryTrait>>#updateWidth
>
> And if you log out the raised exception, you see some calls to
> not initialized fonts and a ZeroDevide and some more errors.
> The above catch, catches and hides wrong / to late initialized objects.
>
>
>
> --
> Best regards,
> Igor Stasenko.

--
www.tudorgirba.com
www.feenk.com

"No matter how many recipes we know, we still value a chef."

stepharo

Re: Catching Exceptions without any notice

In reply to this post by Igor Stasenko

Hi

I do not think that aliaksei is not taking care.
He is working a lot to make sure that Pharo will have a future from the UI perspective. He is spending all his free time
on Bloc and what he is doing is massive. He rewrote the AthensCairoBack end to optimise it for Bloc local coordinates.
I'm sure that if someone would have propose some help to him he would have gladly accepted.

I personnaly thank him for all the energy that he is putting into Bloc. The talk at PharoDays was showing really nice results.
Bloc is really getting to be really nice and change the face of Pharo for the next decades so we should encourage people doing
Bloc!

So instead of bashing someone, I would prefer that we help him.
Yes Pharo has some code that we do not like and I have my own list. Now the best things to do is to
improve code review and to fix when we see something wrong.

Stef

Le 1/4/16 05:54, Igor Stasenko a écrit :

A perfect example of careless programming.
"I'll do it my way, and if it causing any problems, i don't care and i will just ignore them. And it's not my problem anyways, i went to something else already, since this part is works and DONE"
:)

On 30 March 2016 at 14:33, Nicolai Hess <[hidden email]> wrote:

Please don't do this:

updateHeight
    "no need to care about height, when it's logic is not customized"
    self layout isHeightCustom ifFalse: [ ^ self ].
    [ self bounds: (self brickBounds withHeight: self customHeight) ]
        on: Exception
        do: [ "just skip and do nothing" ]

This makes debugging GLM/Brick ui/layout code with "self haltOnce" impossible.
see
GLMBrickGeometryTrait>>#updateHeight
GLMBrickGeometryTrait>>#updateWidth

And if you log out the raised exception, you see some calls to

not initialized fonts and a ZeroDevide and some more errors.

The above catch, catches and hides wrong / to late initialized objects.

--

Best regards,
Igor Stasenko.

Igor Stasenko

Re: Catching Exceptions without any notice

On 1 April 2016 at 21:52, stepharo <[hidden email]> wrote:

Hi

I do not think that aliaksei is not taking care.
He is working a lot to make sure that Pharo will have a future from the UI perspective. He is spending all his free time
on Bloc and what he is doing is massive. He rewrote the AthensCairoBack end to optimise it for Bloc local coordinates.
I'm sure that if someone would have propose some help to him he would have gladly accepted.

I personnaly thank him for all the energy that he is putting into Bloc. The talk at PharoDays was showing really nice results.
Bloc is really getting to be really nice and change the face of Pharo for the next decades so we should encourage people doing
Bloc!

So instead of bashing someone, I would prefer that we help him.
Yes Pharo has some code that we do not like and I have my own list. Now the best things to do is to
improve code review and to fix when we see something wrong.

Hi, Stef. No, it is not about whether you care about things you do or not. It is about approach. If you done something, that good indication that you care about it, isn't? But there's many ways how you doing something.

I am sorry, i won't stop bashing approaches like this piece of code demonstrating. It was not a first time you seen my reaction on such piece of code (we seen that before, exactly related to "on:do: nothing" before), and i don't think you shall be surprised that my reaction is unchanged :)

My take on this: on-do-nothing pattern is good only for one thing: for bug hunting and putting temporary kludges in code in order to find the root of problem. Once problem is solved, you shall remove this.. as well as any 'Transcript show:' , 'Halt once' and stuff like that.
On-do-nothing really don't belongs to category of any production code that pretends to be a good quality one.

Stef

Le 1/4/16 05:54, Igor Stasenko a écrit :

A perfect example of careless programming.
"I'll do it my way, and if it causing any problems, i don't care and i will just ignore them. And it's not my problem anyways, i went to something else already, since this part is works and DONE"
:)

On 30 March 2016 at 14:33, Nicolai Hess <[hidden email]> wrote:

Please don't do this:

updateHeight
    "no need to care about height, when it's logic is not customized"
    self layout isHeightCustom ifFalse: [ ^ self ].
    [ self bounds: (self brickBounds withHeight: self customHeight) ]
        on: Exception
        do: [ "just skip and do nothing" ]

This makes debugging GLM/Brick ui/layout code with "self haltOnce" impossible.
see
GLMBrickGeometryTrait>>#updateHeight
GLMBrickGeometryTrait>>#updateWidth

And if you log out the raised exception, you see some calls to

not initialized fonts and a ZeroDevide and some more errors.

The above catch, catches and hides wrong / to late initialized objects.

--

Best regards,
Igor Stasenko.

Best regards,
Igor Stasenko.

Igor Stasenko

Re: Catching Exceptions without any notice

In reply to this post by Tudor Girba-2

On 1 April 2016 at 09:53, Tudor Girba <[hidden email]> wrote:

Hi,

Nice to hear from you Igor. I am glad to see you around here.

Hi, Doru. Yeah, i am considering whether i want to return to things i left, or not. So, expect more of my acid sarcasm in future. Maybe :)

I do not see how your quote applies to the current case given that the original authors did not leave anywhere, but perhaps it was a joke, and I did not get it.

We all will leave sooner or later. The only what matters is what we left behind :)

Cheers,
Doru

> On Apr 1, 2016, at 5:54 AM, Igor Stasenko <[hidden email]> wrote:
>
> A perfect example of careless programming.
> "I'll do it my way, and if it causing any problems, i don't care and i will just ignore them. And it's not my problem anyways, i went to something else already, since this part is works and DONE"
> :)
>
> On 30 March 2016 at 14:33, Nicolai Hess <[hidden email]> wrote:
> Please don't do this:
>
> updateHeight
> "no need to care about height, when it's logic is not customized"
> self layout isHeightCustom ifFalse: [ ^ self ].
> [ self bounds: (self brickBounds withHeight: self customHeight) ]
> on: Exception
> do: [ "just skip and do nothing" ]
>
> This makes debugging GLM/Brick ui/layout code with "self haltOnce" impossible.
> see
> GLMBrickGeometryTrait>>#updateHeight
> GLMBrickGeometryTrait>>#updateWidth
>
> And if you log out the raised exception, you see some calls to
> not initialized fonts and a ZeroDevide and some more errors.
> The above catch, catches and hides wrong / to late initialized objects.
>
>
>
> --
> Best regards,
> Igor Stasenko.

--
www.tudorgirba.com
www.feenk.com

"No matter how many recipes we know, we still value a chef."

Best regards,
Igor Stasenko.

Aliaksei Syrel

Re: Catching Exceptions without any notice

In reply to this post by Nicolai Hess-3-2

Thanks for feedback :)

Indeed, this is a nice case to learn from. That piece of code was written when I just started working with pharo after coming from Java world.

Purpose of that catch, as mentioned by Nicolai, is indeed to ignore late initialized objects. For example, imagine a Morph with height that is equal to owner's height. In this case one would write in #initialize:

initialize
self height: [ :morph | morph owner height ].

After setting an object as a new height, it gets evaluated and in case of example before throws DNU because owner is obviously still nil.

Trying to improve overall readability I decided to ignore all errors in that kind of blocks instead of putting ifNil:ifNotNil: in hundred places.

Also, I could not imagine that Warning is actually an Exception and exception (in its common sense) is an Error. However, I had in mind that later that kind of code smell should be replaced at least by custom error handler, like we have now in spotter that catches all errors allowing not buggy processors to do their work.
Btw, in first versions of spotter it was the same: it catched exception instead of error but more experienced colleges suggested to not do it like that.

Nevertheless, later I realized that blocks are evil, and instead of trying to work with nils it's much easier and cleaner to just put an assertion.

As a hotfix, I will replace Exception to Error tomorrow. (bad, but still...)

Don't hesitate to post code smells, at least we can make jokes of them :)

Cheers
Alex

On Mar 30, 2016 1:34 PM, "Nicolai Hess" <[hidden email]> wrote:

Please don't do this:

updateHeight
    "no need to care about height, when it's logic is not customized"
    self layout isHeightCustom ifFalse: [ ^ self ].
    [ self bounds: (self brickBounds withHeight: self customHeight) ]
        on: Exception
        do: [ "just skip and do nothing" ]

This makes debugging GLM/Brick ui/layout code with "self haltOnce" impossible.
see
GLMBrickGeometryTrait>>#updateHeight
GLMBrickGeometryTrait>>#updateWidth

And if you log out the raised exception, you see some calls to
not initialized fonts and a ZeroDevide and some more errors.
The above catch, catches and hides wrong / to late initialized objects.

Eliot Miranda-2

Re: Catching Exceptions without any notice

Hi Aliaksei, Hi Stef,

On Fri, Apr 1, 2016 at 2:40 PM, Aliaksei Syrel <[hidden email]> wrote:

Thanks for feedback :)

Indeed, this is a nice case to learn from. That piece of code was written when I just started working with pharo after coming from Java world.

Purpose of that catch, as mentioned by Nicolai, is indeed to ignore late initialized objects. For example, imagine a Morph with height that is equal to owner's height. In this case one would write in #initialize:

initialize
self height: [ :morph | morph owner height ].

After setting an object as a new height, it gets evaluated and in case of example before throws DNU because owner is obviously still nil.

Trying to improve overall readability I decided to ignore all errors in that kind of blocks instead of putting ifNil:ifNotNil: in hundred places.

Also, I could not imagine that Warning is actually an Exception and exception (in its common sense) is an Error. However, I had in mind that later that kind of code smell should be replaced at least by custom error handler, like we have now in spotter that catches all errors allowing not buggy processors to do their work.
Btw, in first versions of spotter it was the same: it catched exception instead of error but more experienced colleges suggested to not do it like that.

Nevertheless, later I realized that blocks are evil, and instead of trying to work with nils it's much easier and cleaner to just put an assertion.

As a hotfix, I will replace Exception to Error tomorrow. (bad, but still...)

Don't hesitate to post code smells, at least we can make jokes of them :)

:-). Stef, no one is "bashing someone". We /must/ be able to criticise code if we want the code base to get better, and this code was bad. Aliaksei responded to the criticism positively and did not take it personally.

But the interesting point is that Aliaksei and people like him are clearly skilled programmers but they are coming to Pharo (or Squeak, or Smalltalk in general) from other systems, and if we as a community are successful in promoting Pharo, Squeak, Newspeak et al, we can expect many more people like Aliaksei to come to our community and want to contribute. So I wonder if we have the energy and resource to write brief documentation on good style vs bad style aimed at the experienced OO programmer who is trying Smalltalk for the first time. I think this is an important demographic to serve, and I think the form of the documentation would be considerably different to normal introductory texts; it can use a much steeper learning curve and can go from "here's how you do it", to "here's how to do it right" and "here's how not to do it", very quickly; something that the usual introductory texts stay away from, leaving these important topics to "advanced" course materials.

Cheers
Alex

Cheers!

On Mar 30, 2016 1:34 PM, "Nicolai Hess" <[hidden email]> wrote:
Please don't do this:

updateHeight
    "no need to care about height, when it's logic is not customized"
    self layout isHeightCustom ifFalse: [ ^ self ].
    [ self bounds: (self brickBounds withHeight: self customHeight) ]
        on: Exception
        do: [ "just skip and do nothing" ]

This makes debugging GLM/Brick ui/layout code with "self haltOnce" impossible.
see
GLMBrickGeometryTrait>>#updateHeight
GLMBrickGeometryTrait>>#updateWidth

And if you log out the raised exception, you see some calls to
not initialized fonts and a ZeroDevide and some more errors.
The above catch, catches and hides wrong / to late initialized objects.

_,,,^..^,,,_

best, Eliot

Tudor Girba-2

Re: Catching Exceptions without any notice

In reply to this post by Igor Stasenko

Hi Igor,

You are more than welcome to come back.

Too much acidity is good neither for you nor for the people around you :). Let’s be kind with one another and assume that we all care and we want to make this world a better place, but that at the same time we are still only humans.

Cheers,
Doru

> On Apr 1, 2016, at 10:51 PM, Igor Stasenko <[hidden email]> wrote:
>
>
>
> On 1 April 2016 at 09:53, Tudor Girba <[hidden email]> wrote:
> Hi,
>
> Nice to hear from you Igor. I am glad to see you around here.
>
> Hi, Doru. Yeah, i am considering whether i want to return to things i left, or not. So, expect more of my acid sarcasm in future. Maybe :)
>
> I do not see how your quote applies to the current case given that the original authors did not leave anywhere, but perhaps it was a joke, and I did not get it.
>
> We all will leave sooner or later. The only what matters is what we left behind :)
>
> Cheers,
> Doru
>
>
> > On Apr 1, 2016, at 5:54 AM, Igor Stasenko <[hidden email]> wrote:
> >
> > A perfect example of careless programming.
> > "I'll do it my way, and if it causing any problems, i don't care and i will just ignore them. And it's not my problem anyways, i went to something else already, since this part is works and DONE"
> > :)
> >
> > On 30 March 2016 at 14:33, Nicolai Hess <[hidden email]> wrote:
> > Please don't do this:
> >
> > updateHeight
> > "no need to care about height, when it's logic is not customized"
> > self layout isHeightCustom ifFalse: [ ^ self ].
> > [ self bounds: (self brickBounds withHeight: self customHeight) ]
> > on: Exception
> > do: [ "just skip and do nothing" ]
> >
> > This makes debugging GLM/Brick ui/layout code with "self haltOnce" impossible.
> > see
> > GLMBrickGeometryTrait>>#updateHeight
> > GLMBrickGeometryTrait>>#updateWidth
> >
> > And if you log out the raised exception, you see some calls to
> > not initialized fonts and a ZeroDevide and some more errors.
> > The above catch, catches and hides wrong / to late initialized objects.
> >
> >
> >
> > --
> > Best regards,
> > Igor Stasenko.
>
> --
> www.tudorgirba.com
> www.feenk.com
>
> "No matter how many recipes we know, we still value a chef."
>
>
>
>
>
>
>
>
>
>
>
> --
> Best regards,
> Igor Stasenko.

--
www.tudorgirba.com
www.feenk.com

"It's not how it is, it is how we see it."

Henrik Nergaard

Re: Catching Exceptions without any notice

In reply to this post by Aliaksei Syrel

>Purpose of that catch, as mentioned by Nicolai, is indeed to ignore late initialized objects. For example, imagine a Morph with height that is equal to >owner's height. In this case one would write in #initialize:

>Initialize

> self height: [ :morph | morph owner height ].

No.

ownerChanged

self height: owner height.

…………..

One should not rely on the owner being set during initialization (90% of the time at least), it makes morphs too reliant on certain cases, which makes them harder to use/understand.

If the morph must have an owner to operate properly, it loses many of its abilities such as being its own root, for example to make images.

ThatMorph new exportAsPng.

Best regards,

Henrik