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 make-screen-heads #344
Conversation
(setf (head-number head) | ||
(incf head-index)))) | ||
(mapc #'set-head-number heads)))) | ||
|
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.
(loop for head in heads
for i upfrom 0
do (setf (head-number head) i))
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.
@Harleqin Yes, that is the code being replaced with a mapc, because it has a higher information density. Ideally I'd use misc-extensions's rflet so the flet would be at the end instead of cluttering the relevant code. Do you think the loop version is more readable in this case?
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.
Yes, absolutely, it also feels more declarative to me.
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.
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.
Honestly the flet+incf+mapc seems like scratching your nose behind your head to keep it functional. The operation is inherently declarative and its easier to mentally parse the loop version.
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.
K, I'll go back to using loop
Refactor make-screen-heads
This is just a refactor or the current head implemenatation to make it more readable. It is a first stop towards #328, which seems to have gone cold