Commit
- Loading branch information
There are no files selected for viewing
5 comments
on commit 6d1d48d
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a habit really, nothing ever references @env directly so it’s purely an implementation detail for within that method.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@NZKoz: Ok, thanks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have you seen ticket #1158
This should resolve the performance issue, although I’m still unsure as why the require is inside the helper method – seems like a really strange pattern to follow.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sentientmonkey
Why do you find it strange that the require is inside the method body? It makes perfect sense to me: only load the StringInquirer class when it’s actually needed. Also, it helps keeping track of which require is used for what. However, it should be taken out of the method body as soon as the class is used somewhere else within the same file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The reason for the require inside the method is described in the ticket you’ve linked to. it is kinda strange but the specific case it was needed for is kinda strange too.
Why the underscore?