Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add helper function to transform string states into int states #126

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

martialblog
Copy link
Member

This PR adds a helper function to transform string states into int states.
It's the invert of the existing StatusText.

Unsure if this should should return an error if "invalid input" is given, the StatusInt doesn't so I decided to go for consistency.

While writing this, I thought: is there a reason we don't have a "Status" type with these methods attached? 🤔

See #125

@martialblog martialblog self-assigned this Dec 23, 2024
@martialblog
Copy link
Member Author

Do we gain anything from a custom Int type?
Implement interfaces like Stringer yes... but otherwise?

type Status int

// String returns the string corresponding to a state
func (s Status) String() string {
	switch s {
	case OK:
		return OKString
	case Warning:
		return WarningString
	case Critical:
		return CriticalString
	case Unknown:
	}

	return UnknownString
}

// NewStatusFromString returns a state corresponding to its
// common string representation
func NewStatusFromString(status string) Status {
...

@RincewindsHat
Copy link
Member

The thought of a Status type has occured to me and I do tend to like it, since the status is semantically not a number and it would improve the clarity of the code in here to handle status differently from numbers.

Therefore I am for introducing a specific status type

case UnknownString, "3":
return Unknown
default:
return Unknown
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do understand the wish for consistency, but IMHO there is a clear error case here and I feel uncomfortable with the function handling this the way it is now.

I think I would be rather (unpleasantly) surprised to find out, that this function transforms "Critiacl" (a typo of "Critical") to Unknown without giving me a way to verify it.

And yes, the function is rather trivial and I might be heavily prone to overengineering.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants