Improved handleArcs function.

[jrbeaumont]

Removed explicit recursion.

Added extra to the cabal file, required for groupSortOn.

[snowleopard]

A couple of comments on existing code:

• I think boolBit is just standard fromEnum.
• Shall we rename DynSignal to just Signal? I think it's clearer: data Signal = Signal Int deriving Eq.
• This will break when there are more than 26 signals: show (Signal i) = [chr (ord 'A' + i)]. Think how to improve.
• This is a bit sad: "+"isSuffixOff. Can't you convert to strings later, so you can avoid testing strings instead of working with Transition type?
• In initVals, can't you replace foldr (\s -> (++)... simply with map?
• Can you remove explicit recursion from arcPairs and initVal?
• In output, use nubOrd from extra, which is faster.
• Should symbLoop be called consistencyLoop as in the paper?

[snowleopard]

If you prefer <= instead of compare you should at least simplify this as follows :-)

instance Ord DynSignal
    where
       Signal x <= Signal y = x <= y

[jrbeaumont]

I actually just forgot to change this to compare, so I'm gonna use that. It just seems a little nicer to me for some reason

[snowleopard]

I think name mapped is not very meaningful (maybe it used to, but lost the meaning after refactoring).

[snowleopard]

We can drop the TODO: there is no explicit recursion any more, and the code is faster (we do sorting + one pass, instead of multiple passes).

[jrbeaumont]

Thanks for the comments, I will get to this ASAP.
It should be possible to do some work on most of these.

This will break when there are more than 26 signals: show (Signal i) = [chr (ord 'A' + i)]. Think how to improve.

I agree this needs improvement, but I don't believe it can be a quick fix, and it can be fixed with #35, allowing a user to define their own signal names.

[snowleopard]

I agree this needs improvement, but I don't believe it can be a quick fix

Well, there are plenty of quick fixes! For example:

show (Signal i)
    | i < 26    = [chr (ord 'A' + i)]
    | otherwise = 'S' : show i

and it can be fixed with #35, allowing a user to define their own signal names.

True, that would be preferable.

[jrbeaumont]

instance Show Signal where show (Signal i)
    | i < 26    = [chr (ord 'A' + i)]
    | otherwise = 'S' : show i

This throws errors, it doesn't like the |.
I'm gonna come back to this soon. 26 signals at this point is quite a lot, but it does need some adjustment.

[snowleopard]

I think formatting is wrong, perhaps this will work:

instance Show Signal where
    show (Signal i)
        | i < 26    = [chr (ord 'A' + i)]
        | otherwise = 'S' : show i

[jrbeaumont]

That works, thanks.
Next issue:

This is a bit sad: "+"isSuffixOff. Can't you convert to strings later, so you can avoid testing strings instead of working with Transition type?

Because of the numbering of transitions (e.g c+/1), I can't change the first argument to transition, (String, String) to (Transition Signal, Transition Signal). I can change it to (Transition Signal, String) which would remove "+" isSuffixOf.

[jrbeaumont]

Made these changes now. Let me know if there's more to be done :)

[snowleopard]

Merged from 10,600m above.