Skip to content

Fix a few issues in the introduction #580

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

Merged
merged 5 commits into from
Apr 17, 2022

Conversation

illia-v
Copy link
Contributor

@illia-v illia-v commented Apr 9, 2022

Nice project, @rmk135!

I found a few issues when I read the introduction:

  • "When the cohesion is high the coupling is low." – this is not always true, see the article;
  • some typing issues: passing a string when an integer is expected and missing return types when arguments are annotated.

@@ -49,7 +49,7 @@ Coupling and cohesion are about how tough the components are tied.
- **High cohesion**. High cohesion is like using the screws. Very easy to disassemble and
assemble back or assemble a different way. It is an opposite to high coupling.

When the cohesion is high the coupling is low.
High cohesion often correlates with loose coupling, and vice versa.
Copy link
Member

Choose a reason for hiding this comment

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

What do you think about something like that:

“Cohesion often correlates with coupling. Higher cohesion usually leads to lower coupling, and vice versa.”

Copy link
Contributor Author

Choose a reason for hiding this comment

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

LGTM! Changed in f8ec061

@rmk135
Copy link
Member

rmk135 commented Apr 9, 2022

Hi @illia-v ,

Thanks a lot for the PR and provided link.

As of the PR, could I please ask you to change the destination branch to develop?

Also, if I’m not wrong the same code sample is used on docs index page, on readme, and also there’s example code somewhere in the “/examples/“ folder. It would be nice to make the typing updates there as well. Please, let me know if it’s something you could help with.

Speaking about the thoughts on correlation of coupling and cohesion, I’m good to change the wording to the softer form, but generally I would say that it’s a separate discussion. For instance, when I think about God Object, I imagine a class with 30-50 methods calling each other in chaotic order and I cannot imagine if there’s room to call it highly cohesive & highly coupled structure — for me it looks just like an example of high coupling.

Same about destructive cohesion, I totally agree about the term, reasoning, consequences and everything else, but I’m not sure if it’s any special type of correlation. I would say that it’s just an extreme level of cohesion, where extremely low level of coupling actually creates a problem.

PS: I’m happy to have a discussion about that, but probably would stop here for now :)

illia-v and others added 2 commits April 13, 2022 21:35
@illia-v illia-v changed the base branch from master to develop April 13, 2022 18:40
@illia-v
Copy link
Contributor Author

illia-v commented Apr 13, 2022

As of the PR, could I please ask you to change the destination branch to develop?

Sure, done.

Also, if I’m not wrong the same code sample is used on docs index page, on readme, and also there’s example code somewhere in the “/examples/“ folder. It would be nice to make the typing updates there as well. Please, let me know if it’s something you could help with.

Done.


Speaking about the thoughts on correlation of coupling and cohesion, I’m good to change the wording to the softer form, but generally I would say that it’s a separate discussion. For instance, when I think about God Object, I imagine a class with 30-50 methods calling each other in chaotic order and I cannot imagine if there’s room to call it highly cohesive & highly coupled structure — for me it looks just like an example of high coupling.

Same about destructive cohesion, I totally agree about the term, reasoning, consequences and everything else, but I’m not sure if it’s any special type of correlation. I would say that it’s just an extreme level of cohesion, where extremely low level of coupling actually creates a problem.

PS: I’m happy to have a discussion about that, but probably would stop here for now :)

There was a discussion in comments of the article about God object. It looks unfinished, unfortunately.

The article author defines "the number of connections inside a code component (cohesion) and between code components (coupling)." If this definition is used, a God object does seem to have high cohesion, and destructive decoupling does seem to result in low cohesion.

BTW, there is an article from his opponent, he provides some other examples of low cohesion + loose coupling and high cohesion + tight coupling.

Speaking for myself, I am still defining cohesion and coupling 🙂. They are pretty subjective.

@illia-v illia-v requested a review from rmk135 April 13, 2022 18:55
@rmk135 rmk135 merged commit daca85d into ets-labs:develop Apr 17, 2022
@rmk135
Copy link
Member

rmk135 commented Apr 17, 2022

@illia-v many thanks for the changes, merged. Also, thanks a lot for some education about cohesion and coupling, super helpful to look at things from various perspectives :) I'll definitely mention this PR when speak about coupling and cohesion next time.

@rmk135 rmk135 self-assigned this Apr 17, 2022
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

Successfully merging this pull request may close these issues.

2 participants