issue-#95 TextMapper Scanner & SETS

[Robert]

The Scanner interprets "{2, 3 6}" as the legal set {2, 3, 6}.

Curiously the compiler recognises the first form as "illegal".


I don't have a fix, but I am working on one.

Please advise me if this is an old bug with a known fix.

Cheers

Robert

[Robert]

{"2, 3, 6,}" is also regarded as legal!

[Robert]

I have been copying this routine into version that reads a String, rather than a Text. My copy was not working, and it took me some time to realise that the original did not work correctly either!

I now have a working (I think) String version. I hesitate to suggest it as a model to correct the TextMappers Scanner as it looks like horrible style. It looks like an incomprehensible, unmaintainable, unstructured state machine, probably because it is an incomprehensible, unmaintainable, unstructured state machine. Modern structured languages do not seem well suited to implementing state machines.

Maybe someone has a more elegant solution?

Code: Select all

PROCEDURE  TxtToSet* (IN txt : ARRAY OF CHAR; OUT set : SET) : BOOLEAN;
  VAR
    j, n, m, state  :  INTEGER;
    ch              :  CHAR;

  PROCEDURE  Get;
    BEGIN
      ch  :=  txt [j]; INC (j)
    END  Get;

  PROCEDURE  Skip;
    BEGIN
      WHILE  ch = ' '  DO  ch  :=  txt [j]; INC (j)  END
    END  Skip;

  PROCEDURE  Cardinal () : INTEGER;
    VAR
       n  :  INTEGER;
    BEGIN
      n   :=  ORD (ch) - ORD ('0'); Get;
      IF  ('0' <= ch)  &  (ch <= '9')  THEN
        n   :=  10 * n + ORD (ch) - ORD ('0'); Get
      END;
      Skip; RETURN  n
    END  Cardinal;

  BEGIN
    j  :=  0; Get; Skip; IF ch # '{'  THEN  RETURN  FALSE  END;
    Get; Skip; set  :=  {}; state  :=  0;
    LOOP
      CASE  ch  OF
        ','      :  IF  state  IN  {1, 3}  THEN  Get; Skip; state  := 4
                                           ELSE  RETURN  FALSE  END
      | '.'      :  IF  state # 1  THEN  RETURN  FALSE  END;
                    Get;
                    IF  ch  #  '.'  THEN  RETURN  FALSE  END;
                    Get; Skip; state  :=  2
      | '}'      :  IF  ~(state  IN  {0, 1, 3})  THEN  RETURN  FALSE  END;
                    Get; Skip; RETURN  ch  =  0X
      | '0'..'9' :  IF  ODD (state)  THEN  RETURN  FALSE  END;
                    IF  state = 2  THEN
                      m  :=  Cardinal (); 
                      IF  (m  < n)  OR  (m > MAX (SET))  THEN  RETURN  FALSE  END;
                      WHILE  m > n  DO  INCL (set, m); DEC (m)  END;
                      state  :=  3
                    ELSE
                      n  :=  Cardinal ();
                      IF  n  >  MAX (SET)  THEN  RETURN  FALSE  END;
                      INCL (set, n); state  :=  1
                   END
      ELSE
        RETURN  FALSE
      END
    END;
  END  TxtToSet;

[Robert]

I enclose a possible solution in the first listing below. I have given it a new name so it can temporarily coexist with the old routine. To use the new one change "Set" to "Sett" once inside the routine TextMappers.Scan in the line "IF interpretSets IN s.opts THEN Set(s) ELSE Char(s) END".

The second listing is a test module with results. I write 7 "Sets" into a Text, of which three are invalid. The current routine only recognises one as invalid. The new routine works correctly on all these cases.

These seven lines are the only strings I have tested it on!

Code: Select all

	PROCEDURE Sett (VAR s: Scanner);
		VAR n, m, state: INTEGER; ch: CHAR;
	BEGIN
		s.type := set; Get(s, ch); s.Skip(ch); s.set := {}; state := 0;
		LOOP
			CASE ch OF
				',' : IF state  IN {1, 3} THEN Get(s, ch); s.Skip(ch); state := 4
						ELSE s.type := invalid; EXIT END
			|	'.' : IF  state # 1 THEN s.type := invalid; EXIT END;
					Get(s, ch); IF ch # '.' THEN s.type := invalid; EXIT END;
					Get(s, ch); s.Skip(ch); state  :=  2
			|	'}' : IF state IN {0, 1, 3} THEN Get(s, ch) ELSE s.type := invalid END; EXIT
			|	'0'..'9' : IF ODD (state) THEN s.type := invalid; EXIT END;
					Cardinal(s,m); s.Skip(ch);
					IF m > MAX(SET) THEN s.type := invalid; EXIT END;
					IF state = 2 THEN
						IF m  < n THEN s.type := invalid; EXIT END;
						WHILE m > n DO INCL(s.set, m); DEC(m) END;
						state := 3
					ELSE
						INCL(s.set, m); n := m; state := 1
					END
			ELSE
				s.type := invalid; EXIT
			END
		END
	END Sett;

>>> The code above has been updated to a slightly simpler version 6-Oct-2015 <<<

Code: Select all

MODULE TestScanner;

(*  Robert Campbell,  *)

	IMPORT
		TextMappers, TextModels, Log := StdLog;

PROCEDURE  Do*;
	VAR
		text: TextModels.Model;
		fmtr: TextMappers.Formatter;
		sc: TextMappers.Scanner;
	BEGIN
		text := TextModels.dir.New();
		fmtr.ConnectTo(text);
		fmtr.WriteString ('{1, 13, 17, 22}'); fmtr.WriteLn;
		fmtr.WriteString ('{2, 13..17, 22}'); fmtr.WriteLn;
		fmtr.WriteString ('{3, 13, 17 22}'); fmtr.WriteLn;
		fmtr.WriteString ('{4, 13, 17, 22,}'); fmtr.WriteLn;
		fmtr.WriteString ('{5, 13, 17, A}'); fmtr.WriteLn;
		fmtr.WriteString ('{6}'); fmtr.WriteLn;
		fmtr.WriteString ('{}'); fmtr.WriteLn;

		sc.ConnectTo (text);
		sc.SetOpts ({TextMappers.interpretSets});
		sc.Scan;
		WHILE sc.type # TextMappers.eot DO
			CASE sc.type OF
				TextMappers.char : Log.String('Character : '); Log.Char(sc.char); Log.Ln
			|	TextMappers.string : Log.String('String : ' + sc.string); Log.Ln
			|	TextMappers.int : Log.String('Integer :  ' ); Log.Int(sc.int); Log.Ln
			|	TextMappers.set : Log.Set(sc.set); Log.Ln
			|	TextMappers.invalid : Log.String('Invalid ! '); Log.Ln
			END;
			sc.Scan;
		END
	END Do;

	END TestScanner.


   DevDebug.Unload

  TestScanner.Do


OMS Scanner:
 {1, 13, 17, 22}
 {2, 13..17, 22}
 {3, 13, 17, 22}
 {4, 13, 17, 22}
Invalid ! 
String : A
Character : }
 {6}
 {}

Suggested modified Scanner:
 {1, 13, 17, 22}
 {2, 13..17, 22}
Invalid ! 
Integer :   22
Character : }
Invalid ! 
Character : }
Invalid ! 
String : A
Character : }
 {6}
 {}

[Josef Templ]

The TextMappers docu says that it can scan sets in the CP notation.
As far as I see, it does not explicitly mention any error behavior.
So, in a sense the current implementation is correct because it
does scan sets in the CP notation indeed.
It may well be that ominc introduced the "error tolerance" (ignoring separation characters)
for the convenience of the users. Note that such a scanner is typically used for processing
command arguments to by typed in by the user.
I would certainly not introduce a fix as complicated as the one that has been proposed.

- Josef

[Ivan Denisov]

There is Strings.Valid procedure. Maybe we can add Strings.ValidSet procedure with Robert's algorithm?

[Robert]

The TextMappers docu says that it can scan sets in the CP notation.
As far as I see, it does not explicitly mention any error behavior.
So, in a sense the current implementation is correct ....

Actually it says it will recognise them. Some invalid sets are flagged with the the scanner type invalid because they have not been recognised as valid. So this "recognise as valid" function is already there, it is just that it does not work correctly - so, in my opinion, is a bug. Ok, not the most serious bug on the planet!

I would certainly not introduce a fix as complicated as the one that has been proposed.

Yes, my suggestion is a bit complicated. Probably someone has a better approach.
But to put it in context it is only 1 line more than the incorrect Oms implementation. And I have a slight simplification which saves that line!

>>> Previous listing updated to include this (minor) simplification. <<<

[Ivan Denisov]

I agree with Robert, to have a check is better than not to have. This place does not require hight performance.

Josef, why you do not like format checking? The same story as it was with Utf8 conversion

[Zinn]

Dear Robert, dear Ivan,
please have a look from the other side of the sky.

It is not a bug - it is a future.
Never hear about a fault tolerant system?

The routine is not used inside a compiler where this kind of errors should be detect.
It is used interactively by user input where auto correction are welcome.

The user types a set of data. Why should he do all the stuff again by a missing comma?

The original solution is more convince.
- Helmut

[Ivan Denisov]

The errors of input syntax is a signal that there can be errors in data input. This case is not the place for the fault tolerance. At least system should generate warning message.