Spooky code

[bitemyapp]

These are suggestions, act/ignore as thou wilt.

• https://github.com/solatis/haskell-network-anonymous-i2p/blob/master/src/Network/Anonymous/I2P/Protocol.hs#L61-L62 datatypes/newtypes pls - also, what if it fails to parse?
• https://github.com/solatis/haskell-network-anonymous-i2p/blob/master/src/Network/Anonymous/I2P/Protocol.hs#L88-L89 Make a Version type, preferably with a smart constructor that validates sensibility of the [Integer]
• https://github.com/solatis/haskell-network-anonymous-i2p/blob/master/src/Network/Anonymous/I2P/Protocol.hs#L103 types pls
• https://github.com/solatis/haskell-network-anonymous-i2p/blob/master/src/Network/Anonymous/I2P/Protocol.hs#L132-L134 Better
• https://github.com/solatis/haskell-network-anonymous-i2p/blob/master/src/Network/Anonymous/I2P/Protocol.hs#L320 Another bare ByteString
• https://github.com/solatis/haskell-network-anonymous-i2p/tree/master/src/Network/Anonymous/I2P/Types Every type synonym -> data/newtype

HTH.

[solatis]

Thanks for the review! I'll take a look at them ASAP -- the type aliases shouldn't be too much of a problem, so that tackles 90% of your comments.

With regards to "what if it fails to parse", either a ProtocolError is thrown (if the message could be parsed but is not what we expected) or fail is called by network-attoparsec: https://github.com/solatis/haskell-network-attoparsec/blob/master/src/Network/Attoparsec.hs#L114

[bitemyapp]

@solatis

With regards to "what if it fails to parse",

I don't have strong opinions here. My preference is to not "hide" input-related errors into an exception/fail/error, and reserve exceptions for exceptional cases but you might very well have a good reason for doing as you do so I'm not going to make a recommendation. Errors that can be caused by mere input invalidity I usually lift into a sum type and return in EitherT. Usually.

Sounds like you're on the right track then. Good hacking :) 🐻

[solatis]

Well I explicitly decided to also use MonadMask / MonadCatch for that reason, to not 'hide' the exceptions. but I get where you're coming from: exceptions have an impure feeling to them, and it might be cleaner to use Either.

Doing that would require a lot of refactoring, though, so at the moment I'm going to pass on that one. I decided to model the library in the same way as Haskell's IO is modeled, which also works with exceptions.

I'll make a separate issue out of that one, though, so I won't forget.

[bitemyapp]

@solatis yeah I don't feel strongly about MonadMask/MonadCatch vs. EitherT in this case at all. I don't think the churn is worth it until you've kicked it around and have a feel for things.