I've taken note of all your comments. I've added some to my todo list as per the package and made a few improvements.
Note that none of these are "the right way", they give you options to make > the code more reusable. I completely get this. Thanks for your time. Uzondu On Wed, Oct 3, 2018 at 2:08 PM Tristan Colgate <tcolg...@gmail.com> wrote: > In case you haven't already checked it out, you can now see your package > at: > > https://godoc.org/github.com/willpoint/treemap > > You can do a sort of "GoDoc Driven Design" here, look at the godoc, and > decide if the interface is something you would like to use. > > Some suggestions: > > - A destination io.Writer is tradionally places as the first argument to a > function. > - DrawTreemap doesn't actually mention that the package writes SVG, the > function names could make it clearer. > - Along a similar line, maybe you could have a DrawToSVG that took an > *svg.SVG instead of a writer (or, better still, define an WriteSVG > interface that just includes the methods on the SVG that you use, this > would allow others to swap out the SVG implementation that was used. This > could allow an SVG to be drawn into an existing svg the user is building up > in sections.. DrawTreemap could still fall back to the default one). > - similarly, you could provide a method that draws to a pre-existing image > - in some scenarios, width/height might be inferrable, from some container > you might want to draw something in to. > - building on a richer underlying drawing abstraction might make things > more re-usable, The SVG interface thing above helps with that, you could > also look at something like https://godoc.org/github.com/llgcode/draw2d > (though > that obviously introduces depedencies). > > Note that none of these are "the right way", they give you options to make > the code more reusable. > > > On Wed, 3 Oct 2018 at 13:49 Uzondu Enudeme <willpo...@gmail.com> wrote: > >> Hi Tristan, >> >> Thanks again for your helpful feedback. Based on your observations, >> here's what I've done so far: >> >> This would be more useful as a package than a single program. You could >>> move main into an example/ folder and keep the rest of the content in the >>> top level as a package. Move the explanatory comment up as a Package >>> comment. >> >> >> I have now moved main to an example folder and left the rest as a >> package. I then defined an interface `TreeMapper` that represents any type >> that can be drawn by the treemap's DrawTreemap function. >> >> The standard library includes color handling, check the image package. >> >> I haven't yet figured this out. I'd look more carefully into the image >> package. >> >> The strconv wrapper is probably a bit excessive for the sake of the cast >>> (you might not need it if you use color.Color anyway) >> >> I removed the strconv wrapper, it really seemed excessive. I'm still yet >> to implement the use of color. >> >> The orientation might be better off as a tyoed parameter, look for docs >>> on typed constants. >> >> >> I read this article <https://blog.golang.org/constants> on the subject >> and found it really great! I have updated to code to reflect that. >> >> Any further advice would be greatly appreciated. The link still remains >> https://github.com/willpoint/treemap >> >> Thanks, >> >> Uzondu >> >> On Wed, Oct 3, 2018, 09:13 Uzondu Enudeme <willpo...@gmail.com> wrote: >> >>> Thanks a lot Tristan, >>> >>> The points you mentioned are really all very important ones. >>> >>> I'm currently following all your suggestions and would repost the link >>> to the updated code in case I missed some points. >>> >>> >>> On Wed, Oct 3, 2018, 06:52 Tristan Colgate <tcolg...@gmail.com> wrote: >>> >>>> A couple of quick observations. >>>> >>>> This would be more useful as a package than a single program. You could >>>> move main into an example/ folder and keep the rest of the content in the >>>> top level as a package. Move the explanatory comment up as a Package >>>> comment. >>>> >>>> The standard library includes color handling, check the image package. >>>> >>>> The strconv wrapper is probably a bit excessive for the sake of the >>>> cast (you might not need it if you use color.Color anyway) >>>> >>>> The orientation might be better off as a tyoed parameter, look for docs >>>> on typed constants. >>>> >>>> >>>> >>>> >>>> >>>> On Tue, 2 Oct 2018, 23:06 Uzondu Enudeme, <willpo...@gmail.com> wrote: >>>> >>>>> Hi everyone, >>>>> >>>>> I've been following the golang-nuts' email threads for a while now and >>>>> I'm amazed at how much I've learnt seeing ways solutions are profferred. >>>>> >>>>> I implemented a treemap for data visualization, and wrote a process.md >>>>> file giving some sort of background and detail. >>>>> >>>>> I want to use this opportunity to learn what I could have done better >>>>> by asking that the code be reviewed, it's in a single main.go file at >>>>> github.com/willpoint/treemap >>>>> >>>>> Thanks in advance for the time that would be spent and any advice >>>>> given. >>>>> >>>>> >>>>> Uzondu >>>>> >>>>> >>>>> -- >>>>> You received this message because you are subscribed to the Google >>>>> Groups "golang-nuts" group. >>>>> To unsubscribe from this group and stop receiving emails from it, send >>>>> an email to golang-nuts+unsubscr...@googlegroups.com. >>>>> For more options, visit https://groups.google.com/d/optout. >>>>> >>>> -- You received this message because you are subscribed to the Google Groups "golang-nuts" group. To unsubscribe from this group and stop receiving emails from it, send an email to golang-nuts+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
