Remove redundant constructor, h/t sergio

debian/changelog

@@ -1,3 +1,12 @@
+govpp-snmp-agentx (1.1.7-1) bookworm; urgency=medium
+
+  * Refactor VPPClient constructor to use idiomatic Go patterns
+  * Remove redundant NewVPPClient() constructor function
+  * Update all code to use direct struct initialization (&VPPClient{})
+  * Improve code maintainability and follow Go best practices
+
+ -- Pim van Pelt <pim@ipng.ch>  Fri, 15 Nov 2024 00:00:00 +0000
+
 govpp-snmp-agentx (1.1.6-1) bookworm; urgency=medium
 
   * Replace forked go-agentx dependency with upstream github.com/posteo/go-agentx

src/agentx/agentx.go

@@ -45,4 +45,4 @@ func StartAgentXRoutine(interfaceMIB *ifmib.InterfaceMIB) error {
 
 	logger.Printf("Successfully registered with AgentX at %s://%s", network, address)
 	return nil
-}
+}

src/agentx/agentx_test.go

@@ -22,7 +22,7 @@ func TestAgentXAddrFlagParsing(t *testing.T) {
 	// Test Unix socket path
 	testAddr := "/var/run/test.sock"
 	*AgentXAddr = testAddr
-	
+
 	if *AgentXAddr != testAddr {
 		t.Errorf("Expected AgentX address to be '%s', got '%s'", testAddr, *AgentXAddr)
 	}
@@ -30,7 +30,7 @@ func TestAgentXAddrFlagParsing(t *testing.T) {
 	// Test TCP address
 	testAddr = "192.168.1.1:705"
 	*AgentXAddr = testAddr
-	
+
 	if *AgentXAddr != testAddr {
 		t.Errorf("Expected AgentX address to be '%s', got '%s'", testAddr, *AgentXAddr)
 	}
@@ -43,12 +43,12 @@ func TestFlagRegistration(t *testing.T) {
 		t.Error("Expected agentx.addr flag to be registered")
 		return
 	}
-	
+
 	if f.DefValue != "localhost:705" {
 		t.Errorf("Expected flag default value to be 'localhost:705', got '%s'", f.DefValue)
 	}
-	
+
 	if f.Usage != "Address to connect to (hostname:port or Unix socket path)" {
 		t.Errorf("Unexpected flag usage string: %s", f.Usage)
 	}
-}
+}

src/config/config.go

@@ -5,4 +5,4 @@ package config
 // Global configuration variables
 var (
 	Debug bool
-)
+)

src/config/config_test.go

@@ -27,4 +27,4 @@ func TestDebugFlagSet(t *testing.T) {
 	if Debug != false {
 		t.Errorf("Expected Debug to be false after setting, got %v", Debug)
 	}
-}
+}

src/logger/logger.go

@@ -6,7 +6,7 @@ import (
 	"fmt"
 	"path/filepath"
 	"runtime"
-	
+
 	"govpp-snmp-agentx/config"
 )
 
@@ -16,15 +16,15 @@ func getCallerInfo() string {
 	if !ok {
 		return "unknown:unknown"
 	}
-	
+
 	fn := runtime.FuncForPC(pc)
 	if fn == nil {
 		return "unknown:unknown"
 	}
-	
+
 	funcName := filepath.Base(fn.Name())
 	fileName := filepath.Base(file)
-	
+
 	return fmt.Sprintf("%s:%s", fileName, funcName)
 }
 
@@ -49,4 +49,4 @@ func Debugf(format string, args ...interface{}) {
 // Sync flushes any buffered log entries (no-op for fmt.Println)
 func Sync() {
 	// No buffering with fmt.Println, so this is a no-op
-}
+}

src/logger/logger_test.go

@@ -33,11 +33,11 @@ func TestPrintf(t *testing.T) {
 	if !strings.HasPrefix(output, "INFO ") {
 		t.Errorf("Expected output to start with 'INFO ', got: %s", output)
 	}
-	
+
 	if !strings.Contains(output, "logger_test.go:logger.TestPrintf") {
 		t.Errorf("Expected output to contain caller info, got: %s", output)
 	}
-	
+
 	if !strings.Contains(output, "test message: hello") {
 		t.Errorf("Expected output to contain message, got: %s", output)
 	}
@@ -71,7 +71,7 @@ func TestDebugfWithDebugEnabled(t *testing.T) {
 	if !strings.HasPrefix(output, "DEBUG ") {
 		t.Errorf("Expected output to start with 'DEBUG ', got: %s", output)
 	}
-	
+
 	if !strings.Contains(output, "debug message: test") {
 		t.Errorf("Expected output to contain message, got: %s", output)
 	}
@@ -110,4 +110,4 @@ func TestDebugfWithDebugDisabled(t *testing.T) {
 func TestSync(t *testing.T) {
 	// Test that Sync doesn't panic (it's a no-op now)
 	Sync()
-}
+}

src/main.go

@@ -16,7 +16,7 @@ import (
 	"govpp-snmp-agentx/vpp"
 )
 
-const Version = "1.1.6-1"
+const Version = "1.1.7-1"
 
 func main() {
 	debug := flag.Bool("debug", false, "Enable debug logging")
@@ -46,7 +46,7 @@ func main() {
 	}
 
 	// Create VPP client and managers
-	vppClient := vpp.NewVPPClient()
+	vppClient := &vpp.VPPClient{}
 	interfaceManager := vpp.NewInterfaceManager(vppClient)
 	statsManager := vpp.NewStatsManager(vppClient)
 

src/vpp/vpp.go

@@ -29,11 +29,6 @@ type VPPClient struct {
 	connected bool
 }
 
-// NewVPPClient creates a new VPP client instance
-func NewVPPClient() *VPPClient {
-	return &VPPClient{}
-}
-
 // Connect establishes connections to both VPP API and Stats sockets
 func (c *VPPClient) Connect() error {
 	logger.Debugf("Connecting to VPP (API: %s, Stats: %s)", *ApiAddr, *StatsAddr)

src/vpp/vpp_iface_test.go

@@ -10,7 +10,7 @@ import (
 )
 
 func TestNewInterfaceManager(t *testing.T) {
-	client := NewVPPClient()
+	client := &VPPClient{}
 	manager := NewInterfaceManager(client)
 
 	if manager == nil {
@@ -36,7 +36,7 @@ func TestNewInterfaceManager(t *testing.T) {
 }
 
 func TestInterfaceManagerSetEventCallback(t *testing.T) {
-	client := NewVPPClient()
+	client := &VPPClient{}
 	manager := NewInterfaceManager(client)
 
 	var callbackCalled bool
@@ -82,7 +82,7 @@ func TestInterfaceManagerSetEventCallback(t *testing.T) {
 }
 
 func TestInterfaceManagerGetAllInterfaceDetailsWithoutConnection(t *testing.T) {
-	client := NewVPPClient()
+	client := &VPPClient{}
 	manager := NewInterfaceManager(client)
 
 	_, err := manager.GetAllInterfaceDetails()
@@ -101,7 +101,7 @@ func TestInterfaceManagerGetAllInterfaceDetailsWithoutConnection(t *testing.T) {
 }
 
 func TestInterfaceManagerStartEventWatcherWithoutConnection(t *testing.T) {
-	client := NewVPPClient()
+	client := &VPPClient{}
 	manager := NewInterfaceManager(client)
 
 	err := manager.StartEventWatcher()
@@ -120,7 +120,7 @@ func TestInterfaceManagerStartEventWatcherWithoutConnection(t *testing.T) {
 }
 
 func TestInterfaceManagerHandleInterfaceEventWithoutCallback(t *testing.T) {
-	client := NewVPPClient()
+	client := &VPPClient{}
 	manager := NewInterfaceManager(client)
 
 	// Should not panic when callback is nil
@@ -128,7 +128,7 @@ func TestInterfaceManagerHandleInterfaceEventWithoutCallback(t *testing.T) {
 }
 
 func TestInterfaceManagerInitializeEventWatchingWithoutConnection(t *testing.T) {
-	client := NewVPPClient()
+	client := &VPPClient{}
 	manager := NewInterfaceManager(client)
 
 	err := manager.InitializeEventWatching()
@@ -220,7 +220,7 @@ func TestInterfaceEventCallback(t *testing.T) {
 }
 
 func TestInterfaceManagerStartStopEventMonitoring(t *testing.T) {
-	client := NewVPPClient()
+	client := &VPPClient{}
 	manager := NewInterfaceManager(client)
 
 	if manager.running {
@@ -248,7 +248,7 @@ func TestInterfaceManagerStartStopEventMonitoring(t *testing.T) {
 }
 
 func TestInterfaceManagerEventMonitoringWithConnectionChanges(t *testing.T) {
-	client := NewVPPClient()
+	client := &VPPClient{}
 	manager := NewInterfaceManager(client)
 
 	// Set a callback to track calls

src/vpp/vpp_stats_test.go

@@ -10,7 +10,7 @@ import (
 )
 
 func TestNewStatsManager(t *testing.T) {
-	client := NewVPPClient()
+	client := &VPPClient{}
 	manager := NewStatsManager(client)
 
 	if manager == nil {
@@ -36,7 +36,7 @@ func TestNewStatsManager(t *testing.T) {
 }
 
 func TestStatsManagerSetStatsCallback(t *testing.T) {
-	client := NewVPPClient()
+	client := &VPPClient{}
 	manager := NewStatsManager(client)
 
 	var callbackCalled bool
@@ -85,7 +85,7 @@ func TestStatsManagerSetStatsCallback(t *testing.T) {
 }
 
 func TestStatsManagerSetPeriod(t *testing.T) {
-	client := NewVPPClient()
+	client := &VPPClient{}
 	manager := NewStatsManager(client)
 
 	newPeriod := 5 * time.Second
@@ -97,7 +97,7 @@ func TestStatsManagerSetPeriod(t *testing.T) {
 }
 
 func TestStatsManagerStartStopStatsRoutine(t *testing.T) {
-	client := NewVPPClient()
+	client := &VPPClient{}
 	manager := NewStatsManager(client)
 
 	if manager.running {
@@ -125,7 +125,7 @@ func TestStatsManagerStartStopStatsRoutine(t *testing.T) {
 }
 
 func TestStatsManagerGetInterfaceStatsWithoutConnection(t *testing.T) {
-	client := NewVPPClient()
+	client := &VPPClient{}
 	manager := NewStatsManager(client)
 
 	_, err := manager.GetInterfaceStats()
@@ -206,7 +206,7 @@ func TestStatsCallback(t *testing.T) {
 }
 
 func TestStatsManagerQueryAndReportStatsWithoutConnection(t *testing.T) {
-	client := NewVPPClient()
+	client := &VPPClient{}
 	manager := NewStatsManager(client)
 
 	// Should return false when not connected
@@ -216,7 +216,7 @@ func TestStatsManagerQueryAndReportStatsWithoutConnection(t *testing.T) {
 }
 
 func TestStatsManagerWithShortPeriod(t *testing.T) {
-	client := NewVPPClient()
+	client := &VPPClient{}
 	manager := NewStatsManager(client)
 
 	// Set a very short period for testing

src/vpp/vpp_test.go

@@ -7,7 +7,7 @@ import (
 )
 
 func TestNewVPPClient(t *testing.T) {
-	client := NewVPPClient()
+	client := &VPPClient{}
 
 	if client == nil {
 		t.Fatal("NewVPPClient() returned nil")
@@ -27,7 +27,7 @@ func TestNewVPPClient(t *testing.T) {
 }
 
 func TestVPPClientDisconnect(t *testing.T) {
-	client := NewVPPClient()
+	client := &VPPClient{}
 
 	// Should be safe to call disconnect on unconnected client
 	client.Disconnect()
@@ -38,7 +38,7 @@ func TestVPPClientDisconnect(t *testing.T) {
 }
 
 func TestVPPClientNewAPIChannelWithoutConnection(t *testing.T) {
-	client := NewVPPClient()
+	client := &VPPClient{}
 
 	_, err := client.NewAPIChannel()
 	if err == nil {
@@ -56,7 +56,7 @@ func TestVPPClientNewAPIChannelWithoutConnection(t *testing.T) {
 }
 
 func TestVPPClientCheckLivenessWithoutConnection(t *testing.T) {
-	client := NewVPPClient()
+	client := &VPPClient{}
 
 	if client.CheckLiveness() {
 		t.Error("CheckLiveness() should return false when not connected")
@@ -86,7 +86,7 @@ func TestVPPClientConnectWithInvalidPaths(t *testing.T) {
 		*StatsAddr = origStatsAddr
 	}()
 
-	client := NewVPPClient()
+	client := &VPPClient{}
 	err := client.Connect()
 
 	if err == nil {