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

Fix #79 #80

Merged
merged 3 commits into from
May 30, 2018
Merged

Fix #79 #80

merged 3 commits into from
May 30, 2018

Conversation

lehitoskin
Copy link
Owner

Whenever we find an rdf:li that contains more than one element, run string-append and move on.

Copy link
Collaborator

@IonoclastBrigham IonoclastBrigham left a 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.

Copy link
Collaborator

@IonoclastBrigham IonoclastBrigham left a 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])
Copy link
Collaborator

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))
Copy link
Collaborator

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.

Copy link
Collaborator

@IonoclastBrigham IonoclastBrigham left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@IonoclastBrigham IonoclastBrigham merged commit 364b4fd into master May 30, 2018
@lehitoskin lehitoskin deleted the xml-control branch August 4, 2018 15:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants