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

Breaking change: Remove getter methods on constraints and expose fields instead #154

Closed
m1kola opened this issue Nov 2, 2023 · 0 comments · Fixed by #166
Closed

Breaking change: Remove getter methods on constraints and expose fields instead #154

m1kola opened this issue Nov 2, 2023 · 0 comments · Fixed by #166
Assignees
Labels
kind/design Categorizes issue or PR as related to design.
Milestone

Comments

@m1kola
Copy link
Member

m1kola commented Nov 2, 2023

Today constraints hold important data in unexported fields and provide getters to access data.

It seems like there is not much value in these getters: Deppy itself doesn't seem to use them and on the user side they are normally not required in production code.

However this makes testing of components which generate variables with constraints harder for deppy users. Users have to use something like go-cmp with AllowUnexported & use constructors such as constraint.AtMost() to create constraints instead of simply just comparing literals.

I think we should explore exporting constraint fields and removing getter methods.

For example, AtMostConstraint which looks today like this:

type AtMostConstraint struct {
ids []deppy.Identifier
n int
}
func (constraint *AtMostConstraint) String(subject deppy.Identifier) string {
s := make([]string, len(constraint.ids))
for i, each := range constraint.ids {
s[i] = string(each)
}
return fmt.Sprintf("%s permits at most %d of %s", subject, constraint.n, strings.Join(s, ", "))
}
func (constraint *AtMostConstraint) N() int {
return constraint.n
}
func (constraint *AtMostConstraint) Ids() []deppy.Identifier {
return constraint.ids
}

will be turned into something like this:

type AtMostConstraint struct { 
 	IDs []deppy.Identifier 
 	N   int 
 } 
  
 func (constraint *AtMostConstraint) String(subject deppy.Identifier) string { 
 	s := make([]string, len(constraint.IDs)) 
 	for i, each := range constraint.IDs { 
 		s[i] = string(each) 
 	} 
 	return fmt.Sprintf("%s permits at most %d of %s", subject, constraint.N, strings.Join(s, ", ")) 
 } 
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/design Categorizes issue or PR as related to design.
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

1 participant