fmidue / logic-tasks

0 stars 1 forks source link

make Delayed type abstract #127

Closed jvoigtlaender closed 4 months ago

jvoigtlaender commented 4 months ago

@owestphal, wouldn't it be better to hide the data constructor of the newtype thus?

I've also been thinking about further replacing

newtype Delayed a = Delayed {unDelayed :: String} deriving (Eq, Typeable, Generic)

by

newtype Delayed a = Delayed String deriving (Eq, Typeable, Generic)

instance Show (Delayed a) where
  show (Delayed str) = str

and

    Left err -> reject $ case parse (fully tokenSequence) "" (unDelayed ans) of

by

    Left err -> reject $ case parse (fully tokenSequence) "" (show ans) of

Is there a need somewhere to really have the default derived Show instance for Delayed, i.e., with "Delayed " being output?

jvoigtlaender commented 4 months ago

For symmetry with parse, maybe the order of arguments of parseDelayed should be flipped?

owestphal commented 4 months ago

Should the signature of parseDelayedRaw :: Parser b -> Delayed a -> Either ParseError b also be restricted to achieve better isolation? With the current signature, the internal string is fully recoverable.

The obvious Parser () -> Delayed a -> Either ParseError () might be too restrictive though. E.g., when we want to count the number of parentheses (see #126).

jvoigtlaender commented 4 months ago

I'd say that parseDelayedRaw as a name is good enough in order to convey that one is dealing with internals if one uses it.

After all, the point of the abstraction is not really to guard against "malicious actors" here, but against "accidental misuse". If someone calls parse...Raw, like unsafe..., they have it coming. :smile: