-
-
Notifications
You must be signed in to change notification settings - Fork 0
Closed
Labels
Description
Problem
The string literal "public" is used as the default schema in multiple locations throughout main.go, creating a magic string constant that's duplicated and harder to maintain.
Locations:
- Line 146:
schema := "public" - Line 375:
schema := "public"(approximately) - Multiple tool handlers
Issues
- Duplication: Same literal repeated in multiple places
- Maintenance: If default changes, must update all locations
- Unclear intent: Not obvious why "public" is special
- Error-prone: Easy to typo or use inconsistent casing
Proposed Solution
Option 1: Define Constant in main.go
const (
// DefaultSchema is the default PostgreSQL schema when none is specified
DefaultSchema = "public"
)
// Usage in tool handlers
schema := args["schema"].(string)
if schema == "" {
schema = DefaultSchema
}Option 2: Define in App Package (Better)
Since this is PostgreSQL-specific knowledge, define it in the app package:
// internal/app/constants.go
package app
const (
// DefaultSchema is the default PostgreSQL schema used when none is specified.
// PostgreSQL creates a "public" schema by default in every database.
DefaultSchema = "public"
)Then use in main.go:
import "github.com/sgaunet/postgresql-mcp/internal/app"
// In tool handler
schema := args["schema"].(string)
if schema == "" {
schema = app.DefaultSchema
}Option 3: Add to App Config
If schema defaults should be configurable:
type AppConfig struct {
DefaultSchema string
}
func NewWithConfig(client PostgreSQLClient, config AppConfig) *App {
if config.DefaultSchema == "" {
config.DefaultSchema = "public"
}
return &App{
client: client,
defaultSchema: config.DefaultSchema,
}
}Recommended Approach
Use Option 2 (constant in app package) because:
- Keeps PostgreSQL-specific knowledge in the app layer
- Makes the constant reusable across packages
- Documents why "public" is the default
- Simple and doesn't require config changes
Other Magic Strings to Review
Search for other repeated string literals:
# Find repeated string literals
grep -r '"public"' --include="*.go"
grep -r '"postgres"' --include="*.go"
grep -r '"text"' --include="*.go"Impact
- Severity: TRIVIAL
- Type: Code Quality, Refactoring
- Location:
main.go(multiple locations) - Benefits: Better maintainability, clearer intent
Checklist
- Create constant for default schema name
- Replace all hardcoded "public" strings with constant
- Add documentation explaining why "public" is the default
- Search for other magic strings that should be constants
- Update tests if needed
- Consider making schema default configurable in the future