dillonkearns / elm-pages-starter

Starter blog for elm-pages
https://elm-pages-starter.netlify.com
95 stars 41 forks source link

Improve code style of DocumentSvg #18

Closed sparksp closed 4 years ago

sparksp commented 4 years ago
dillonkearns commented 4 years ago

Hey, thanks again for all the PRs!

In the context of a module that is only serving as an SVG, I actually like having those values exposed. I try to avoid that in cases where there are several different responsibilities in a module, but I think it's clear since this is a very focused module that all of those unqualified functions are related to SVG since that's the only thing this module is concerned with.

I like the dead code removal, added type annotations, and breaking up the long lines. But I think I'd prefer to keep the unqualified functions for this particular module. What do you think?

sparksp commented 4 years ago

The biggest reason for not exposing all is usually to make it clear where functions and types are coming from. As you say this is a really focussed module so it shouldn't be a surprise where things are coming from, although there's still two modules to figure out here. If you understand how the modules interact (nodes vs attributes) then that probably makes complete sense, but if you're new to it then that might not be so obvious.

Just having this conversation documented here is probably a good reference for anyone looking for a reason why this breaks the trend.

dillonkearns commented 4 years ago

Looks good! 👍