search

albertogoffi / toradocu

Toradocu - automated generation of test oracles from Javadoc documentation
Other
42 stars 21 forks source link

Fix normalization of "iff" in Javadoc comments #155

Closed khaeghar closed 7 years ago

khaeghar commented 7 years ago

Minor changes

khaeghar commented 7 years ago

Yes, is for the substring issue. The only case that would need an special treat would be the throws in case it starts with an "iff". But yes, for things like this there should be an special treatment, I'll do it inside the preprocessor or in another one.

On Jun 23, 2017 11:53 AM, "Alberto Goffi" notifications@github.com wrote:

@albertogoffi commented on this pull request.

Can you explain why we need a whitespace before and after "iff"? Is that to avoid to replace "iff" with "if" when "iff" is a substring of a word? In that case, should we apply the same strategy for the other replacement? And what if the "iff" is the beginning of a comment (i.e., there is no trailing whitespace)?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/albertogoffi/toradocu/pull/155#pullrequestreview-45944250, or mute the thread https://github.com/notifications/unsubscribe-auth/AU0fSHgSYh4nAvhlIs4FYWLVUXdUpfPJks5sG4shgaJpZM4ODUW_ .

albertogoffi commented 7 years ago

Yes, is for the substring issue.

Clear, thanks.

The only case that would need an special treat would be the throws in case it starts with an "iff". But yes, for things like this there should be an special treatment, I'll do it inside the preprocessor or in another one.

I'd not make assumptions on the input comment, and I'd keep the replacement in the preprocessor. I suggest to use String#replaceAll, String#replaceFirst to replace the occurrences of a pattern with proper replacement. Probably we can keep a map regex pattern -> replacement and then apply the replacement for each key-value pair in the map.

As a note: the class you are changing is called NormalizeIfs. However, NormalizeIfs also replaces "non-empty" and "non-null", and this is misleading. Can you move the code replacing "non-empty" and "non-null" to a new class (or classes)?