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

GET_NAME_FIELD_BY_TAG: Added get name of field from it's json tag, ad… #16

Merged
merged 8 commits into from
Sep 5, 2022

Conversation

Mansouri147
Copy link

…ded converting a map to a struct

@oleiade
Copy link
Owner

oleiade commented Aug 31, 2022

Hi @Mansouri147

Thanks a lot for the contribution, much appreciated 🙇🏻
I'm a bit "under the water" at the moment, but I'll give it a review in the upcoming days 🌦️ Thanks for your patience!

@Mansouri147
Copy link
Author

Mansouri147 commented Aug 31, 2022 via email

Copy link
Owner

@oleiade oleiade left a comment

Choose a reason for hiding this comment

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

LGTM 👍🏻

I'd be very curious as to what's the use case for this feature though 🤔 Would you mind sharing in what scenario you actually needed to do that?

I've left some change requests, but I believe it should be somewhat small changes. Namely I'd like the main function prototype to change, its documentation to be somewhat more thorough, and I prefer to not merge the map to struct function as is.

Once again, thanks a lot for your contribution 🙇🏻

reflections.go Outdated
@@ -91,6 +91,26 @@ func GetFieldTag(obj interface{}, fieldName, tagKey string) (string, error) {
return field.Tag.Get(tagKey), nil
}

// Function to get the the Field from it's json tag
func GetNameFieldByTag(obj interface{}, tag string, key string) (string, error) {
Copy link
Owner

Choose a reason for hiding this comment

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

I would prefer this function to be called GetFieldNameByTagValue instead. Moreover, I would find it more intuitive if the two last arguments were inverted and renamed for clarity:

func GetFieldNameByTagValue(obj interface{}, tagKey, tagValue string) (string, error)

@oleiade oleiade added this to the 1.0.2 milestone Sep 4, 2022
@oleiade
Copy link
Owner

oleiade commented Sep 4, 2022

I also realized that it would be nice to a usage example to the example_test.go file 🙇🏻

Ala-Mansouri added 2 commits September 4, 2022 19:49
…ToStruct and it's test, updated function name, added more test cases in reflections_test.go, added short example in Readme file
@Mansouri147
Copy link
Author

Done 💯 💪 👍

@Mansouri147
Copy link
Author

Mansouri147 commented Sep 4, 2022

quite basic usage example:

a service written in go for a smart-restaurant that has a conjob that checks for any change in dishes to update it based on some specific conditions,

the conditions can be changed from any other service ( using NodeJS, Python… )

// GO type of order
type Order struct {
		Step string `json:"order_step"`
		Id string `json:"id"`
		Category string `json:"category"`
}

// JSON data served to us from external DB or service
order ={
		"order_step": "cooking", // we get the name of this field which is in go will be "Step"
		"id": "45457-fv54f54",
		"category": "Pizzas"
}
condition = {
		"field": "order_step", // "step" here is the value not the key
		"value": "cooking",
		"next": "serve"
}

// returns filedName = "Step" 
fieldName, _ := GetFieldNameByTagValue(order, condition.Field, "json")
fieldValue, _ := GetField(order, fieldName)

so the name of the field that needs to changed is the value not the key we need to get the key from value stored as JSON here we need to get "cooking" from order json GetField(order, GetFieldNameByTagValue(order, condition.Field, "json"))

the external services doesn’t need to pay attention for how they are in go they just save it as normal JSON

Note I've added more explained example in example_test.go

@Mansouri147
Copy link
Author

Thanks for the effort in the review 🚀 🙌

@Mansouri147
Copy link
Author

Mansouri147 commented Sep 4, 2022 via email

@oleiade oleiade merged commit d6f33f5 into oleiade:master Sep 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants