Skip to content

Commit

Permalink
rename 'lookback'-accessors
Browse files Browse the repository at this point in the history
  • Loading branch information
owestphal committed Jul 28, 2023
1 parent 0de0234 commit 71f6d82
Show file tree
Hide file tree
Showing 4 changed files with 12 additions and 12 deletions.
4 changes: 2 additions & 2 deletions examples/Example.hs
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ example8 =
readInput x ints AssumeValid <>
tillExit (
readInput x ints AssumeValid <>
branch (currentValue' x 1 .+. currentValue x .==. intLit 0)
branch (recentValue x 1 .+. currentValue x .==. intLit 0)
exit
nop
) <>
Expand Down Expand Up @@ -387,7 +387,7 @@ hangmanSpec word = tillExit (
branch (winCond $ allValues g) (writeOutput [Text "correct!"] <> exit) mempty
<> writeOutput [Text "Game state:" <> Wildcard]
<> readInput g digits AssumeValid
<> branch ((currentValue g `isIn` listLit word) .&&. (currentValue g `isNotIn` allValues' g 1))
<> branch ((currentValue g `isIn` listLit word) .&&. (currentValue g `isNotIn` recentValues g 1))
(writeOptionalOutput [Text "good guess!"])
(writeOptionalOutput [Text "wrong guess!"])
)
Expand Down
8 changes: 4 additions & 4 deletions src/Test/IOTasks/Internal/Term.hs
Original file line number Diff line number Diff line change
Expand Up @@ -126,14 +126,14 @@ termVarExps (termStruct -> VariableC e _) = [toVarList e]
termVarExps (termStruct -> VariableA e _) = [toVarList e]

instance Accessor Term where
currentValue' :: forall a e. (Typeable a, VarExp e) => e -> Int -> Term a
currentValue' = matchType @a
recentValue :: forall a e. (Typeable a, VarExp e) => e -> Int -> Term a
recentValue = matchType @a
[ inCaseOfE' @Integer $ \HRefl -> Current
, inCaseOfE' @String $ \HRefl -> Current
, fallbackCase' $ error $ "variable type not supported for Terms: " ++ show (typeRep @a)
]
allValues' :: forall a e. (Typeable a, VarExp e) => e -> Int -> Term [a]
allValues' = matchType @a
recentValues :: forall a e. (Typeable a, VarExp e) => e -> Int -> Term [a]
recentValues = matchType @a
[ inCaseOfE' @Integer $ \HRefl -> All
, inCaseOfE' @String $ \HRefl -> All
, fallbackCase' $ error $ "variable type not supported for Terms: " ++ show (typeRep @a)
Expand Down
4 changes: 2 additions & 2 deletions src/Test/IOTasks/OutputTerm.hs
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,8 @@ instance Ord (OutputTerm a) where
compare = compare `on` toExpr

instance Accessor OutputTerm where
currentValue' x n = checkNames x $ Transparent $ Current x n
allValues' x n = checkNames x $ Transparent $ All x n
recentValue x n = checkNames x $ Transparent $ Current x n
recentValues x n = checkNames x $ Transparent $ All x n

data SomeOutputTerm where
SomeOutputTerm :: Typeable a => OutputTerm a -> SomeOutputTerm
Expand Down
8 changes: 4 additions & 4 deletions src/Test/IOTasks/Terms.hs
Original file line number Diff line number Diff line change
Expand Up @@ -111,12 +111,12 @@ instance Typeable a => VarExp [Var a] where

class Accessor t where
currentValue :: (OverflowType a, VarExp e) => e -> t a
currentValue x = currentValue' x 0
currentValue' :: (OverflowType a, VarExp e) => e -> Int -> t a
currentValue x = recentValue x 0
recentValue :: (OverflowType a, VarExp e) => e -> Int -> t a

allValues :: (OverflowType a, VarExp e) => e -> t [a]
allValues x = allValues' x 0
allValues' :: (OverflowType a, VarExp e) => e -> Int -> t [a]
allValues x = recentValues x 0
recentValues :: (OverflowType a, VarExp e) => e -> Int -> t [a]

This comment has been minimized.

Copy link
@jvoigtlaender

jvoigtlaender Jul 29, 2023

Member

Is recentValues a good name? It seems like it skips the most recent n values, and then returns the ones that occured before that. If that is the behavior, it seems more like earlierValues or maybe even pastValuesBefore rather than recentValues would make sense.

This comment has been minimized.

Copy link
@jvoigtlaender

jvoigtlaender Jul 29, 2023

Member

Put differently, if the function is named recentValues, one might expect that recentValues 3 = [recentValue 0, recentValue 1, recentValue 2. recentValue 3] (or something like that, maybe in reverse order of the list), but that is incompatible with allValues = recentValues 0.

This comment has been minimized.

Copy link
@jvoigtlaender

jvoigtlaender Jul 29, 2023

Member

Maybe a good naming scheme is: recentValue -> valueBefore, recentValues -> valuesBefore.

Then also the order of arguments might better be switched, though, as „valueBefore 1 x“ reads better than „valueBefore x 1“?


as :: Typeable a => t a -> t a
as = id
Expand Down

1 comment on commit 71f6d82

@jvoigtlaender
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changing the accessor names should also impact the Demo repo, since at least one example there used currentValue'.

Please sign in to comment.