On 09.09.2010 04:27, Andreas Raab wrote:
Phillipe wrote:
Did you change your position towards #squeakToUtf8 and string concatenation (I didn't follow the entire previous thread)?
Implicitly yes,
That's fine.
but let's discuss this.
Personal coding styles are a matter of taste, no way we ever reach a consensus (the discussion that will go nowhere has already started).
You're saying you want WebClient "without overrides" but is this really what you mean? The only reason these methods are marked overrides is so that they don't kill your system if the methods themselves get added to the system at some point. For example, Grease currently adds extension methods for Collection>>sorted but these methods are already in Squeak and while nothing harmful happens if you load Grease, unloading it will destroy several methods. As a consequence it is vastly advantageous to mark methods as overrides if they have even the slightest possibility to conflict (but of course if you'd rather have them straightforward extensions, I have no problems with that).
We need to fix that, thanks for pointing that out.
As for integrating the changes itself, I think that we're talking two very different issues here. I suspect that the only objection to #squeakToUtf8 and #utf8ToSqueak is that they have "Squeak" in their name. I don't think you'd have any objection if they would've been called #utf8Encoded and #utf8Decoded. Which, honestly, is a childish attitude from my point of view.
For string concatenation on the other hand, we're basically talking about dispersing a whole load of FUD about all the things that "may" go wrong. Fact is, nothing actually *does* go wrong with the change, but the change does fix situations that are *very* difficult to handle otherwise. One of the unfortunate realities of string concatenation is that it's very often used around error messages and logging, often in corner cases that aren't well tested, like:
<some impossible condition> ifTrue:[ self log: 'Found impossible condition: ', foo. ].
The problem with these uses is that it's quite easy to forget some asString, or #printString (for example, if you generally had expected foo to be a string but in the impossible condition it's actually nil) and when you hit the condition your logging screws up and instead of getting informed about the impossible condition the program quits due to the error. In fact, I'm willing to bet that there's at least one bug in Pharo and/or Seaside which is the result of erroneous string concatenation of this kind. It's just very easy to get wrong.
The other relevant bit about this change is that it's entirely type-safe. I.e., the return type does not depend on the argument, the return type is always a string. That means that the change does *not* introduce failures down the road due to type violations. It *does* mean that if you have a bug in your code you might print "a SomethingOrOther" when you didn't mean to, but unless you're the kind of person who believes that a program that doesn't raise an error must be obviously correct, this makes little difference. The only difference it makes is that your program will not abort when the *intention* is so utterly obvious.
Put differently, the change adds nothing but robustness to the system. There is really no data to back up the FUD about all the changes that "may" go wrong, but from my experience there is ample evidence to show that logging and error handling involving string concatenation is highly susceptible to this kind of problem and generally not very well tested and difficult to QA. And the major use of string concatenation is right in these areas.
Last but not least, programming is about efficiency. Why would you waste your time in typing 'The result is: ', x printString" when you could just type "'The result is: ', x" and spare the time to write all the extra characters? Do you realize how much time you've wasted sprinkling all those #printString and #asString around WebClient?
Cheers Philippe