Issue 3817 in pharo: make Object >> error: deprecated

Status: New
Owner: ----

New issue 3817 by [hidden email]: make Object >> error: deprecated
http://code.google.com/p/pharo/issues/detail?id=3817

Object >> error: should be deprecated.
It encourages the usage of raw Errors too much. Using specialized errors  
everywhere in the system should be encouraged.

Comment #1 on issue 3817 by marianopeck: make Object >> error: deprecated
http://code.google.com/p/pharo/issues/detail?id=3817

I think this change is too radical, and it should be discussed. I think a  
lot of packages will be broken. Is this part of ANSI ?

Comment #2 on issue 3817 by [hidden email]: make Object >> error:  
deprecated
http://code.google.com/p/pharo/issues/detail?id=3817

it is not too radical for 1.3. Using Error is as useful as saying that  
something went wrong somewhere. You don't want that. I agree that `self  
error: 'foo'` is nice during development, but it should throw and error  
indicating that you should NOT use this for your final code.

Comment #3 on issue 3817 by [hidden email]: make Object >> error:  
deprecated
http://code.google.com/p/pharo/issues/detail?id=3817

I don't fully agree.  You have to use particular exception types, when you  
want to give them a special treatment (by code).

Most of the times, the error just flows to the user, where the only  
important thing is the message or the information the exception carries to  
show them to him.  And that is the normal treatment, not an special one.

Comment #4 on issue 3817 by [hidden email]: make Object >> error:  
deprecated
http://code.google.com/p/pharo/issues/detail?id=3817

Yes, Object>>#error: is part of the ANSI protocol.
I agree in spirit 100% with what Camillo is saying though.

To me, a better approach would be to lead by example.
Only 700 uses to fix in Core :)

Comment #5 on issue 3817 by [hidden email]: make Object >> error:  
deprecated
http://code.google.com/p/pharo/issues/detail?id=3817

only :)
and I'm sure that I fixed some of them already :).
What would be good is to perform an analysis to see their usage.

Comment #6 on issue 3817 by marianopeck: make Object >> error: deprecated
http://code.google.com/p/pharo/issues/detail?id=3817

Your point of view, is .. only one point of view.
If you check my code, you will see that sometimes, I reify an exception in  
a subclass of Error. This is because of the users of such code, do special  
handling for them. Nevertheless, a lot of times, I don't need this, and  
just throwing a generic error is enough.

Comment #7 on issue 3817 by [hidden email]: make Object >> error:  
deprecated
http://code.google.com/p/pharo/issues/detail?id=3817

I agree that `self error:` is amazingly useful, but it should be used with  
caution. Lazyness is never an argument for keeping a partly broken design!  
It is similar to using dictionaries or arrays instead of real objects with  
a proper class attached.

If your error is just to inform the user, then it belongs into the View,  
nowhere else! And then you should use something like a  
UserNotificationError to make this very explicit.

If your error should inform the "user" from within the model, then you  
should use an explicit class which gives additional semantics (besides the  
human readable string) to it.

So what I suggest is to replace `Object >> error:` with something like a  
GenericError or UndefinedError, to encourage the use of proper error  
classes.

Comment #8 on issue 3817 by [hidden email]: make Object >> error:  
deprecated
http://code.google.com/p/pharo/issues/detail?id=3817

What's a broken design?  Tell me how does a more specific subclass of Error  
help you if you don't give it an special treatment.  If you don't do that,  
the semantics IN a class name are the same as the semantics in the string  
it contains if you just use the name for give information.

So, if I have to show an error to the user, should I catch all exceptions  
and rethrow a new generic exception on the "view"?  I don't catch up that  
part.

Tell me WHY do this: "If your error should inform the "user" from within  
the model, then you should use an explicit class which gives additional  
semantics (besides the human readable string) to it."
I think it doesn't make sense, as I said before why.

Comment #9 on issue 3817 by [hidden email]: make Object >> error:  
deprecated
http://code.google.com/p/pharo/issues/detail?id=3817

Just to give an example of what I mean.

In Java the general error handling is mainly based on a set of specific  
classes, the generic Exception is almost never used. For instance  
InvalidArgumentException is a common use-case where in the existing  
Smalltalk code `self error` is used, whereas a specific would be  
appropriate. In many places like BlockContext >> valueError generic errors  
are thrown with only a user readable warning.

So the main issue here is, that in may places a specific error could be  
used, but it isn't and hence a specific treatment of said error makes no  
sense at thus is not used (this is what I called a broken design).

Of course the whole system has moved towards the usage of generic errors,  
but this is the same as starting to use generic dictionaries instead of  
proper objects, just because it is less effort to do so.

  Besides work on adapting all the senders of `Object >> error:` what would  
be other negative effects?

Comment #10 on issue 3817 by [hidden email]: make Object >> error:  
deprecated
http://code.google.com/p/pharo/issues/detail?id=3817

http://www.springerlink.com/content/e7170h6104136721/ seems to be a  
thorough study on exception handling.

One important claim is, that in order to increase software quality errors  
have to be handled. However, what I can see in Pharo is, that using a  
generic Error does not help to accomplish this goal.

Comment #11 on issue 3817 by [hidden email]: make Object >> error:  
deprecated
http://code.google.com/p/pharo/issues/detail?id=3817

Camillo,

Regarding your comment in #6, I think replacing `Object >> error:` with  
something like..." is not an option, except if you're canvassing for  
reducing Pharo compatibility with ANSI Standard.

We should find a way to arrive at the results you propose creating a tool  
and perhaps adding one or more methods to achieve the substitutions your  
deem necessary, but not change compatibility with standards.

NOW, if you're calling for a different Standard, then we need to find the  
resources and the stamina for doing it ;-)

Comment #12 on issue 3817 by [hidden email]: make Object >> error:  
deprecated
http://code.google.com/p/pharo/issues/detail?id=3817

Lets convey that if you have an InvalidArgumentException, that's a bug in  
your code.  There's no way that it can be handled.  It is no useful.  You  
can catch it?  Yes, you can, but what can you do about it?

I'm in favor of using specific errors, where it is useful.

And there are errors you cannot (actually you shouldn't) handle, as the  
InvalidArgumentException.

So, about deprecating Object>>error: I say -1.

Comment #13 on issue 3817 by marianopeck: make Object >> error: deprecated
http://code.google.com/p/pharo/issues/detail?id=3817

Thanks Guille. -1 too
And +1 to use special errors when necessary (again, check my projects and  
you will see I do this)

Comment #14 on issue 3817 by [hidden email]: make Object >> error:  
deprecated
http://code.google.com/p/pharo/issues/detail?id=3817

I totally agree with Camillo. Error: does not convey any meaning. Even  
though your code currently does not handle the error specifically, you  
might later on want to catch it from the outside. Just getting back strings  
makes this impossible.

It is just object-oriented programming to use specific objects wherever you  
have a specific scenario! What do you gain by using strings? Maybe it's a  
tiny bit shorter syntax-wise? It definitely doesn't gain you anything else.  
It definitely makes your code less reusable.

It's the same as using `nil` everywhere as marker-object, and as Camillo  
said, using arrays and dictionaries rather than objects of specific types.  
It totally clutters the code and does not convey any meaning.

A very common "pattern" you end up using when throwing strings is  
concatenating strings and serialized versions of objects to give more  
information. If you throw a specific exception with the relevant objects  
inside, you can actually start using standard Pharo tools such as object  
inspectors to figure out what went wrong. The concatenated string doesn't  
contain the actual relevant objects anymore and this makes it very hard to  
understand what's going on. Using strings is just laziness, bad programming  
and bad design all rolled up into one fuzzy ball.