Part III - Refactors

* chore: minor refactors and updates to comments, address review #686 (comment)

---------

Co-authored-by: Pijush Chakraborty <[email protected]>

Author: Namyalg

remoteconfig/condition_evaluator.go

@@ -148,11 +148,13 @@ func (ce *conditionEvaluator) evaluatePercentCondition(percentCondition *percent
 }
 
 func computeInstanceMicroPercentile(seed string, randomizationID string) uint32 {
-	seedPrefix := ""
+	var sb strings.Builder
 	if len(seed) > 0 {
-		seedPrefix = fmt.Sprintf("%s.", seed)
+		sb.WriteString(seed)
+		sb.WriteRune('.')
 	}
-	stringToHash := fmt.Sprintf("%s%s", seedPrefix, randomizationID)
+	sb.WriteString(randomizationID)
+	stringToHash := sb.String()
 
 	hash := sha256.New()
 	hash.Write([]byte(stringToHash))
@@ -192,7 +194,7 @@ func (ce *conditionEvaluator) evaluateCustomSignalCondition(customSignalConditio
 			return result
 		})
 
-	// For numeric operators only one target value is allowed
+	// For numeric operators only one target value is allowed.
 	case numericLessThan:
 		return compareNumbers(customSignalCondition.TargetCustomSignalValues[0], actualValue, func(result int) bool { return result < 0 })
 	case numericLessThanEqual:

remoteconfig/condition_evaluator_test.go

@@ -296,41 +296,41 @@ func TestPercentConditionMicroPercent(t *testing.T) {
 	}{
 		{
 			description:  "Evaluate LESS_OR_EQUAL to true when MicroPercent is max",
-			operator:     "LESS_OR_EQUAL",
+			operator:     lessThanOrEqual,
 			microPercent: 100_000_000,
 			outcome:      true,
 		},
 		{
 			description:  "Evaluate LESS_OR_EQUAL to false when MicroPercent is min",
-			operator:     "LESS_OR_EQUAL",
+			operator:     lessThanOrEqual,
 			microPercent: 0,
 			outcome:      false,
 		},
 		{
 			description: "Evaluate LESS_OR_EQUAL to false when MicroPercent is not set (MicroPercent should use zero)",
-			operator:    "LESS_OR_EQUAL",
+			operator:    lessThanOrEqual,
 			outcome:     false,
 		},
 		{
 			description: "Evaluate GREATER_THAN to true when MicroPercent is not set (MicroPercent should use zero)",
-			operator:    "GREATER_THAN",
+			operator:    greaterThan,
 			outcome:     true,
 		},
 		{
 			description:  "Evaluate GREATER_THAN max to false",
-			operator:     "GREATER_THAN",
+			operator:     greaterThan,
 			outcome:      false,
 			microPercent: 100_000_000,
 		},
 		{
 			description:  "Evaluate LESS_OR_EQUAL to 9571542 to true",
-			operator:     "LESS_OR_EQUAL",
+			operator:     lessThanOrEqual,
 			microPercent: 9_571_542, // instanceMicroPercentile of abcdef.123 (testSeed.testRandomizationID) is 9_571_542
 			outcome:      true,
 		},
 		{
 			description:  "Evaluate greater than 9571542 to true",
-			operator:     "GREATER_THAN",
+			operator:     greaterThan,
 			microPercent: 9_571_541, // instanceMicroPercentile of abcdef.123 (testSeed.testRandomizationID) is 9_571_542
 			outcome:      true,
 		},
@@ -361,40 +361,40 @@ func TestPercentConditionMicroPercentRange(t *testing.T) {
 	}{
 		{
 			description: "Evaluate to false when microPercentRange is not set",
-			operator:    "BETWEEN",
+			operator:    between,
 			outcome:     false,
 		},
 		{
 			description:    "Evaluate to false when upper bound is not set",
 			microPercentLb: 0,
-			operator:       "BETWEEN",
+			operator:       between,
 			outcome:        false,
 		},
 		{
 			description:    "Evaluate to true when lower bound is not set and upper bound is max",
 			microPercentUb: 100_000_000,
-			operator:       "BETWEEN",
+			operator:       between,
 			outcome:        true,
 		},
 		{
 			description:    "Evaluate to true when between lower and upper bound", // instanceMicroPercentile of abcdef.123 (testSeed.testRandomizationID) is 9_571_542
 			microPercentLb: 9_000_000,
 			microPercentUb: 9_571_542, // interval is (9_000_000, 9_571_542]
-			operator:       "BETWEEN",
+			operator:       between,
 			outcome:        true,
 		},
 		{
 			description:    "Evaluate to false when lower and upper bounds are equal",
 			microPercentLb: 98_000_000,
 			microPercentUb: 98_000_000,
-			operator:       "BETWEEN",
+			operator:       between,
 			outcome:        false,
 		},
 		{
 			description:    "Evaluate to false when not between 9_400_000 and 9_500_000", // instanceMicroPercentile of abcdef.123 (testSeed.testRandomizationID) is 9_571_542
 			microPercentLb: 9_400_000,
 			microPercentUb: 9_500_000,
-			operator:       "BETWEEN",
+			operator:       between,
 			outcome:        false,
 		},
 	}

remoteconfig/remoteconfig.go

@@ -12,7 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-// Package remoteconfig allows clients to use Firebase Remote Config with Go.
+// Package remoteconfig provides functions to fetch and evaluate a server-side Remote Config template.
 package remoteconfig
 
 import (
@@ -34,7 +34,7 @@ type Client struct {
 	*rcClient
 }
 
-// NewClient initializes a RemoteConfigClient with app-specific detail and a returns a
+// NewClient initializes a RemoteConfigClient with app-specific detail and returns a
 // client to be used by the user.
 func NewClient(ctx context.Context, c *internal.RemoteConfigClientConfig) (*Client, error) {
 	if c.ProjectID == "" {
@@ -67,6 +67,7 @@ func newRcClient(client *internal.HTTPClient, conf *internal.RemoteConfigClientC
 		internal.WithHeader("x-goog-api-client", internal.GetMetricsHeader(conf.Version)),
 	}
 
+	// Handles errors for non-success HTTP status codes from Remote Config servers.
 	client.CreateErrFn = handleRemoteConfigError
 
 	return &rcClient{
@@ -77,7 +78,7 @@ func newRcClient(client *internal.HTTPClient, conf *internal.RemoteConfigClientC
 	}
 }
 
-// GetServerTemplate Initializes a new ServerTemplate instance and fetches the server template.
+// GetServerTemplate initializes a new ServerTemplate instance and fetches the server template.
 func (c *rcClient) GetServerTemplate(ctx context.Context,
 	defaultConfig map[string]any) (*ServerTemplate, error) {
 	template, err := c.InitServerTemplate(defaultConfig, "")
@@ -97,10 +98,10 @@ func (c *rcClient) InitServerTemplate(defaultConfig map[string]any,
 	template, err := newServerTemplate(c, defaultConfig)
 
 	if templateDataJSON != "" && err == nil {
-		template.Set(templateDataJSON)
+		err = template.Set(templateDataJSON)
 	}
 
-	return template, nil
+	return template, err
 }
 
 func handleRemoteConfigError(resp *internal.Response) error {

remoteconfig/server_config.go

@@ -15,19 +15,20 @@
 package remoteconfig
 
 import (
+	"slices"
 	"strconv"
 	"strings"
 )
 
-// ValueSource represents the source of a value
+// ValueSource represents the source of a value.
 type ValueSource int
 
-// Constants for value source
+// Constants for value source.
 const (
 	sourceUnspecified ValueSource = iota
 	Static                        // Static represents a statically defined value.
-	Remote                        // Default represents a default value.
-	Default                       // Remote represents a value fetched from a remote source.
+	Remote                        // Remote represents a value fetched from a remote source.
+	Default                       // Default represents a default value.
 )
 
 // Value defines the interface for configuration values.
@@ -36,7 +37,7 @@ type value struct {
 	value  string
 }
 
-// Default Values for config parameters.
+// Default values for different parameter types.
 const (
 	DefaultValueForBoolean = false
 	DefaultValueForString  = ""
@@ -47,30 +48,41 @@ var booleanTruthyValues = []string{"1", "true", "t", "yes", "y", "on"}
 
 // ServerConfig is the implementation of the ServerConfig interface.
 type ServerConfig struct {
-	ConfigValues map[string]value
+	configValues map[string]value
 }
 
 // NewServerConfig creates a new ServerConfig instance.
-func NewServerConfig(configValues map[string]value) *ServerConfig {
-	return &ServerConfig{ConfigValues: configValues}
+func newServerConfig(configValues map[string]value) *ServerConfig {
+	return &ServerConfig{configValues: configValues}
 }
 
 // GetBoolean returns the boolean value associated with the given key.
+//
+// It returns true if the string value is "1", "true", "t", "yes", "y", or "on" (case-insensitive).
+// Otherwise, or if the key is not found, it returns the default boolean value (false).
 func (s *ServerConfig) GetBoolean(key string) bool {
 	return s.getValue(key).asBoolean()
 }
 
 // GetInt returns the integer value associated with the given key.
+//
+// If the parameter value cannot be parsed as an integer, or if the key is not found,
+// it returns the default numeric value (0).
 func (s *ServerConfig) GetInt(key string) int {
 	return s.getValue(key).asInt()
 }
 
 // GetFloat returns the float value associated with the given key.
+//
+// If the parameter value cannot be parsed as a float64, or if the key is not found,
+// it returns the default float value (0).
 func (s *ServerConfig) GetFloat(key string) float64 {
 	return s.getValue(key).asFloat()
 }
 
 // GetString returns the string value associated with the given key.
+//
+// If the key is not found, it returns the default string value ("").
 func (s *ServerConfig) GetString(key string) string {
 	return s.getValue(key).asString()
 }
@@ -82,10 +94,10 @@ func (s *ServerConfig) GetValueSource(key string) ValueSource {
 
 // getValue returns the value associated with the given key.
 func (s *ServerConfig) getValue(key string) *value {
-	if val, ok := s.ConfigValues[key]; ok {
+	if val, ok := s.configValues[key]; ok {
 		return &val
 	}
-	return newValue(Static, "")
+	return newValue(Static, DefaultValueForString)
 }
 
 // newValue creates a new value instance.
@@ -107,13 +119,7 @@ func (v *value) asBoolean() bool {
 		return DefaultValueForBoolean
 	}
 
-	for _, truthyValue := range booleanTruthyValues {
-		if strings.ToLower(v.value) == truthyValue {
-			return true
-		}
-	}
-
-	return false
+	return slices.Contains(booleanTruthyValues, strings.ToLower(v.value))
 }
 
 // asInt returns the value as an integer.
@@ -130,12 +136,12 @@ func (v *value) asInt() int {
 	return num
 }
 
-// asFloat returns the value as an integer.
+// asFloat returns the value as a float.
 func (v *value) asFloat() float64 {
 	if v.source == Static {
 		return DefaultValueForNumber
 	}
-	num, err := strconv.ParseFloat(v.value, 64)
+	num, err := strconv.ParseFloat(v.value, doublePrecision)
 
 	if err != nil {
 		return DefaultValueForNumber