Skip to content

Commit 0238324

Browse files
jedib0tjedib0t
authored
text: transformer code quality fixes (#380)
1 parent a9ab184 commit 0238324

File tree

2 files changed

+87
-80
lines changed

2 files changed

+87
-80
lines changed

‎text/transformer.go‎

Lines changed: 68 additions & 80 deletions
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,15 @@ import (
1111

1212
// Transformer related constants
1313
const (
14-
unixTimeMinMilliseconds = int64(10000000000)
15-
unixTimeMinMicroseconds = unixTimeMinMilliseconds * 1000
16-
unixTimeMinNanoSeconds = unixTimeMinMicroseconds * 1000
14+
// Pre-computed time conversion constants to avoid repeated calculations
15+
nanosPerSecond = int64(time.Second)
16+
microsPerSecond = nanosPerSecond / 1000
17+
millisPerSecond = nanosPerSecond / 1000000
18+
19+
// Thresholds for detecting unix timestamp units (10 seconds worth in each unit)
20+
unixTimeMinMilliseconds = 10 * nanosPerSecond
21+
unixTimeMinMicroseconds = 10 * nanosPerSecond * 1000
22+
unixTimeMinNanoSeconds = 10 * nanosPerSecond * 1000000
1723
)
1824

1925
// Transformer related variables
@@ -40,103 +46,84 @@ type Transformer func(val interface{}) string
4046
// - transforms the number as directed by 'format' (ex.: %.2f)
4147
// - colors negative values Red
4248
// - colors positive values Green
49+
//
50+
//gocyclo:ignore
4351
func NewNumberTransformer(format string) Transformer {
44-
return func(val interface{}) string {
45-
if valStr := transformInt(format, val); valStr != "" {
46-
return valStr
47-
}
48-
if valStr := transformUint(format, val); valStr != "" {
49-
return valStr
50-
}
51-
if valStr := transformFloat(format, val); valStr != "" {
52-
return valStr
53-
}
54-
return fmt.Sprint(val)
55-
}
56-
}
52+
// Pre-compute negative format string to avoid repeated allocations
53+
negFormat := "-" + format
5754

58-
func transformInt(format string, val interface{}) string {
59-
transform := func(val int64) string {
55+
transformInt64 := func(val int64) string {
6056
if val < 0 {
61-
return colorsNumberNegative.Sprintf("-"+format, -val)
57+
return colorsNumberNegative.Sprintf(negFormat, -val)
6258
}
6359
if val > 0 {
6460
return colorsNumberPositive.Sprintf(format, val)
6561
}
6662
return colorsNumberZero.Sprintf(format, val)
6763
}
6864

69-
if number, ok := val.(int); ok {
70-
return transform(int64(number))
71-
}
72-
if number, ok := val.(int8); ok {
73-
return transform(int64(number))
74-
}
75-
if number, ok := val.(int16); ok {
76-
return transform(int64(number))
77-
}
78-
if number, ok := val.(int32); ok {
79-
return transform(int64(number))
80-
}
81-
if number, ok := val.(int64); ok {
82-
return transform(number)
83-
}
84-
return ""
85-
}
86-
87-
func transformUint(format string, val interface{}) string {
88-
transform := func(val uint64) string {
65+
transformUint64 := func(val uint64) string {
8966
if val > 0 {
9067
return colorsNumberPositive.Sprintf(format, val)
9168
}
9269
return colorsNumberZero.Sprintf(format, val)
9370
}
9471

95-
if number, ok := val.(uint); ok {
96-
return transform(uint64(number))
97-
}
98-
if number, ok := val.(uint8); ok {
99-
return transform(uint64(number))
100-
}
101-
if number, ok := val.(uint16); ok {
102-
return transform(uint64(number))
103-
}
104-
if number, ok := val.(uint32); ok {
105-
return transform(uint64(number))
106-
}
107-
if number, ok := val.(uint64); ok {
108-
return transform(number)
109-
}
110-
return ""
111-
}
112-
113-
func transformFloat(format string, val interface{}) string {
114-
transform := func(val float64) string {
72+
transformFloat64 := func(val float64) string {
11573
if val < 0 {
116-
return colorsNumberNegative.Sprintf("-"+format, -val)
74+
return colorsNumberNegative.Sprintf(negFormat, -val)
11775
}
11876
if val > 0 {
11977
return colorsNumberPositive.Sprintf(format, val)
12078
}
12179
return colorsNumberZero.Sprintf(format, val)
12280
}
12381

124-
if number, ok := val.(float32); ok {
125-
return transform(float64(number))
126-
}
127-
if number, ok := val.(float64); ok {
128-
return transform(number)
82+
// Use type switch for O(1) type checking instead of sequential type assertions
83+
return func(val interface{}) string {
84+
switch v := val.(type) {
85+
case int:
86+
return transformInt64(int64(v))
87+
case int8:
88+
return transformInt64(int64(v))
89+
case int16:
90+
return transformInt64(int64(v))
91+
case int32:
92+
return transformInt64(int64(v))
93+
case int64:
94+
return transformInt64(v)
95+
case uint:
96+
return transformUint64(uint64(v))
97+
case uint8:
98+
return transformUint64(uint64(v))
99+
case uint16:
100+
return transformUint64(uint64(v))
101+
case uint32:
102+
return transformUint64(uint64(v))
103+
case uint64:
104+
return transformUint64(v)
105+
case float32:
106+
return transformFloat64(float64(v))
107+
case float64:
108+
return transformFloat64(v)
109+
default:
110+
return fmt.Sprint(val)
111+
}
129112
}
130-
return ""
131113
}
132114

133115
// NewJSONTransformer returns a Transformer that can format a JSON string or an
134116
// object into pretty-indented JSON-strings.
135117
func NewJSONTransformer(prefix string, indent string) Transformer {
136118
return func(val interface{}) string {
137119
if valStr, ok := val.(string); ok {
120+
valStr = strings.TrimSpace(valStr)
121+
// Validate JSON before attempting to indent to avoid unnecessary processing
122+
if !json.Valid([]byte(valStr)) {
123+
return fmt.Sprintf("%#v", valStr)
124+
}
138125
var b bytes.Buffer
139-
if err := json.Indent(&b, []byte(strings.TrimSpace(valStr)), prefix, indent); err == nil {
126+
if err := json.Indent(&b, []byte(valStr), prefix, indent); err == nil {
140127
return b.String()
141128
}
142129
} else if b, err := json.MarshalIndent(val, prefix, indent); err == nil {
@@ -154,17 +141,17 @@ func NewJSONTransformer(prefix string, indent string) Transformer {
154141
// location (use time.Local to get localized timestamps).
155142
func NewTimeTransformer(layout string, location *time.Location) Transformer {
156143
return func(val interface{}) string {
157-
rsp := fmt.Sprint(val)
144+
// Check for time.Time first to avoid unnecessary fmt.Sprint conversion
158145
if valTime, ok := val.(time.Time); ok {
159-
rsp = formatTime(valTime, layout, location)
160-
} else {
161-
// cycle through some supported layouts to see if the string form
162-
// of the object matches any of these layouts
163-
for _, possibleTimeLayout := range possibleTimeLayouts {
164-
if valTime, err := time.Parse(possibleTimeLayout, rsp); err == nil {
165-
rsp = formatTime(valTime, layout, location)
166-
break
167-
}
146+
return formatTime(valTime, layout, location)
147+
}
148+
// Only convert to string if not already time.Time
149+
rsp := fmt.Sprint(val)
150+
// Cycle through some supported layouts to see if the string form
151+
// of the object matches any of these layouts
152+
for _, possibleTimeLayout := range possibleTimeLayouts {
153+
if valTime, err := time.Parse(possibleTimeLayout, rsp); err == nil {
154+
return formatTime(valTime, layout, location)
168155
}
169156
}
170157
return rsp
@@ -217,12 +204,13 @@ func formatTime(t time.Time, layout string, location *time.Location) string {
217204
}
218205

219206
func formatTimeUnix(unixTime int64, timeTransformer Transformer) string {
207+
// Use pre-computed constants instead of repeated time.Second.Nanoseconds() calls
220208
if unixTime >= unixTimeMinNanoSeconds {
221-
unixTime = unixTime / time.Second.Nanoseconds()
209+
unixTime = unixTime / nanosPerSecond
222210
} else if unixTime >= unixTimeMinMicroseconds {
223-
unixTime = unixTime / (time.Second.Nanoseconds() / 1000)
211+
unixTime = unixTime / microsPerSecond
224212
} else if unixTime >= unixTimeMinMilliseconds {
225-
unixTime = unixTime / (time.Second.Nanoseconds() / 1000000)
213+
unixTime = unixTime / millisPerSecond
226214
}
227215
return timeTransformer(time.Unix(unixTime, 0))
228216
}

‎text/transformer_test.go‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,16 @@ func TestNewJSONTransformer(t *testing.T) {
144144
]
145145
}`
146146
assert.Equal(t, expectedOutput, transformer(input))
147+
148+
// Test error case: object that cannot be marshaled (channel) - triggers error path in json.MarshalIndent
149+
ch := make(chan int)
150+
result := transformer(ch)
151+
assert.Contains(t, result, "chan int")
152+
153+
// Test error case: function that cannot be marshaled - triggers error path in json.MarshalIndent
154+
fn := func() {}
155+
result = transformer(fn)
156+
assert.Contains(t, result, "func()")
147157
}
148158

149159
func TestNewTimeTransformer(t *testing.T) {
@@ -180,6 +190,15 @@ func TestNewTimeTransformer(t *testing.T) {
180190
for _, possibleTimeLayout := range possibleTimeLayouts {
181191
assert.Equal(t, expected, transformer(inTime.Format(possibleTimeLayout)), possibleTimeLayout)
182192
}
193+
194+
// Test case where string doesn't match any time layout (returns original string)
195+
transformer = NewTimeTransformer(time.RFC3339, nil)
196+
nonTimeString := "not a time string"
197+
assert.Equal(t, nonTimeString, transformer(nonTimeString))
198+
199+
// Test with nil location
200+
transformer = NewTimeTransformer(time.RFC3339, nil)
201+
assert.Equal(t, "2010-11-12T13:14:15-07:00", transformer(inTime))
183202
}
184203

185204
func TestNewUnixTimeTransformer(t *testing.T) {

0 commit comments

Comments
 (0)