Extract `func (c *CommandArgumentValidator) validate(args []strin…

[tangrufus]

• simplify argument count validation logic
• normalize argument count error message

---

Example usage:

# $ trellis deploy [options] ENVIRONMENT [SITE]

args = c.flags.Args()

argCountErr := validateArgumentCount(args, 1, 1)
if argCountErr != nil {
	c.UI.Error(argCountErr.Error())
	c.UI.Output(c.Help())
	return 1
}

environment := args[0]

siteName := ""
if len(args) == 2 {
	siteName = args[1]
}

---

Note: validateArgumentCount doesn't support unlimited arguments, i.e:

trellis-cli/cmd/exec.go

Lines 28 to 36 in c043cfd

	switch len(args) {
	case 0:
	c.UI.Error("Error: missing COMMAND argument\n")
	c.UI.Output(c.Help())
	return 1
	default:
	command = args[0]
	cmdArgs = args
	}

[swalkinshaw]

What if we made this a struct with methods too? It could look something like:

type CommandArgumentValidator struct {
  args []string{}
  required Int
  optional Int
  // or maybe initialize with UI too
}

func (c *CommandArgumentValidator) validate(ui cli.Ui) int {
  // etc
  ui.Error(argCountErr.Error())
  ui.Output(c.Help())
  return 1
}

// in a command

validator := CommandArgumentValidator{args: args, required: 0, optional: 0}
validator.validate(c.UI)

One reason I like this is for the named parameters

[tangrufus]

Updated.

Usage:

commandArgumentValidator := &CommandArgumentValidator{required: 1, optional: 1}
commandArgumentErr := commandArgumentValidator.validate(args)
if commandArgumentErr != nil {
	c.UI.Error(commandArgumentErr.Error())
	c.UI.Output(c.Help())
	return 1
}

---

To keep CommandArgumentValidator simple., I am not passing c.UI & c.Help() to CommandArgumentValidator.

Polluting CommandArgumentValidator with c.UI & c.Help() to make 2 less lines of code doesn't worth and looks like a violation of single responsibility principle to me. :

commandArgumentValidator := &CommandArgumentValidator{required: 1, optional: 1, UI: c.UI, Help: c.Help()}
commandArgumentErr := commandArgumentValidator.validate(args)
if commandArgumentErr != nil {
	return 1
}

What do you think?

[swalkinshaw]

I think I agree 👍

[swalkinshaw]

Maybe just declare totalArgs := c.required+c.optional up front?

[tangrufus]

done

[swalkinshaw]

You can re-use argCount in both places here?

[tangrufus]

done

[swalkinshaw]

🤔 does it complain if you explicitly do return err? Makes more sense to me if not (even though this works)

[tangrufus]

• added explicitly return in case proposal: Go 2: remove bare return golang/go#21291 happens
• return inside the if block to short-circuit it

[swalkinshaw]

Sorry I meant you can use argCount instead of len(args) in both these error messages?

[tangrufus]

i need coffee