On 23/09/13 17:18, Bart wrote:
> On 9/23/13, Lukasz Sokol <el.es...@gmail.com> wrote:
>
>> function TryRomanToInt(AInput:String; out AResult: integer):boolean;
>> var i, Len, N, Np1 : integer;
> [snip]
>>
>>
>> if N >= Np1 then AResult := AResult + N
>> else AResult := AResult - N;
>>
>> end;
>> else // i = Len-1 = last char we just add (consider : MCM : add 1000,
>
> Compiler doesn't like the else here. I removed the ; after the end 1 line
> above.
I said below it's only mind-compiled ;)
>
>> sub 100, add 1000 = 1900)
>> begin
>> AResult := AResult + N;
>> end;
>> end; // for
>> // if the above, after all characters are through, has resulted in 0 or
>> less,
>> // we invalidate everything at the end (consider : CMLM, IIIM )
>>
>> Result := AResult > 0;
>> if not Result then AResult := 0;
>> end;
>>
>> (only mind-compiled ;) tests welcome ;) )
>>
>
> It produces "odd" output though:
>
> C:\Users\Bart\LazarusProjecten\ConsoleProjecten>test
> Enter Roman numeral: LL //should be illegal
> TryRomanToInt('LL') -> 50
> StrUtils.RomanToInt('LL') = 100
Oh I see the loop needs to run from 0 to Len-1 ?.... it missed the last (or
first) char
>
> Enter Roman numeral: MM
> TryRomanToInt('MM') -> 1050 //obviously should be 2000
But this looks like you're adding the results? Or are you reusing the
same variable ? In that case stick AResult := 0 at the top of the function
please
> StrUtils.RomanToInt('MM') = 2000
>
> Enter Roman numeral: II
> TryRomanToInt('II') -> 1051 // nice one!
Yeah totally added to former Aresult, and missed last char ...
> StrUtils.RomanToInt('II') = 2
>
> So, it's a little bit flawed.
:)
function TryRomanToInt(AInput:String; out AResult: integer):boolean;
var i, Len, N, Np1 : integer;
function TryRomanCharToInt(AIn: Char; out AOut: integer): boolean;
begin
case AIn of
'M' : AOut := 1000;
'D' : AOut := 500;
'C' : AOut := 100;
'L' : AOut := 50;
'X' : AOut := 10;
'V' : AOut := 5;
'I' : AOut := 1;
else
AOut := 0;
end;
Result := (AOut > 0);
end;
begin
// if it looks like shameless c&p, it's because it is ;)
Result := False;
AResult := 0; // << here you've reused the variable passed to AResult
somewhere
AInput := UpperCase(AInput); //don't use AnsiUpperCase please
Len := Length(AInput);
if (Len = 0) then Exit;
//
i := 1;
for i := 0 to Len-1 do // which char am I actually missing here???
begin
if not TryRomanCharToInt(AInput[i], N) then
begin
AResult := -1; // invalidate everything
exit;
// writeln('Not a valid roman numeral at position ',i);
end;
if (i > Len-1) then
begin
if not TryRomanCharToInt(AInput[i+1], Np1) then
begin
AResult := -1; // invalidate everithing
exit;
// writeln('Not a valid roman numeral at position ',i+1);
end;
// according to wikipedia, .
if not (((N = 100) and (Np1 > 100)) or // C can be placed before L or M
((N = 10) and (Np1 > 10) and (Np1 <= 100)) or // X can be
placed before L or C
((N = 1) and (Np1 > 1) and (Np1 <= 50))) // I can be placed
before V and X << here too
then
begin
AResult := -1; // invalidate everithing: catches MDCLXVIVXLDM
exit;
// writeln('Not a valid roman numeral combination at position ',i,
' and ',i+1);
end;
if N >= Np1 then AResult := AResult + N
else AResult := AResult - N;
end // i > Len-1, you're right about the ;
else // i = Len-1 = last char we just add (consider : MCM : add 1000, sub
100, add 1000 = 1900)
begin // alternatively this should exit the loop maybe on the N-1th
character
AResult := AResult + N;
end;
end; // for
// if the above, after all characters are through, has resulted in 0 or less,
// we invalidate everything at the end (consider : CMLM, IIIM )
Result := AResult > 0;
if not Result then AResult := 0;
end;
>
> Anyhow, let's not focus upon what the new code should be.
> The question was: is current behaviour (accepting IIMIIC etc.) a bug or not.
>
> Bart
> _______________________________________________
> fpc-pascal maillist - fpc-pascal@lists.freepascal.org
> http://lists.freepascal.org/mailman/listinfo/fpc-pascal
>
_______________________________________________
fpc-pascal maillist - fpc-pascal@lists.freepascal.org
http://lists.freepascal.org/mailman/listinfo/fpc-pascal
