-
Notifications
You must be signed in to change notification settings - Fork 3
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
Fix #79 #80
Fix #79 #80
Conversation
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.
Seems like a good opportunity extract out the common parts of the (if len (apply string-append ...
pattern into a function. Could possibly leverage some currying or higher-order function goodness to make it fit across the handful of variations you've got.
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.
map
seems like a better choice in this case.
embed.rkt
Outdated
; fixes issues when we have multiple elements inside a single rdf:li | ||
(define/contract (rdf:li-fixer rdf:li) | ||
((listof is-rdf:li?) . -> . list?) | ||
(for/fold ([lst empty]) |
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.
So if we're iterating over a list, and outputting a new list that's the same length, that is starting to look a lot like a candidate for map
instead of for/fold
. Then we don't have to manage an explicit accumulator or worry about manually appending anything.
Consider the following snippet, which is pretty artificial, but incorporates all the same basic moving parts as rdf:li-fixer
:
#lang racket
(map (λ (elem)
(if (> (length elem) 1)
(apply string-append elem)
elem))
(map (λ (num) (list "0" (number->string num))) (list 1 2 3 4)))
embed.rkt
Outdated
(append lst (list (apply string-append elem))) | ||
(append lst elem)))) | ||
(is-rdf:li? . -> . string?) | ||
(define elem (get-elements rdf:li)) |
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.
Ah, yes. I like this better.
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.
👍
Whenever we find an
rdf:li
that contains more than one element, runstring-append
and move on.