Skip to content

Commit

Permalink
util: add helper to avoid long format strings
Browse files Browse the repository at this point in the history
A relatively common pattern in the ghw codebase
is concatenating quite a lot of strings inside
a format string. This leads to a pattern like '%s%s%s%s%s'
which are hard to read and to debug.

Ad a simple helper to concatenate strings to simplify the
format strings.

This may be the definitive solution here, but
it should improve the maintenability and readability,
and it's very cheap to implement and maintain.

Signed-off-by: Francesco Romani <fromani@redhat.com>
  • Loading branch information
ffromani committed Jan 31, 2022
1 parent 4d0ed8f commit dd578ec
Show file tree
Hide file tree
Showing 6 changed files with 75 additions and 21 deletions.
6 changes: 1 addition & 5 deletions pkg/baseboard/baseboard.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,6 @@
package baseboard

import (
"fmt"

"github.com/jaypipes/ghw/pkg/context"
"github.com/jaypipes/ghw/pkg/marshal"
"github.com/jaypipes/ghw/pkg/option"
Expand Down Expand Up @@ -44,14 +42,12 @@ func (i *Info) String() string {
productStr = " product=" + i.Product
}

res := fmt.Sprintf(
"baseboard%s%s%s%s",
return "baseboard" + util.ConcatStrings(
vendorStr,
serialStr,
versionStr,
productStr,
)
return res
}

// New returns a pointer to an Info struct containing information about the
Expand Down
14 changes: 8 additions & 6 deletions pkg/block/block.go
Original file line number Diff line number Diff line change
Expand Up @@ -248,18 +248,20 @@ func (d *Disk) String() string {
removable = " removable=true"
}
return fmt.Sprintf(
"%s %s (%s) %s [@%s%s]%s%s%s%s%s",
"%s %s (%s) %s [@%s%s]%s",
d.Name,
d.DriveType.String(),
sizeStr,
d.StorageController.String(),
d.BusPath,
atNode,
vendor,
model,
serial,
wwn,
removable,
util.ConcatStrings(
vendor,
model,
serial,
wwn,
removable,
),
)
}

Expand Down
6 changes: 1 addition & 5 deletions pkg/chassis/chassis.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,6 @@
package chassis

import (
"fmt"

"github.com/jaypipes/ghw/pkg/context"
"github.com/jaypipes/ghw/pkg/marshal"
"github.com/jaypipes/ghw/pkg/option"
Expand Down Expand Up @@ -81,14 +79,12 @@ func (i *Info) String() string {
versionStr = " version=" + i.Version
}

res := fmt.Sprintf(
"chassis type=%s%s%s%s",
return "chassis type=" + util.ConcatStrings(
i.TypeDescription,
vendorStr,
serialStr,
versionStr,
)
return res
}

// New returns a pointer to a Info struct containing information
Expand Down
6 changes: 1 addition & 5 deletions pkg/product/product.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,6 @@
package product

import (
"fmt"

"github.com/jaypipes/ghw/pkg/context"
"github.com/jaypipes/ghw/pkg/marshal"
"github.com/jaypipes/ghw/pkg/option"
Expand Down Expand Up @@ -57,8 +55,7 @@ func (i *Info) String() string {
versionStr = " version=" + i.Version
}

res := fmt.Sprintf(
"product%s%s%s%s%s%s%s",
return "product" + util.ConcatStrings(
familyStr,
nameStr,
vendorStr,
Expand All @@ -67,7 +64,6 @@ func (i *Info) String() string {
skuStr,
versionStr,
)
return res
}

// New returns a pointer to a Info struct containing information
Expand Down
7 changes: 7 additions & 0 deletions pkg/util/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,3 +50,10 @@ func SafeIntFromFile(ctx *context.Context, path string) int {
}
return res
}

// ConcatStrings concatenate strings in a larger one. This function
// addresses a very specific ghw use case. For a more general approach,
// just use strings.Join()
func ConcatStrings(items ...string) string {
return strings.Join(items, "")
}
57 changes: 57 additions & 0 deletions pkg/util/util_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
//
// Use and distribution licensed under the Apache license version 2.
//
// See the COPYING file in the root project directory for full text.
//

package util_test

import (
"testing"

"github.com/jaypipes/ghw/pkg/util"
)

// nolint: gocyclo
func TestConcatStrings(t *testing.T) {
type testCase struct {
items []string
expected string
}

testCases := []testCase{
{
items: []string{},
expected: "",
},
{
items: []string{"simple"},
expected: "simple",
},
{
items: []string{
"foo",
"bar",
"baz",
},
expected: "foobarbaz",
},
{
items: []string{
"foo ",
" bar ",
" baz",
},
expected: "foo bar baz",
},
}

for _, tCase := range testCases {
t.Run(tCase.expected, func(t *testing.T) {
got := util.ConcatStrings(tCase.items...)
if got != tCase.expected {
t.Errorf("expected %q got %q", tCase.expected, got)
}
})
}
}

0 comments on commit dd578ec

Please sign in to comment.