-
-
Notifications
You must be signed in to change notification settings - Fork 192
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
tree-building: port #835
base: main
Are you sure you want to change the base?
tree-building: port #835
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,83 @@ | ||
# Tree Building | ||
|
||
Refactor a tree building algorithm. | ||
|
||
Some web-forums have a tree layout, so posts are presented as a tree. However | ||
the posts are typically stored in a database as an unsorted set of records. Thus | ||
when presenting the posts to the user the tree structure has to be | ||
reconstructed. | ||
|
||
Your job will be to refactor a working but slow and ugly piece of code that | ||
implements the tree building logic for highly abstracted records. The records | ||
only contain an ID number and a parent ID number. The ID number is always | ||
between 0 (inclusive) and the length of the record list (exclusive). All records | ||
have a parent ID lower than their own ID, except for the root record, which has | ||
a parent ID that's equal to its own ID. | ||
|
||
An example tree: | ||
|
||
```text | ||
root (ID: 0, parent ID: 0) | ||
|-- child1 (ID: 1, parent ID: 0) | ||
| |-- grandchild1 (ID: 2, parent ID: 1) | ||
| +-- grandchild2 (ID: 4, parent ID: 1) | ||
+-- child2 (ID: 3, parent ID: 0) | ||
| +-- grandchild3 (ID: 6, parent ID: 3) | ||
+-- child3 (ID: 5, parent ID: 0) | ||
``` | ||
|
||
|
||
## Getting Started | ||
|
||
For installation and learning resources, refer to the | ||
[exercism help page](http://exercism.io/languages/haskell). | ||
|
||
## Running the tests | ||
|
||
To run the test suite, execute the following command: | ||
|
||
```bash | ||
stack test | ||
``` | ||
|
||
#### If you get an error message like this... | ||
|
||
``` | ||
No .cabal file found in directory | ||
``` | ||
|
||
You are probably running an old stack version and need | ||
to upgrade it. | ||
|
||
#### Otherwise, if you get an error message like this... | ||
|
||
``` | ||
No compiler found, expected minor version match with... | ||
Try running "stack setup" to install the correct GHC... | ||
``` | ||
|
||
Just do as it says and it will download and install | ||
the correct compiler version: | ||
|
||
```bash | ||
stack setup | ||
``` | ||
|
||
## Running *GHCi* | ||
|
||
If you want to play with your solution in GHCi, just run the command: | ||
|
||
```bash | ||
stack ghci | ||
``` | ||
|
||
## Feedback, Issues, Pull Requests | ||
|
||
The [exercism/haskell](https://github.com/exercism/haskell) repository on | ||
GitHub is the home for all of the Haskell exercises. | ||
|
||
If you have feedback about an exercise, or want to help implementing a new | ||
one, head over there and create an issue. We'll do our best to help you! | ||
|
||
## Submitting Incomplete Solutions | ||
It's possible to submit an incomplete solution so you can see how others have completed the exercise. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
name: tree-building | ||
version: 1.0.0.0 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. upon further review of other example package.yaml, I find that we just don't have
sshine marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
dependencies: | ||
- base | ||
|
||
library: | ||
exposed-modules: TreeBuilding | ||
source-dirs: src | ||
ghc-options: -Wall | ||
# dependencies: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. these lines can (and should) be removed from example's package.yaml
sshine marked this conversation as resolved.
Show resolved
Hide resolved
|
||
# - foo # List here the packages you | ||
sshine marked this conversation as resolved.
Show resolved
Hide resolved
|
||
# - bar # want to use in your solution. | ||
sshine marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
tests: | ||
test: | ||
main: Tests.hs | ||
source-dirs: test | ||
dependencies: | ||
- tree-building | ||
- hspec |
Original file line number | Diff line number | Diff line change | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,77 @@ | ||||||||||||
module TreeBuilding (newTree, Record(..), Tree(..)) where | ||||||||||||
|
||||||||||||
import Data.List | ||||||||||||
import Data.Maybe (isNothing) | ||||||||||||
|
||||||||||||
type Id = Int | ||||||||||||
type Children = [Tree] | ||||||||||||
type ParentGrouping = (Maybe Id, [Id]) | ||||||||||||
|
||||||||||||
data Record = Record Id (Maybe Id) deriving (Eq, Show) | ||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since you created these two as helper functions, you might as well create them as record fields instead.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good idea |
||||||||||||
|
||||||||||||
data Tree = Leaf Id | Branch Id Children deriving (Eq, Show) | ||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd go with There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nice. I'll look into that. |
||||||||||||
|
||||||||||||
newTree :: [Record] -> Maybe Tree | ||||||||||||
newTree records | ||||||||||||
| cycles records = Nothing | ||||||||||||
| succIdCheck records = build . groupByParent $ records | ||||||||||||
sshine marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||
| otherwise = Nothing | ||||||||||||
|
||||||||||||
-- Checks for cycling in newTree | ||||||||||||
cycles :: [Record] -> Bool | ||||||||||||
cycles = any checkRecord | ||||||||||||
where checkRecord r = | ||||||||||||
case r of | ||||||||||||
(Record i Nothing) -> if' (i /= 0) True False | ||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh yeah, that should be pretty obvious 😂 |
||||||||||||
(Record i (Just p)) -> if' (p >= i) True False | ||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||
|
||||||||||||
build :: [ParentGrouping] -> Maybe Tree | ||||||||||||
build [] = Nothing | ||||||||||||
build (x:xs) | ||||||||||||
| not . validRoot $ x = Nothing | ||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||
| null xs = Just (Leaf 0) | ||||||||||||
| otherwise = Just (Branch 0 (build' xs)) | ||||||||||||
|
||||||||||||
-- Internal build | ||||||||||||
build' :: [ParentGrouping] -> [Tree] | ||||||||||||
build' xs | ||||||||||||
| length xs == 1 = map Leaf (snd (head xs)) | ||||||||||||
| otherwise = buildWithChildren xs | ||||||||||||
where | ||||||||||||
buildWithChildren (y:ys) = | ||||||||||||
map (\yid -> | ||||||||||||
let children = filter ((== Just yid) . fst) ys | ||||||||||||
in if not (null children) | ||||||||||||
then Branch yid (build' children) | ||||||||||||
else Leaf yid | ||||||||||||
) (snd y) | ||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This function is not exhaustive. I might write it like: build' :: [ParentGrouping] -> [Tree]
build' [(_, yids)] = map Leaf yids
build' ((_, yids):pgs) = map buildWithChildren yids
where
buildWithChildren yid =
let children = [ child | child <- pgs, fst child == Just yid ]
in if null children
then Leaf yid
else Branch yid (build' children) And I might address that it's not exhaustive by concluding that it shouldn't be empty. |
||||||||||||
|
||||||||||||
-- Validate the Root node | ||||||||||||
validRoot :: ParentGrouping -> Bool | ||||||||||||
validRoot (p, ids) = isNothing p && length ids == 1 && sum ids == 0 | ||||||||||||
|
||||||||||||
groupByParent :: [Record] -> [ParentGrouping] | ||||||||||||
groupByParent = sortOn fst | ||||||||||||
. map (\xs -> (recordParent (head xs), map recordId xs)) | ||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. import Control.Arrow ((&&&))
(\x -> (f x, g x)) = f &&& g
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nice. I haven't used the Control.Arrow library before. It is exciting to see it in action here. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I only really use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You may like to know that |
||||||||||||
. groupBy parentsEq | ||||||||||||
. sortOn recordParent | ||||||||||||
. sortOn recordId | ||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this will have the possibly unintuitive consequence that it'll primarily sort on sortBy $ \x y -> comparing recordParent x y `mappend` comparing recordId x y or something similar... There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Cool |
||||||||||||
|
||||||||||||
parentsEq :: Record -> Record -> Bool | ||||||||||||
parentsEq rx ry = recordParent rx == recordParent ry | ||||||||||||
|
||||||||||||
recordParent :: Record -> Maybe Int | ||||||||||||
recordParent (Record _ p) = p | ||||||||||||
|
||||||||||||
recordId :: Record -> Id | ||||||||||||
recordId (Record i _) = i | ||||||||||||
|
||||||||||||
-- verification | ||||||||||||
succIdCheck :: [Record] -> Bool | ||||||||||||
succIdCheck = all (\(x,y) -> succ x == y) . pairwise . sort . map recordId | ||||||||||||
where pairwise = zip <*> tail | ||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure why this check is necessary. Can't a database skip IDs as long as they're monotonically increasing? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Probably. Taking all of the tests from the golang version of this exercise has the concequence of those tests incorporated simply by virtue of their being there. I have not had to deal with the premis of this exercise in the real world. I suppose if users delete comments, then we could definitely end up with trees that have gaps, although a lit of forums seem to keep a marker that it is deleted. I suppose that is what we have to think about here. |
||||||||||||
|
||||||||||||
-- General Tools | ||||||||||||
|
||||||||||||
if' :: Bool -> a -> a -> a | ||||||||||||
if' p a b = if p then a else b | ||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No longer necessary. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
name: tree-building | ||
version: 1.0.0.0 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. since there is no canonical-data.json, by what's written in https://github.com/exercism/haskell/blob/master/README.md#exercise-versioning please use |
||
|
||
dependencies: | ||
- base | ||
|
||
library: | ||
exposed-modules: TreeBuilding | ||
source-dirs: src | ||
ghc-options: -Wall | ||
# dependencies: | ||
# - foo # List here the packages you | ||
# - bar # want to use in your solution. | ||
|
||
tests: | ||
test: | ||
main: Tests.hs | ||
source-dirs: test | ||
dependencies: | ||
- tree-building | ||
- hspec |
Original file line number | Diff line number | Diff line change | ||
---|---|---|---|---|
@@ -0,0 +1,49 @@ | ||||
module TreeBuilding (newTree, Record(..), Tree(..)) where | ||||
|
||||
import Data.List | ||||
import Data.Ord (comparing) | ||||
--import Data.Maybe (fromMaybe) | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure an unused import is necessary in the stub.
Suggested change
|
||||
|
||||
type Id = Int | ||||
type Children = [Tree] | ||||
|
||||
data Record = Record Id (Maybe Id) deriving (Show) | ||||
|
||||
data Tree = Leaf Id | Branch Id Children deriving (Eq, Show) | ||||
|
||||
newTree :: [Record] -> Maybe Tree | ||||
newTree records | ||||
| cycleCheck records = Nothing | ||||
| succIdCheck records == True = build . groupByParent $ records | ||||
| otherwise = Nothing | ||||
where | ||||
recordParent = \(Record _ p) -> p | ||||
recordId = \(Record i _) -> i | ||||
succIdCheck = all (\(x,y) -> succ x == y) . pairwise . sort . map recordId | ||||
where pairwise = zip <*> tail | ||||
cycleCheck = any id . map (\(r) -> case r of | ||||
(Record i Nothing) -> if i == 0 | ||||
then False | ||||
else True | ||||
(Record i (Just p)) -> if p >= i | ||||
then True | ||||
else False) | ||||
groupByParent = sortOn fst . map (\xs -> (recordParent (head xs), map recordId xs)) . groupBy parentsEq . sortBy (comparing recordParent) . sortBy (comparing recordId) | ||||
parentsEq rx ry = recordParent rx == recordParent ry | ||||
build [] = Nothing | ||||
build (x:xs) | ||||
| not . rootCheck $ x = Nothing | ||||
| xs == [] = Just (Leaf 0) | ||||
| otherwise = Just (Branch 0 (recursiveBuilding xs)) | ||||
where | ||||
rootCheck (p,ids) | ||||
| p /= Nothing || (length ids /= 1 && (head ids) /= 0) = False | ||||
| otherwise = True | ||||
recursiveBuilding ys | ||||
| length ys == 1 = map (\yid -> Leaf yid) (snd (head ys)) | ||||
| otherwise = (\(z:zs) -> map (\xid -> | ||||
let children = filter ((== (Just xid)) . fst) zs | ||||
in if length children > 0 | ||||
then Branch xid (recursiveBuilding children) | ||||
else Leaf xid | ||||
) (snd z)) ys |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
resolver: lts-12.4 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
since there is no canonical-data.json, by what's written in https://github.com/exercism/haskell/blob/master/README.md#exercise-versioning please use
0.1.0.s
instead of1.0.0.s
version numbersThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, cool. I wasn't sure of the protocol.