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

Support for Kind Interface and Pointer to Facts #345

Closed
rifqifatih opened this issue Dec 27, 2022 · 1 comment · Fixed by #349
Closed

Support for Kind Interface and Pointer to Facts #345

rifqifatih opened this issue Dec 27, 2022 · 1 comment · Fixed by #349

Comments

@rifqifatih
Copy link
Contributor

rifqifatih commented Dec 27, 2022

I notice there is a similar Issue which was closed. However, I think there is some use case which can still be consider supporting interface / pointer.

I'm currently providing the Facts through REST API which can be in any arbitrary form, hence map[string]interface{}. But rule evaluation gives me

can not use data type of interface in EQ comparison

or

can not use data type of ptr in EQ comparison

if I use pointer.

Solution
I propose that in reflectmath.go/Evaluate* handles these "reference" type with e.g.:

func EvaluateEqual(left, right reflect.Value) (reflect.Value, error) {
	if left.Kind() == reflect.Pointer || left.Kind() == reflect.Interface {
		return EvaluateEqual(left.Elem(), right)
	}

	if right.Kind() == reflect.Pointer || right.Kind() == reflect.Interface {
		return EvaluateEqual(left, right.Elem())
	}
....
}

Would gladly make PR contribution but I would like to open this for discussion first to know what are the implications and if it's aligned with the design.

@newm4n
Copy link
Member

newm4n commented Jan 13, 2023

Hi @rifqifatih

I can not figure any implication at this point. Please submit your PR including a unit test that shows whether your fix really works.
I will be very grateful if you can make a PR and make sure make test complete successfully.

rifqifatih added a commit to rifqifatih/grule-rule-engine that referenced this issue Jan 14, 2023
rifqifatih added a commit to rifqifatih/grule-rule-engine that referenced this issue Jan 14, 2023
rifqifatih added a commit to rifqifatih/grule-rule-engine that referenced this issue Jan 14, 2023
newm4n pushed a commit that referenced this issue Jan 26, 2023
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 a pull request may close this issue.

2 participants