Re: [go-nuts] new project: pixels to svg

[Nigel Tao]

On Sat, Sep 2, 2017 at 12:50 AM,   wrote:
> Feel free to give me feedback on how I can improve the code or the project
> setup, etc.

I had a quick look. Some thoughts on style, in no particular order:

0. Run gofmt. Your code mixes tabs and spaces, amongst other issues.
Gofmt will fix that.

1. Run "gofmt -s" to simplify your code. For example, the inner "[2]int" in
outlinePoints := [][2]int{[2]int{colX, rowY}}
is redundant.

2. In methods like GetSVGText, string concatenation inside a loop,
"nextSVG += fmt.Sprintf(etc)", has quadratic complexity. Better is to
use a bytes.Buffer and fmt.Fprintf (with a F, not a S).

3. Some of your backslash-rich string literals, the format arguments
to fmt.Sprintf, might be better as backtick strings: `x1="%d"` instead
of "x1=\"%d\"".

4. If Uint8ToHex is an internal function, not supposed to be used by
code that imports that package, then don't export it: change the
leading U to u. This makes it easier to see what API actually matters
in the package's godoc output.

5. The same applies to other helper functions like Split2Int.

6. But you don't need the Uint8ToHex function at all. Instead,
GetHexColor can return fmt.Sprintf("#%02X%02X%02X", etc, etc, etc).

7. Don't panic on errors. Delete your check function. The
WriteSVGToFile method should return an error instead. Callers, instead
of callees, usually have more context on e.g. whether an error is
fatal or recoverable.

8. Lines like "return [][2]int{}" can probably be the simpler "return
nil". A nil slice is a valid (empty) slice: it has 0 length, you can
range over it (0 times), you can append to it, etc.

9. The [4]uint8 type could probably be replaced by either the
color.RGBA or color.NRGBA type from the standard library's image/color
package. It shouldn't matter much in terms of the actual code
generated, but it might make it clearer what the semantics of those 4
uint8s are. It would also clarify whether your [4]uint8 is
alpha-premultiplied or non-alpha-premultiplied.

10. In getNeighborEvaluator, you have:
neighborEvaluators := map[int]func(int, int, [4]uint8) bool{
0: etc,
1: etc,
etc,
}
return neighborEvaluators[direction]
Using a map[int]etc here is overkill, as the keys are just a dense
range of natural numbers. An [8]etc would be more efficient. You could
also make that array a global instead of having to build it up on
every method call.

-- 
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.

[go-nuts] new project: pixels to svg

[baggerone]

Based on the feedback I received, I've started a new little go project on 
Github - https://github.com/Baggerone/gopixels2svg

Feel free to give me feedback on how I can improve the code or the project 
setup, etc.

To get a quick idea of how to use it, look at *TestWriteSVGToFile *in 
*pixels2svg_test.go*

-- 
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.