0003712: Number readFrom: (1/2) storeString

Notes
(0005034 - 126 - 132 - 132 - 132 - 132 - 132) vj 05-24-06 16:04	The atached bugfix is a simplae solution delegating the problem to Object which can read everything using Compiler>>evaluate:.

(0005132 - 620 - 698 - 698 - 698 - 698 - 698) Duncan 05-30-06 16:30	I'm not sure if this is the right fix. I tried your test expression in Dolphin, and it doesn't work there either. There's an insightful comment in their implementation: Implementation Note: The basic idea is to read the initial integer part and then proceed to read a Float if a decimal point is found, a ScaledDecimal if a $s is found, etc. Note that Fractions are not a literal type, as otherwise the meaning of expressions such as '1+3/4' would not be consistent with the normal language semantics (Smalltalk has no operator precedence). Maybe the correct thing to do is not to use Number>>readFrom: ?

(0005133 - 573 - 753 - 753 - 753 - 753 - 753) vj 05-30-06 17:08	Since the following expressions are true (1/2) storeString = '(1/2)' (1/2) isNumber = true one could expect that Number readFrom '(1/2)' had to work (as expected e.g. by SIXX). It can be, of course, solved by fixing SIXX or anything to use Object readFrom:, but I think that the inability of Number to read its own storeString is a strange inconsistency. The attached "fix" is a quick'n'dirty workaroud. The problem can be solved in a much better way, of course - read just what storeString says: '(', then integer, then '/', then integer, then ')'.

(0005143 - 85 - 97 - 97 - 97 - 97 - 97) Duncan 05-31-06 01:16	With the help of Ken Causey on IRC, here's a nicer patch for this bug. Thanks Ken!

(0005144 - 245 - 269 - 269 - 269 - 269 - 269) KenCausey 05-31-06 01:19	vj's fix relies Object class>>readFrom: which uses the Compiler and therefore seems rather heavyweight to me. Duncan's fix adds Fraction class>>readFrom: and tries to do this in much the way other Number subclasses do. I prefer Duncan's fix.

(0005147 - 43 - 43 - 43 - 43 - 43 - 43) vj 05-31-06 11:16	Yes, that's it. I also prefer Duncan's fix.

(0005158 - 360 - 420 - 420 - 420 - 420 - 420) Duncan 06-01-06 16:06	I noticed that with v1 of my changeset: Fraction readFrom: '(1)' would result in a ZeroDivide error, but Object readFrom: '(1)' answers '1' as expected. So in v2 of the changeset, I added a check for zeros before returning the fraction. Just a quick 2 line change: numerator = 0 ifTrue: [^ denominator]. denominator = 0 ifTrue: [^ numerator].

(0005159 - 561 - 561 - 561 - 561 - 561 - 561) KenCausey 06-01-06 17:09	Good catch. In general this sounds good but in the case where the numerator is equal to 0, '(0/8)' or something like that shouldn't the result be 0 (aka the numerator)? Or if you mean that you have read something like '(/23)' then shouldn't that be an error? I don't think you would want to get 23 back from that. Also, maybe you should instead check also for the '/' in addition to the leading parenthesis and if you don't get it just strip off the parentheses and hand it back to Number. That way if someone does '(23.3784)' it will be handled correctly.

(0005160 - 672 - 672 - 672 - 672 - 672 - 672) KenCausey 06-01-06 17:30	Actually, I guess what you really need to check for is that you have a number, a /, and another number with the second number not being 0, this of course after the detection of parentheses has handed off handling to Fraction. If you have the slash but not both numbers (and a literal 0 should count as a number) then I think it should be an error. If there is no slash then it should be handled back to Number. If the denominator = 0 then that's an error (division by zero). I believe that covers all the 'degenerate cases'. Everthing else, including the numerator being zero should be OK (of course for performance reasons you might shortcut the numerator = 0 case).

(0005174 - 373 - 464 - 464 - 464 - 464 - 464) Duncan 06-02-06 01:42	rev3 does the following checks: "The first char after the opening paren must be a digit" aStream peek isDigit ifFalse: [^ self error: 'This is not well-formed.']. numerator = 0 ifTrue: [^ 0]. This seems to be consistent with Object>>readFrom:. '(123)' => 123 with both '(1/)' => ZeroDivide with Fraction, and Syntax error with Object '(0/2)' => 0 with both

(0010708 - 111 - 167 - 167 - 167 - 167 - 167) nicolas cellier 05-11-07 23:58	Well, it seems you forgot this one: Fraction readFrom: (-3/4) storeString. waiting for rev4 cheers

(0010709 - 222 - 262 - 262 - 262 - 262 - 262) Duncan 05-12-07 07:40	I think the following check is unncessary: "The first char after the opening paren must be a digit" aStream peek isDigit ifFalse: [^ self error: 'This is not well-formed.']. Am I missing any cases by removing this?

(0010711 - 708 - 792 - 792 - 904 - 904 - 904) nicolas cellier 05-12-07 21:52	Hi Duncan. Version 4 is now perfect for well formed Fraction as produced by storeString. Of course, not so perfect for non well formed... What we learn here is: - when we introduce error tests we often introduce errors (of course, i count myself in we)... - it is more difficult to handle error cases than regular cases - this is because error can happen at each parsing stage That's why i proposed to read numbers with a NumberParser better equipped with error handling see - SqNumberParser class - bug 0003512 - 'Fun with Number readFrom: What should we do ?' in squeak-dev for lengthy arguments. A simple exercize would be to port Fraction reading in some SqNumberParsser specific method...

Issue History
Date Modified	Username	Field	Change
05-24-06 15:50	vj	New Issue
05-24-06 16:02	vj	File Added: Number class-readFrom.st
05-24-06 16:04	vj	Note Added: 0005034
05-28-06 21:03	vj	Issue Monitored: vj
05-30-06 16:30	Duncan	Note Added: 0005132
05-30-06 17:08	vj	Note Added: 0005133
05-31-06 01:16	Duncan	Note Added: 0005143
05-31-06 01:16	Duncan	File Added: FractionReadFromEnh-dm.1.cs
05-31-06 01:19	KenCausey	Note Added: 0005144
05-31-06 11:16	vj	Note Added: 0005147
06-01-06 16:04	Duncan	File Added: FractionReadFromEnh-dm.2.cs
06-01-06 16:06	Duncan	Note Added: 0005158
06-01-06 17:09	KenCausey	Note Added: 0005159
06-01-06 17:30	KenCausey	Note Added: 0005160
06-01-06 21:14	Duncan	File Added: FractionReadFromEnh-dm.3.cs
06-02-06 01:42	Duncan	Note Added: 0005174
05-11-07 23:58	nicolas cellier	Note Added: 0010708
05-12-07 07:40	Duncan	File Added: FractionReadFromEnh-dm.4.cs
05-12-07 07:40	Duncan	Note Added: 0010709
05-12-07 21:52	nicolas cellier	Note Added: 0010711

Relationships