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

introduction of mocking on "Functions should only be one level of abstraction" should be removed #51

Open
xMarkusx opened this issue Sep 3, 2017 · 4 comments

Comments

@xMarkusx
Copy link

xMarkusx commented Sep 3, 2017

https://github.com/jupeter/clean-code-php#functions-should-only-be-one-level-of-abstraction

I think your refactoring results in two helper-classes "Lexer" and "Tokenizer". These types of classes should not be mocked nor handled by the di-framework.

Uncle Bob suggests to

Mock across architecturally significant boundaries, but not within those boundaries.

http://blog.cleancoder.com/uncle-bob/2014/05/10/WhenToMock.html

@peter-gribanov
Copy link
Contributor

@xMarkusx please explain, i not understood you.
Did i do wrong by taking Lexer and Tokenizer to dependances?

If i leave Lexer and Tokenizer inside of BetterJSAlternative, i can't re-use them for other purposes or another language.

@xMarkusx
Copy link
Author

xMarkusx commented Sep 5, 2017

No there is nothing wrong with the refactoring. Maybe this example is to abstract as i don't fully understand what Lexer, Tokenizer and BetterJSAlternative are supposed to do.

Since this is unclear, i would simply remove the last sentence
"Now we can mock the dependencies and test only the work of method BetterJSAlternative::parse()."
It could be better to mock them but it could be also correct to test them indirect through the parse function of BetterJSAlternative. It depends on the implementation of Lexer and Tokenizer.

Maybe the topic unit testing and mocking should be handled in a separate section where we can go more into detail when and how to mock?

@peter-gribanov
Copy link
Contributor

I also do not understand the purpose of the parseBetterJSAlternative() function.
I made a guess from the context. Maybe i was wrong.

In one thing, i agree with you. The example is too complex and not obvious.

We need to think about the best example.

@ranggakd
Copy link

ranggakd commented Oct 4, 2022

please closed this once it is solved already @xMarkusx @peter-gribanov
thank you 🙏

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

No branches or pull requests

3 participants