None of the error returned by reporters are checked

[ccoVeille]

In pkg/cli/cli.go there is a section where none of the reporter.Print* method that could return errors are checked

config-file-validator/pkg/cli/cli.go

Lines 116 to 141 in 54b6781

	if _, ok := c.Reporter.(reporter.JsonReporter); ok {
	reporter.PrintSingleGroupJson(reportGroup)
	} else if !Quiet {
	reporter.PrintSingleGroupStdout(reportGroup)
	}
	} else if len(GroupOutput) == 2 {
	reportGroup, err := GroupByDouble(reports, GroupOutput)
	if err != nil {
	return 1, fmt.Errorf("unable to group by double value: %v", err)
	}
	if _, ok := c.Reporter.(reporter.JsonReporter); ok {
	reporter.PrintDoubleGroupJson(reportGroup)
	} else if !Quiet {
	reporter.PrintDoubleGroupStdout(reportGroup)
	}
	} else if len(GroupOutput) == 3 {
	reportGroup, err := GroupByTriple(reports, GroupOutput)
	if err != nil {
	return 1, fmt.Errorf("unable to group by triple value: %v", err)
	}
	if _, ok := c.Reporter.(reporter.JsonReporter); ok {
	reporter.PrintTripleGroupJson(reportGroup)
	} else if !Quiet {
	reporter.PrintTripleGroupStdout(reportGroup)
	}

This code requires a bit of refactoring, I think.

The addition of quiet mode (great addition BTW) with #124 made it even more complex to read and maintain.

[HakanVardarr]

I added a new pull request. I did some refactoring on cli.go can you look it?

[ccoVeille]

I added a new pull request. I did some refactoring on cli.go can you look it?

Can you reference the link? Because I looked for it and didn't see it.

[HakanVardarr]

I added a new pull request. I did some refactoring on cli.go can you look it?

Can you reference the link? Because I looked for it and didn't see it.

#131 (comment)

[ccoVeille]

I like the refactoring you initiated, but it doesn't address the issue I raised here

Please look at this

config-file-validator/pkg/cli/cli.go

Lines 137 to 141 in e9d7f48

	if _, ok := c.Reporter.(reporter.JsonReporter); ok {
	reporter.PrintSingleGroupJson(reportGroup.(map[string][]reporter.Report))
	} else {
	reporter.PrintSingleGroupStdout(reportGroup.(map[string][]reporter.Report))
	}

Either reporter.PrintSingleGroupJson and reporter.PrintSingleGroupStdout may return an error.

and the error are not tested.

Your code is now simpler, so it's good, but you should consider something like this

       if len(GroupOutput) == 1 && GroupOutput[0] != "" {
		// Check reporter type to determine how to print
		if _, ok := c.Reporter.(reporter.JsonReporter); ok {
			err = reporter.PrintSingleGroupJson(reportGroup.(map[string][]reporter.Report))
		} else {
			err = reporter.PrintSingleGroupStdout(reportGroup.(map[string][]reporter.Report))
		}
	} else if len(GroupOutput) == 2 {
		if _, ok := c.Reporter.(reporter.JsonReporter); ok {
			err = reporter.PrintDoubleGroupJson(reportGroup.(map[string]map[string][]reporter.Report))
		} else {
			err = reporter.PrintDoubleGroupStdout(reportGroup.(map[string]map[string][]reporter.Report))
		}
	} else if len(GroupOutput) == 3 {
		if _, ok := c.Reporter.(reporter.JsonReporter); ok {
			err = reporter.PrintTripleGroupJson(reportGroup.(map[string]map[string]map[string][]reporter.Report))
		} else {
			err = reporter.PrintTripleGroupStdout(reportGroup.(map[string]map[string]map[string][]reporter.Report))
		}
	} else {
		err =  c.Reporter.Print(reports)
	}

	if err != nil {
		return err           
        }

	return nil

[HakanVardarr]

Yeah I will return the error. Thanks for the suggestion.

[HakanVardarr]

Instead of

err = reporter.PrintSingleGroupJson(reportGroup.(map[string][]reporter.Report))

Can't we use

return reporter.PrintSingleGroupJson(reportGroup.(map[string][]reporter.Report))

[ccoVeille]

You refactored it the right way.

I mean the current version is already applying the pattern you talked about.

So you solved the issue with #131 now, which is great.

I will comment the other PR then, because I have a code suggestion