search

joomcode / errorx

A comprehensive error handling library for Go
MIT License
1.13k stars 29 forks source link

delete String function #5

Closed RikiyaFujii closed 6 years ago

RikiyaFujii commented 6 years ago

Hello. I has a doubt about the String function of namespace.go .Since processing with this function is made by FullName () function, I felt it was not necessary. I deleted the String() function and tested it, but I passed.

PeterIvanov commented 6 years ago

Hello, At a glance, I can see no harm in deleting this function, but also no gain. Is there a particular example where the presence of Namespace.String() is harmful or confusing? I would propose to change it instead, so that String() returned full name, as it probably should.

PeterIvanov commented 6 years ago

My bad, it actually already is consistent with FullName().

RikiyaFujii commented 6 years ago

Thanks for your reply. It is certainly necessary to check whether the String () function is harmful, but I think that the String () function itself is not harmful. The reason why I deleted the String () function is consistent with the FullName () function. It is, so to speak, refactoring. I think that functions that are not needed should be deleted.

PeterIvanov commented 6 years ago

String() may not be used directly in the package itself, but it will be used by any user code that uses formatter:

  fmt.Sprintf("namespace %s", namespace)

If anything, it is FullName() function that is unnecessary. But it gives more clarity to the code that uses it, as opposed to using String() function. At some point, String() may change its behaviour, but FullName() will not.

RikiyaFujii commented 6 years ago

Thanks for your reply. Looking to your story, I judged that the String () function should not be deleted now. Thank you for a polite review.