From 67a585262a3aa4c8e768cf0b6a8759199c20490d Mon Sep 17 00:00:00 2001
From: haim-kermany <haim.kermany@gmail.com>
Date: Wed, 10 Jan 2024 10:26:53 +0200
Subject: [PATCH] code review

---
 cmd/analyzer/parse_args.go | 24 +++++++++++-------------
 pkg/vpcmodel/output.go     |  6 +++---
 2 files changed, 14 insertions(+), 16 deletions(-)

diff --git a/cmd/analyzer/parse_args.go b/cmd/analyzer/parse_args.go
index 548512580..c3883ec64 100644
--- a/cmd/analyzer/parse_args.go
+++ b/cmd/analyzer/parse_args.go
@@ -3,6 +3,7 @@ package main
 import (
 	"flag"
 	"fmt"
+	"slices"
 	"strings"
 )
 
@@ -75,7 +76,7 @@ var supportedOutputFormatsMap = map[string]bool{
 // supportedAnalysisTypesMap is a map from analysis type to its list of supported output formats
 var supportedAnalysisTypesMap = map[string][]string{
 	allEndpoints:     {TEXTFormat, MDFormat, JSONFormat, DRAWIOFormat, ARCHDRAWIOFormat, DEBUGFormat},
-	allSubnets:       {TEXTFormat, JSONFormat},
+	allSubnets:       {TEXTFormat, JSONFormat, DRAWIOFormat, ARCHDRAWIOFormat},
 	singleSubnet:     {TEXTFormat},
 	allEndpointsDiff: {TEXTFormat, MDFormat},
 	allSubnetsDiff:   {TEXTFormat, MDFormat},
@@ -189,11 +190,18 @@ func errorInErgs(args *InArgs, flagset *flag.FlagSet) error {
 	}
 	if _, ok := supportedAnalysisTypesMap[*args.AnalysisType]; !ok {
 		flagset.PrintDefaults()
-		return fmt.Errorf("wrong analysis type %s; must be one of: %s", *args.AnalysisType, strings.Join(supportedAnalysisTypesList, separator))
+		return fmt.Errorf("wrong analysis type '%s'; must be one of: '%s'",
+			*args.AnalysisType, strings.Join(supportedAnalysisTypesList, separator))
 	}
 	if !supportedOutputFormatsMap[*args.OutputFormat] {
 		flagset.PrintDefaults()
-		return fmt.Errorf("wrong output format %s; must be one of: %s", *args.OutputFormat, strings.Join(supportedOutputFormatsList, separator))
+		return fmt.Errorf("wrong output format '%s'; must be one of: '%s'",
+			*args.OutputFormat, strings.Join(supportedOutputFormatsList, separator))
+	}
+	if !slices.Contains(supportedAnalysisTypesMap[*args.AnalysisType], *args.OutputFormat) {
+		flagset.PrintDefaults()
+		return fmt.Errorf("wrong output format '%s' for analysis type '%s'; must be one of: %s",
+			*args.OutputFormat, *args.AnalysisType, strings.Join(supportedAnalysisTypesMap[*args.AnalysisType], separator))
 	}
 	if *args.OutputFormat == DEBUGFormat && *args.AnalysisType != allEndpoints {
 		return fmt.Errorf("output format %s supported on for %s", DEBUGFormat, allEndpoints)
@@ -213,16 +221,6 @@ func errorInErgs(args *InArgs, flagset *flag.FlagSet) error {
 //gocyclo:ignore
 func notSupportedYetArgs(args *InArgs) error {
 	diffAnalysis := *args.AnalysisType == allEndpointsDiff || *args.AnalysisType == allSubnetsDiff
-	if *args.OutputFormat == DRAWIOFormat || *args.OutputFormat == ARCHDRAWIOFormat {
-		if *args.AnalysisType != allEndpoints && *args.AnalysisType != allSubnets {
-			return fmt.Errorf("drawio output format is not supported with %s analysis type", *args.AnalysisType)
-		}
-		return nil
-	}
-	if !diffAnalysis && *args.AnalysisType != allEndpoints && *args.OutputFormat != TEXTFormat &&
-		*args.OutputFormat != JSONFormat {
-		return fmt.Errorf("currently only txt/json output format supported with %s analysis type", *args.AnalysisType)
-	}
 	if diffAnalysis && *args.OutputFormat != TEXTFormat && *args.OutputFormat != MDFormat {
 		return fmt.Errorf("currently only txt/md output format supported with %s analysis type", *args.AnalysisType)
 	}
diff --git a/pkg/vpcmodel/output.go b/pkg/vpcmodel/output.go
index 260c08643..b0037a66a 100644
--- a/pkg/vpcmodel/output.go
+++ b/pkg/vpcmodel/output.go
@@ -108,7 +108,7 @@ type SingleAnalysisOutput struct {
 	format     OutFormat
 }
 
-// Generate returns SingleAnalysisOutput for its VPC analysis results
+// Generate returns a string representing the analysis output for all input VPCs
 func (o *OutputGenerator) Generate(f OutFormat, outFile string) (string, error) {
 	var formatter OutputFormatter
 	switch f {
@@ -143,8 +143,8 @@ type OutputFormatter interface {
 // serialOutputFormatter is the formatter for json, md and txt formats.
 // serialOutputFormatter implements the interface OutputFormatter.
 // the main flow of WriteOutput() of serialOutputFormatter is:
-// 1. for each vpc, create and use a SingleVpcOutputFormatter to create a VPCsubnetConnectivity,
-// 2. aggregate the VPCsubnetConnectivity to one output
+// 1. for each vpc, create and use a SingleVpcOutputFormatter to create a SingleAnalysisOutput ,
+// 2. aggregate the SingleAnalysisOutputs to one output
 type serialOutputFormatter struct {
 	outFormat OutFormat
 }