On 23/09/13 17:18, Bart wrote:

> On 9/23/13, Lukasz Sokol <

[hidden email]> 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 -

[hidden email]
>

http://lists.freepascal.org/mailman/listinfo/fpc-pascal>

_______________________________________________

fpc-pascal maillist -

[hidden email]
http://lists.freepascal.org/mailman/listinfo/fpc-pascal