Homework-New/milestone_1_feedback.md

14 KiB

Jack Attack

  • Your comments are very thorough! Have you, however, consider the special "haddock" syntax for your documentation? For instance, in your Object record, you clearly describe what each field does. If you used the "haddock" syntax (maybe something like --^ comment goes here), you'd be able to generate static documentation pages much like those you see on hackage or stackage. Those are useful because they allow the user to browse the documentation for your package from the web before deciding to install and use it. And, of course, I think it's nicer to look at a web page than comments!

  • There's a lot of duplicate functionality in your doTimedTransform function compared to your doTransform function. It seems almost as though there should be one function that takes care of both the interpolation and the transformation; perhaps doTransform can take an argument 0.0..1.0 of "progress", and go from there. This will save you the trouble of unpacking and repacking your Transformation objects. Even if you don't go for that approach, however, there's still an improvement you can make. Rather than making your doTimedTransform function do everything, you may want to define a function

    tween :: Object -> Float -> Transformation -> Transformation
    

    This function would do kind of like what your doTimeTransform does: it would interpolate the transformation given the current object and the "progress" within the transformation. The difference, though, is that it will not actually apply the transformation, but create a new one (much like you're already doing in doTimedTransform anyway). You would then be able to feed this transformation into doTransform, and it would work as expected. The advantage here is that you won't have to be writing duplicate code for stuff like:

    doTransform (Combine xs) $ doTransform x obj
    doTimedTransform (seconds, x) (doTimedTransform (seconds, Combine xs) obj elapsed) elapsed
    

    There's a fairly clear similarity (and consequently duplication) here, and that's what I think my approach would suggest.

    And there's one more thing: does tween really need access to the object being animated? It sure seems to if your transformations are "absolute", like "move to (1,2)". This "absoluteness" of transformations forces you to use obj in doTimedTransform, and would also force you to use it in tween. But if your transformations were delta based ("move left 1, up t2"), your tween could be only a function of the current "progress" Float:

    tween :: Float -> Transformation -> Transformation
    

    This may help clean up some of the code, and it also allows for an interesting experiment: what if you compose tween with various functions Float -> Float? For instance, you could compose it with \x -> x * x, which I think would make your animation start out slow and speed up at the end. You may be able to use this to play around with various ways of animating your objects that are used in "actual" animation: from what I know, linear transformations are not as common as, say, sigmoid ones (animation starts slow, gets fast in the middle, and slows down again at the end).

  • I would try to factor out the "seconds" part of your calculations as soon as possible. In my opinion, it would be clearer to pass around a "percentage completed" float, especially if you don't have any other dependence on the current time.

  • Cody is right, you do have a couple of places that can be used by folds or other higher order functions. For instance, getAniLength is just sum . map fst, and the Combine case of doTransform is something like foldr (flip doTransform) obj (x:xs).

  • Are transformations a monoid? I suspect that they are, with mempty = Wait, mappend x y = Combine [x,y], and mconcat = Combine. I don't know if this will be useful, but it's always nice to have access to more standard library functions!

  • I'm not sure I follow the AST vs Lib modules. They both seem to have something in mind, but I can't quite figure out how they work together. Does the AST eventually produce a Transformation? I just don't really know what to make of it!

About your design questions:

  • Q1: It would be cool if you had a slider that lets you play the animation back and forward (or at an arbitrary point in time). I think it may help in debugging:

Why did my object just make a 360 and walk away?

It would be even better if you could label your transformations with the function / piece of code that induced them, so that at any point, you can kind of see what part of your code is causing the current transformation.

  • Q2: At the implementation level, you may need to replace the Float argument of doTransform with an Env argument, which may be a record-like thing that contains the current mouse position, the current time, and so on. You'd then thread it as before.

    At a language level, things are more interesting. Have you looked into functional reactive programming? You may be interested in encoding transformations declaratively, given, say the current mouse position, time, etc. as input. Your programs would then be descriptions of how to convert these inputs into figures on the screen.

    Elm is a language that implements quasi-FRP. It doesn't deal with continuous inputs like you would be, but it encodes programs as transition functions (Event -> State -> State). You may be interested in adapting a similar approach - it's a nice way to deal with "imperative" environments from a functional language.

  • Q3: This is a tough one! I think you can come up with a textual functional language that can be used for your domain, but I'd be thrilled to see how you may incorporate visual (scratch-like) elements into your project. At this point, though, I can't help but wonder: would you have time to implement something like Scratch-like blocks? We're in week 8, after all. That would be a very graphical type of user interface, and I can't personally imagine defining a UI from scratch, in Haskell, using only graphics library primitives. Of course, you're probably more versed than I am in Haskell's graphics tooling, but it seems to me like a fairly daunting task.

Escape

  • I played with your game for a bit, pretty cool! I think you did a nice job with the textual users interface; it feels pretty much like I'd expect a command line interface to work. I did run into an error on my first playthrough, and decided not to try again: Prelude.head: empty list. This happened because I typed search without any arguments. Now, I understand that what I did was an incorrect command, but crashing the game doesn't seem like the right thing to do! head is a dangerous function; I think that if Haskell were redesigned today, from the ground up, this function would no longer have type [a] -> a. In general, exceptions in Haskell are a bit of an antipattern, because they're not indicated by the types (how could you know head throws an exception), but also because they're cumbersome to work with!

    Languages defined after Haskell actually define head with a Maybe. Here's the Idris version, and here's the Elm version. You may want to define your own head function, safeHead :: [a] -> Maybe a! You can then use pattern matching or the maybe function to properly handle the error that occurs when the user doesn't give the command enough arguments. You could, for instance, print "Search what?" when a user types in "search".

  • This is a minor nitpick, but I'm pretty sure snake_case is not the preferred standard for Haskell variable names. Your records (and module names!) should be in camelCase. In fact, my linter plugin covers the screen with warnings about style when it sees your code!

  • Funcfunctor is a funny name for a typeclass. It seems to imply that t is a functor; however, Functor is a type constructor class, whereas your code treats t as just a type, not a type constructor! In fact, I'm really not sure what to make of the word Funcfunctor. What does it mean?

  • On the other hand, I think that this is a good application of type classes. If you want to print a variety of different objects on the screen in your command line game, defining a common type class with the various properties of these objects is a great idea. You can certainly get a very consistent interface this way!

  • In Escape_funs, a lot of your pattern matching code can be significantly simplified! You always write out stuff like Object {object_id=id, object_name=...}, but use only one of these variables (for instance, id). Instead, you can only write Object {object_id=id} -- that's it! You don't need to manually enumerate every single key in your records to pattern match on them.

    A cute fun fact of this notation: it's "preferred style" according to Haskell's linter to rewrite code like (Constructor _ _ _ _) (pattern matching on a data type and ignoring all its fields) as Constructor{}.

  • foldr (||) False (map (==(get_key obj)) list) is good use of a fold a map, but it also corresponds to the Haskell function any: any (==(get_key obj)) list.

  • Notice how your run_code is a bit fragile: your help function prints out all the commands available to the user, while run_code actually implements these commands. It must be very easy to edit run_code to add a new command, while forgetting to update help. You may be able to work around this by defining a Command data structure: maybe it'll have a description field that you print to the user, and another action filed that's maybe some monad like IO () that actually performs the actions that your command does. Then, you can have a global list of commands commands :: Map String Command, which contains all the allowed commands in your game. Your help function would go through this commands variable and print out the description field of every available command; on the other hand, the run_code function would read a command from the user, look it up in commands, and try to execute the action field.

  • Notice that you're explicitly handling your application's state: you will need to pass the correct house to run_code every time, which means that for each command, you need a separate call to run_code with whatever house that command puts your player in.

    An alternative approach is to handle state implicitly. You won't need to take a house argument explicitly, and you therefore won't have to explicitly pass it to your recursive calls to run_code. In your case, this would be best done with a State monad, since the current location of the player is, well, a piece of state.

    The question, though, becomes: how the heck do you incorporate a state monad into your IO-based code? The answer to that is the StateT monad transformer. We haven't learned about these in class just yet, but we will soon, and I think it would serve you well! Instead of making your code of type IO () (what it currently is), you'd use something like StateT [Room] IO (). Your calls to putStrLn would have to go through a liftIO, but you'd also gain access to functions get and put, which are the functions of the State monad that we have covered in class. Then, whenever your user goes to a different house, you'd run put new_house, and whenever you need to check the current location of the player, you'd run get.

    You may then incorporate other monad transformers into this implementation; I think of interest to you would be ExceptT (which is not the same as Haskell's exceptions that I mentioned in my first bullet point!), which might help you exit out of your code when your player types "exit".

  • By the way, you should probably explicitly type out the signatures of your function; I think that's considered good style. For instance, for run_code, you'd write:

    run_code :: Player -> [Room] -> IO ()
    run_code plauer house = ...
    

Your questions:

  • Q1: Hey, I brought up StateT and ExceptT and monad transformers in general earlier! Check them out. I think you may find them useful for what you're doing. The only difficulty is that you really need to see a few examples of code before-and-after the use of State and Exception monads to be comfortable in applying them (at least for me! It took me a while to properly understand monads, and more time to understand monad transformers).

  • Q2: Haskell's IO monad has readFile and writeFile methods, but these may be a bit basic for your taste. You may be interested in various formats like INI or even JSON (for which you can use something like Aeson, especially its encodeFile and decodeFile methods).

I'm running out of steam writing this comment, but I may post a follow up if I think of anything else!

Eric, if you're reading this: have I been evangelizing monad transformers too much? I feel like I have been. It would be good to know if I'm being overzealous; everything starts to look like a nail when all you have is a hammer!