Skip to content

Commit

Permalink
Style and minor performance improvements
Browse files Browse the repository at this point in the history
This change updates `golangci-lint` to include a range of linters not previously used. The goal of this is to help maintain a consistent style across the project.

Style changes were made as found from the linter, in addition to some manual review.

Performance gains were from preallocating slice sizes and also reducing the usage of `fmt.Sprintf`.
  • Loading branch information
jentfoo committed Jan 20, 2025
1 parent 0fa9eab commit ca9fb88
Show file tree
Hide file tree
Showing 93 changed files with 587 additions and 653 deletions.
3 changes: 2 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -14,4 +14,5 @@ bench:
go test -parallel=$(CORES_HALF) --benchmem -benchtime=20s -bench=. -run='^$$' ./...

lint:
golangci-lint run
golangci-lint run --timeout=600s --enable=asasalint,asciicheck,bidichk,containedctx,contextcheck,exportloopref,decorder,durationcheck,errorlint,exptostd,fatcontext,forbidigo,gocheckcompilerdirectives,gochecksumtype,goconst,gofmt,goimports,gosmopolitan,grouper,iface,importas,mirror,misspell,nilerr,nilnil,perfsprint,prealloc,reassign,recvcheck,sloglint,testifylint,unconvert,wastedassign,whitespace

14 changes: 7 additions & 7 deletions assert_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,25 +6,25 @@ import (
"strings"
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

func assertEqualSVG(t *testing.T, expected string, actual []byte) {
t.Helper()

actualStr := string(actual)
if expected != actualStr {
actualFile, err := writeTempFile(actual, t.Name()+"-actual", "svg")
assert.NoError(t, err)
actualFile, err1 := writeTempFile(actual, t.Name()+"-actual", "svg")

if expected == "" {
t.Fatalf("SVG written to %s", actualFile)
t.Errorf("SVG written to %s", actualFile)
} else {
expectedFile, err := writeTempFile([]byte(expected), t.Name()+"-expected", "svg")
assert.NoError(t, err)
t.Fatalf("SVG content does not match. Expected file: %s, Actual file: %s",
expectedFile, err2 := writeTempFile([]byte(expected), t.Name()+"-expected", "svg")
t.Errorf("SVG content does not match. Expected file: %s, Actual file: %s",
expectedFile, actualFile)
require.NoError(t, err2)
}
require.NoError(t, err1)
}
}

Expand Down
3 changes: 1 addition & 2 deletions axis.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,8 +97,7 @@ func (a *axisPainter) Render() (Box, error) {

textMaxWidth, textMaxHeight := top.measureTextMaxWidthHeight(opt.Data, opt.TextRotation, fontStyle)

width := 0
height := 0
var width, height int
if isVertical {
width = textMaxWidth + tickLength<<1
height = top.Height()
Expand Down
12 changes: 9 additions & 3 deletions benchmark_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,9 @@ func renderPainterMultiChartPNGRender(painter *Painter) {
Max: FloatPointer(90),
},
}
painter.LineChart(lineOpt)
if err := painter.LineChart(lineOpt); err != nil {
panic(err)
}

barOpt := NewBarChartOptionWithData([][]float64{
{40.1, 62.2, 69.5, 36.4, 45.2, 32.5},
Expand All @@ -152,7 +154,9 @@ func renderPainterMultiChartPNGRender(painter *Painter) {
barOpt.Legend = lineOpt.Legend
barOpt.XAxis = lineOpt.XAxis
barOpt.YAxis = lineOpt.YAxis
painter.BarChart(barOpt)
if err := painter.BarChart(barOpt); err != nil {
panic(err)
}

pieOpt := PieChartOption{
SeriesList: NewSeriesListPie([]float64{
Expand All @@ -169,7 +173,9 @@ func renderPainterMultiChartPNGRender(painter *Painter) {
Theme: GetDefaultTheme(),
Font: GetDefaultFont(),
}
painter.PieChart(pieOpt)
if err := painter.PieChart(pieOpt); err != nil {
panic(err)
}

if buf, err := painter.Bytes(); err != nil {
panic(err)
Expand Down
4 changes: 2 additions & 2 deletions chart_option.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package charts

import (
"fmt"
"errors"

"github.com/golang/freetype/truetype"
)
Expand Down Expand Up @@ -228,7 +228,7 @@ func (o *ChartOption) fillDefault() error {

yaxisCount := o.SeriesList.getYAxisCount()
if yaxisCount < 0 {
return fmt.Errorf("series specified invalid y-axis index")
return errors.New("series specified invalid y-axis index")
}
if len(o.YAxis) < yaxisCount {
yAxisOptions := make([]YAxisOption, yaxisCount)
Expand Down
4 changes: 2 additions & 2 deletions chartdraw/annotation_series.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package chartdraw

import (
"fmt"
"errors"
"math"
)

Expand Down Expand Up @@ -87,7 +87,7 @@ func (as AnnotationSeries) Render(r Renderer, canvasBox Box, xrange, yrange Rang
// Validate validates the series.
func (as AnnotationSeries) Validate() error {
if len(as.Annotations) == 0 {
return fmt.Errorf("annotation series requires annotations to be set and not empty")
return errors.New("annotation series requires annotations to be set and not empty")
}
return nil
}
13 changes: 4 additions & 9 deletions chartdraw/bar_chart.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package chartdraw

import (
"errors"
"fmt"
"io"
"math"

Expand Down Expand Up @@ -33,8 +32,7 @@ type BarChart struct {
UseBaseValue bool
BaseValue float64

Font *truetype.Font
defaultFont *truetype.Font
Font *truetype.Font

Bars []Value
Elements []Renderable
Expand All @@ -50,9 +48,6 @@ func (bc BarChart) GetDPI() float64 {

// GetFont returns the text font.
func (bc BarChart) GetFont() *truetype.Font {
if bc.Font == nil {
return bc.defaultFont
}
return bc.Font
}

Expand Down Expand Up @@ -97,7 +92,7 @@ func (bc BarChart) Render(rp RendererProvider, w io.Writer) error {
r := rp(bc.GetWidth(), bc.GetHeight())

if bc.Font == nil {
bc.defaultFont = GetDefaultFont()
bc.Font = GetDefaultFont()
}
r.SetDPI(bc.GetDPI())

Expand All @@ -111,7 +106,7 @@ func (bc BarChart) Render(rp RendererProvider, w io.Writer) error {
canvasBox = bc.getDefaultCanvasBox()
yr = bc.getRanges()
if yr.GetMax()-yr.GetMin() == 0 {
return fmt.Errorf("invalid data range; cannot be zero")
return errors.New("invalid data range; cannot be zero")
}
yr = bc.setRangeDomains(canvasBox, yr)
yf = bc.getValueFormatters()
Expand Down Expand Up @@ -367,7 +362,7 @@ func (bc BarChart) getAdjustedCanvasBox(r Renderer, canvasBox Box, yrange Range,

cursor := canvasBox.Left
for _, bar := range bc.Bars {
if len(bar.Label) > 0 {
if bar.Label != "" {
barLabelBox := Box{
Top: canvasBox.Bottom + DefaultXAxisMargin,
Left: cursor,
Expand Down
30 changes: 15 additions & 15 deletions chartdraw/bar_chart_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,9 @@ func TestBarChartProps(t *testing.T) {

bc := BarChart{}

assert.Equal(t, DefaultDPI, bc.GetDPI())
assert.InDelta(t, DefaultDPI, bc.GetDPI(), 0)
bc.DPI = 100
assert.Equal(t, float64(100), bc.GetDPI())
assert.InDelta(t, float64(100), bc.GetDPI(), 0)

assert.Nil(t, bc.GetFont())
bc.Font = GetDefaultFont()
Expand Down Expand Up @@ -91,8 +91,8 @@ func TestBarChartGetRanges(t *testing.T) {
assert.NotNil(t, yr)
assert.False(t, yr.IsZero())

assert.Equal(t, -math.MaxFloat64, yr.GetMax())
assert.Equal(t, math.MaxFloat64, yr.GetMin())
assert.InDelta(t, -math.MaxFloat64, yr.GetMax(), 0)
assert.InDelta(t, math.MaxFloat64, yr.GetMin(), 0)
}

func TestBarChartGetRangesBarsMinMax(t *testing.T) {
Expand All @@ -109,8 +109,8 @@ func TestBarChartGetRangesBarsMinMax(t *testing.T) {
assert.NotNil(t, yr)
assert.False(t, yr.IsZero())

assert.Equal(t, float64(10), yr.GetMax())
assert.Equal(t, float64(1), yr.GetMin())
assert.InDelta(t, float64(10), yr.GetMax(), 0)
assert.InDelta(t, float64(1), yr.GetMin(), 0)
}

func TestBarChartGetRangesMinMax(t *testing.T) {
Expand All @@ -137,8 +137,8 @@ func TestBarChartGetRangesMinMax(t *testing.T) {
assert.NotNil(t, yr)
assert.False(t, yr.IsZero())

assert.Equal(t, float64(15), yr.GetMax())
assert.Equal(t, float64(5), yr.GetMin())
assert.InDelta(t, float64(15), yr.GetMax(), 0)
assert.InDelta(t, float64(5), yr.GetMin(), 0)
}

func TestBarChartGetRangesTicksMinMax(t *testing.T) {
Expand All @@ -161,8 +161,8 @@ func TestBarChartGetRangesTicksMinMax(t *testing.T) {
assert.NotNil(t, yr)
assert.False(t, yr.IsZero())

assert.Equal(t, float64(11), yr.GetMax())
assert.Equal(t, float64(7), yr.GetMin())
assert.InDelta(t, float64(11), yr.GetMax(), 0)
assert.InDelta(t, float64(7), yr.GetMin(), 0)
}

func TestBarChartHasAxes(t *testing.T) {
Expand Down Expand Up @@ -294,13 +294,13 @@ func TestBarChatGetTitleFontSize(t *testing.T) {
t.Parallel()

size := BarChart{Width: 2049, Height: 2049}.getTitleFontSize()
assert.Equal(t, float64(48), size)
assert.InDelta(t, float64(48), size, 0)
size = BarChart{Width: 1025, Height: 1025}.getTitleFontSize()
assert.Equal(t, float64(24), size)
assert.InDelta(t, float64(24), size, 0)
size = BarChart{Width: 513, Height: 513}.getTitleFontSize()
assert.Equal(t, float64(18), size)
assert.InDelta(t, float64(18), size, 0)
size = BarChart{Width: 257, Height: 257}.getTitleFontSize()
assert.Equal(t, float64(12), size)
assert.InDelta(t, float64(12), size, 0)
size = BarChart{Width: 128, Height: 128}.getTitleFontSize()
assert.Equal(t, float64(10), size)
assert.InDelta(t, float64(10), size, 0)
}
24 changes: 18 additions & 6 deletions chartdraw/benchmark_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,14 +38,18 @@ func makeBenchmarkLineChart() Chart {
func BenchmarkLineChartPNGRender(b *testing.B) {
for i := 0; i < b.N; i++ {
buffer := bytes.Buffer{}
makeBenchmarkLineChart().Render(PNG, &buffer)
if err := makeBenchmarkLineChart().Render(PNG, &buffer); err != nil {
panic(err)
}
}
}

func BenchmarkLineChartSVGRender(b *testing.B) {
for i := 0; i < b.N; i++ {
buffer := bytes.Buffer{}
makeBenchmarkLineChart().Render(SVG, &buffer)
if err := makeBenchmarkLineChart().Render(SVG, &buffer); err != nil {
panic(err)
}
}
}

Expand Down Expand Up @@ -75,14 +79,18 @@ func makeBenchmarkBarChart() BarChart {
func BenchmarkBarChartPNGRender(b *testing.B) {
for i := 0; i < b.N; i++ {
buffer := bytes.Buffer{}
makeBenchmarkBarChart().Render(PNG, &buffer)
if err := makeBenchmarkBarChart().Render(PNG, &buffer); err != nil {
panic(err)
}
}
}

func BenchmarkBarChartSVGRender(b *testing.B) {
for i := 0; i < b.N; i++ {
buffer := bytes.Buffer{}
makeBenchmarkBarChart().Render(SVG, &buffer)
if err := makeBenchmarkBarChart().Render(SVG, &buffer); err != nil {
panic(err)
}
}
}

Expand Down Expand Up @@ -115,13 +123,17 @@ func makeBenchmarkPieChart() PieChart {
func BenchmarkPieChartPNGRender(b *testing.B) {
for i := 0; i < b.N; i++ {
buffer := bytes.Buffer{}
makeBenchmarkPieChart().Render(PNG, &buffer)
if err := makeBenchmarkPieChart().Render(PNG, &buffer); err != nil {
panic(err)
}
}
}

func BenchmarkPieChartSVGRender(b *testing.B) {
for i := 0; i < b.N; i++ {
buffer := bytes.Buffer{}
makeBenchmarkPieChart().Render(SVG, &buffer)
if err := makeBenchmarkPieChart().Render(SVG, &buffer); err != nil {
panic(err)
}
}
}
4 changes: 2 additions & 2 deletions chartdraw/bollinger_band_series.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package chartdraw

import (
"fmt"
"errors"
)

// Interface Assertions.
Expand Down Expand Up @@ -129,7 +129,7 @@ func (bbs *BollingerBandsSeries) Render(r Renderer, canvasBox Box, xrange, yrang
// Validate validates the series.
func (bbs *BollingerBandsSeries) Validate() error {
if bbs.InnerSeries == nil {
return fmt.Errorf("bollinger bands series requires InnerSeries to be set")
return errors.New("bollinger bands series requires InnerSeries to be set")
}
return nil
}
9 changes: 4 additions & 5 deletions chartdraw/bollinger_band_series_test.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package chartdraw

import (
"fmt"
"math"
"testing"

Expand Down Expand Up @@ -29,7 +28,7 @@ func TestBollingerBandSeries(t *testing.T) {
}

for x := bbs.GetPeriod(); x < 100; x++ {
assert.True(t, y1values[x] > y2values[x], fmt.Sprintf("%v vs. %v", y1values[x], y2values[x]))
assert.Greater(t, y1values[x], y2values[x])
}
}

Expand All @@ -46,7 +45,7 @@ func TestBollingerBandLastValue(t *testing.T) {
}

x, y1, y2 := bbs.GetBoundedLastValues()
assert.Equal(t, 100.0, x)
assert.Equal(t, float64(101), math.Floor(y1))
assert.Equal(t, float64(83), math.Floor(y2))
assert.InDelta(t, 100.0, x, 0)
assert.InDelta(t, float64(101), math.Floor(y1), 0)
assert.InDelta(t, float64(83), math.Floor(y2), 0)
}
9 changes: 5 additions & 4 deletions chartdraw/box.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package chartdraw

import (
"errors"
"fmt"
"math"
)
Expand Down Expand Up @@ -305,16 +306,16 @@ func (b Box) OuterConstrain(bounds, other Box) Box {

func (b Box) Validate() error {
if b.Left < 0 {
return fmt.Errorf("invalid left; must be >= 0")
return errors.New("invalid left; must be >= 0")
}
if b.Right < 0 {
return fmt.Errorf("invalid right; must be > 0")
return errors.New("invalid right; must be > 0")
}
if b.Top < 0 {
return fmt.Errorf("invalid top; must be > 0")
return errors.New("invalid top; must be > 0")
}
if b.Bottom < 0 {
return fmt.Errorf("invalid bottom; must be > 0")
return errors.New("invalid bottom; must be > 0")
}
return nil
}
Expand Down
Loading

0 comments on commit ca9fb88

Please sign in to comment.