Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor Stringly Typed Functions to Properly typed functions #25

Closed
tmspzz opened this issue Oct 1, 2016 · 4 comments
Closed

Refactor Stringly Typed Functions to Properly typed functions #25

tmspzz opened this issue Oct 1, 2016 · 4 comments

Comments

@tmspzz
Copy link
Owner

tmspzz commented Oct 1, 2016

Refactor all the type Something = String to newtype Something = Something String

Steps which explain the enhancement

  1. In Data.Cartfile refactor:
type Location = String
type Version = String

to

newtype Location = Location String
newtype Version = Version String
  1. In Data.Romefile refactor:
type FrameworkName = String
type GitRepoName = String

to

newtype FrameworkName = FrameworkName String
newtype GitRepoName  = GitRepoName String
  1. Adapt the remaining code to the new types

Current and suggested behavior

Currently some functions are stirngly typed and this leads to quite some confusion around them.
For example in deriveFrameworkNameAndVersion the return type is [(FrameworkName, Version)] which is just an alias for [(String, String)]. This is not optimal as it's possible to mix up FrameworkName with Version later down then code.

If FrameworkName and Version were defined using newtype instead of type mixing FrameworkName and Version would not be possible.

Rome version: v.0.6.1.11
OS and version: MacOSX 10.11.6 - El Capitan

@2016rshah
Copy link
Contributor

I'd be happy to work on this!

@tmspzz
Copy link
Owner Author

tmspzz commented Oct 1, 2016

@2016rshah Please go ahead.

@tmspzz
Copy link
Owner Author

tmspzz commented Oct 1, 2016

@2016rshah Once you are done I can open another issue and if you know QuickCheck you could write some test for the refactored code too.

@2016rshah 2016rshah mentioned this issue Oct 2, 2016
@tmspzz tmspzz closed this as completed in ef1cfac Oct 2, 2016
@tmspzz
Copy link
Owner Author

tmspzz commented Oct 2, 2016

@2016rshah Thanks for your contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants