Skip to content

Replace hardcoded magic numbers with named constants #570

Description

@Oluwaseyi89

Summary

Replace hardcoded magic numbers with named constants — Hardcoded values like a 45 ms dummy latency, Soroban poll attempt counts, and password hash cost are scattered without explanation or named constants.

Social Media Link

Let's collaborate on Discord. And ensure to star our repo.

Problem Statement

Confirmed in project-portal/project-portal-backend/internal/integration/service.go, project-portal/project-portal-backend/internal/financing/tokenization/stellar_client.go, and project-portal/project-portal-backend/internal/auth/service.go:

  1. Hardcoded dummy latency in integration health checks: TestConnection in integration/service.go sets LatencyMs: 45, // Dummy value with no named constant and no real measurement of the request round-trip.

  2. Hardcoded default poll attempts: newRealClientFromEnv in stellar_client.go sets pollAttempts := 15 inline before checking CARBON_ASSET_POLL_ATTEMPTS — the fallback value has no named constant.

  3. Hardcoded poll interval: RealStellarClient.pollInterval is set to a literal 2 * time.Second in newRealClientFromEnv with no named constant or environment override.

  4. Hardcoded transaction timeout: txnbuild.NewTimeout(300) appears twice in stellar_client.go (simulation and submit transactions) as a bare literal 300 with no named constant explaining the 300-second choice.

  5. Hardcoded default password hash cost: NewService in auth/service.go falls back to hashCost = 12 when hashCost == 0 is passed in, with no named constant documenting why 12 rounds was chosen.

  6. Hardcoded default methodology mock start token: NewContractClientFromEnv-equivalent logic in the methodology client defaults startToken := 1000 inline (mirrors the same pattern as stellar_client.go's literals).

  7. Hardcoded email/password-reset token expiry: generateAuthToken in auth/service.go defaults expiry to 24 * time.Hour when zero, and callers pass 24*time.Hour / 1*time.Hour literals directly rather than named constants (EmailVerificationTokenTTL, PasswordResetTokenTTL).

  8. Webhook health check default latency has no unit-documented constant: The 45 in LatencyMs: 45 is unitless at the call site beyond the field name — a named constant with a comment would make the placeholder nature explicit and easy to grep for.

  9. No single location for tunable defaults: These values (poll attempts, poll interval, timeout, hash cost, token TTLs) are scattered across three unrelated packages instead of being defined as named constants near their usage or in a shared internal constants package.

  10. Magic numbers make placeholder code harder to find: Because 45 (dummy latency) is a bare literal, a future contributor removing mock/placeholder behavior (tracked in other issues) cannot grep for a named constant like DummyLatencyMs to find every such placeholder.

  11. No tests pin these defaults: Because the values are inline literals, there are no unit tests asserting DefaultPollAttempts == 15 or DefaultPasswordHashCost == 12, so a future refactor could silently change behavior.

  12. Inconsistent style with adjacent code: stellar_client.go already uses named constants for DefaultCarbonAssetContractID and defaultSorobanRPCURL — the poll attempts/interval/timeout literals sit right next to these named constants without the same treatment.

Required Changes

  1. Add a named constant DefaultPollAttempts = 15 in stellar_client.go, replacing the inline literal.

  2. Add a named constant DefaultPollInterval = 2 * time.Second, replacing the inline literal.

  3. Add a named constant MintTransactionTimeoutSeconds = 300 (or time.Duration), replacing both txnbuild.NewTimeout(300) call sites.

  4. Add a named constant DefaultPasswordHashCost = 12 in auth/service.go, replacing the inline fallback.

  5. Add named constants EmailVerificationTokenTTL = 24 * time.Hour and PasswordResetTokenTTL = 1 * time.Hour, replacing literal call-site durations.

  6. Add a named constant DefaultMethodologyMockStartToken = 1000 in the methodology contract client, replacing the inline literal.

  7. Add a named constant PlaceholderHealthCheckLatencyMs = 45 with a comment explaining it is a temporary placeholder pending real latency measurement in TestConnection.

  8. Consolidate related defaults (poll/timeout/hash-cost/TTL constants) at the top of their respective files, grouped and commented, consistent with the existing DefaultCarbonAssetContractID style.

  9. Add unit tests asserting the named constants retain their expected default values.

  10. Grep the rest of internal/ and pkg/ for similarly undocumented magic numbers introduced by the same author/PR pattern and apply the same treatment where found.

Acceptance Criteria

  1. No bare numeric literal remains for poll attempts, poll interval, or transaction timeout in stellar_client.go.
  2. Password hash cost default is a named, documented constant.
  3. Email verification and password reset token TTLs are named constants.
  4. The dummy health-check latency is a named constant with a comment marking it as a placeholder.
  5. Methodology mock start token default is a named constant.
  6. All new constants are grouped consistently with existing constants in each file.
  7. Unit tests pin the value of each new constant.
  8. Behavior is unchanged — only naming/documentation improves, no functional regression.

Directory to Work on:

project-portal/project-portal-backend/

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    backendThis issue is about building backend API services.ginThis issue is to be implemented with the `golang` `gin` framework for backend APIs.golangThis issue is to be implemented with `golang` programming language.

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions