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

Some changes in connections #111

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Conversation

tombenke
Copy link

Dear Javier,

  1. I think the counterpart of inheritance in golang is the embedding of structures, so in case of the extends relation the current connection is not correct in my opinion. Either the *-- should point to the opposite direction or it's even better to use the <|-- relation instead.

  2. In case of interfaces the implements relation should be noted with the dotted line, instead of the solid one.

So, I would like to kindly ask you to accept and approve these two changes in the generated notations:

  • Change 'extends' connection from *-- to <|--
  • Change 'implements' connection from <|-- to <|..

I also added a go.mod file to the repo:

  • Add go.mod file to the repo

Best Regards,
Tamás Benke

Change 'implements' connection from <|-- to <|..
Add go.mod file to the repo
@jfeliu007
Copy link
Owner

jfeliu007 commented Mar 25, 2021

Thanks, @tombenke for your suggestion. Let me take a look at this to see how it changes the definitions. In any case, this PR is failing tests because it changes how relations are rendered. If I ultimately agree with you on this change, we need to have a PR that does not break the testing of the packages.

@codecov
Copy link

codecov bot commented Mar 25, 2021

Codecov Report

Merging #111 (c0fda4d) into master (2adf2cf) will not change coverage.
The diff coverage is 100.00%.

❗ Current head c0fda4d differs from pull request most recent head 13c6c11. Consider uploading reports for the commit 13c6c11 to get more accurate results

@@            Coverage Diff             @@
##            master      #111    +/-   ##
==========================================
  Coverage   100.00%   100.00%            
==========================================
  Files            5         5            
  Lines          625       510   -115     
==========================================
- Hits           625       510   -115     
Impacted Files Coverage Δ
parser/class_parser.go 100.00% <100.00%> (ø)
parser/alias.go 100.00% <0.00%> (ø)
parser/field.go 100.00% <0.00%> (ø)
parser/struct.go 100.00% <0.00%> (ø)
parser/function.go 100.00% <0.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@tombenke
Copy link
Author

Sorry, I forgot to change the test cases, now I have fixed them, hopefully I made it well.
Anyway, what about using GitHub actions instead of Travis?

@fractalqb
Copy link

Dear Javier,

1. I think the counterpart of inheritance in golang is the embedding of structures, so in case of the extends relation the current connection is not correct in my opinion. Either the `*--` should point to the opposite direction or it's even better to use the `<|--` relation instead.

[…]

Sorry, but I have to disagree. Inheritance has something to do with the substitution principle, i.e. everywhere where a baseclass is required a derived class can be passed. For Go struct + embedding this is simply not the case. IMHO Go only has interface inheritance, i.e. embedding of interfaces – and that's it.

Embedding of struct only is some syntactical sugar that makes addressing of fields somewhat less verbose. But I don't see a real correspondence to UML concepts.

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.

None yet

3 participants