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

TCC.xsl:48-50: directly-related-pairs currently don't... #120

Closed
0pdd opened this issue Jan 28, 2018 · 23 comments
Closed

TCC.xsl:48-50: directly-related-pairs currently don't... #120

0pdd opened this issue Jan 28, 2018 · 23 comments

Comments

@0pdd
Copy link
Collaborator

0pdd commented Jan 28, 2018

The puzzle 9-57e7e31a from #9 has to be resolved:

https://github.com/yegor256/jpeek/blob/b625770426cbfa670c987c135c0b3c98b7a24c8d/src/main/resources/org/jpeek/metrics/TCC.xsl#L48-L50

The puzzle was created by Kapralov Sergey on 26-Jan-18.

Estimate: 30 minutes, role: IMP.

If you have any technical questions, don't ask me, submit new tickets instead. The task will be "done" when the problem is fixed and the text of the puzzle is removed from the source code. Here is more about PDD and about me.

@0crat
Copy link
Collaborator

0crat commented Jan 28, 2018

@yegor256/z please, pay attention to this issue

@yegor256 yegor256 added the bug label Jan 28, 2018
@0crat 0crat added the scope label Jan 28, 2018
@0crat
Copy link
Collaborator

0crat commented Jan 28, 2018

Job #120 is now in scope, role is DEV

@0crat
Copy link
Collaborator

0crat commented Feb 14, 2018

The job #120 assigned to @llorllale/z, here is why. The budget is 30 minutes, see §4. Please, read §8 and §9. If the task is not clear, read this and this.

llorllale added a commit to llorllale/jpeek that referenced this issue Feb 14, 2018
llorllale added a commit to llorllale/jpeek that referenced this issue Feb 14, 2018
@llorllale
Copy link
Contributor

My own notes:

  • They keep using the term "instance variable" but as far as I can see they do not actually differentiate it from class variables
  • They deliberately exclude ctors from the analysis because "It will generally access all instance variables in the class, and thus, share instance variables with virtually all other methods. Constructors create connections between methods even if the methods do not have any other relationships."
  • They deliberately exclude "non-visible" methods from outside the class because "Class cohesion refers the relatedness of visible components of the class which represent its functionality. Class cohesion is the degree that those components are related. In our model of class cohesion, invisible components of a class are included only indirectly through the visible components."
  • They leave open the possibility for including inherited methods and instance variables. We are currently including the FQN of methods used (will do the same for fields in the near future), but we also need to know if the owning class is a parent of the one under analysis. I opened Skeleton is broken: no way to tell what a class inherits from its parent #187 for this.
  • Formula is TCC(C) = NDC(C)/NP(C), where NP is the max number of possible connections (N*(N-1)/2), and NDC is the number of directly connected methods
  • The direct connectivity between methods is determined from the class abstraction. If there exists one or more common instance variables between two method abstractions then the two corresponding methods are directly connected.
  • They note the following when they applied their cohesion measure: "Overloaded methods within the same class are treated as one method."
  • Class cohesion measures for visible methods defined only within the class are also useful, because the measures are not affected by the cohesion of a superclass. This should be configurable?
  • Quote: "Our results show that the classes that are heavily reused via inheritance exhibit lower cohesion".

@llorllale
Copy link
Contributor

Based on my notes above I've decided the following:

  • TCC does not care about relations between methods via method calls, so the request to do so in the puzzle will be discarded
  • Considering the consistent use of the term "instance variable" and their consideration of inheritance, etc., I am going to only consider class attributes that are not static.
  • I am going to exclude ctors
  • I am going to exclude private methods
  • I am going to leave a puzzle to come back and fix TCC after Skeleton is broken: no way to tell what a class inherits from its parent #187 is closed. Inclusion of inherited attrs and methods should be configurable.

llorllale added a commit to llorllale/jpeek that referenced this issue Feb 15, 2018
Revised after these notes were taken:
cqfn#120 (comment)
@0pdd
Copy link
Collaborator Author

0pdd commented Feb 17, 2018

The puzzle 9-57e7e31a has disappeared from the source code, that's why I closed this issue.

@0crat
Copy link
Collaborator

0crat commented Feb 17, 2018

@ypshenychka/z please review this job, as in §30

@llorllale
Copy link
Contributor

@yegor256 missed points+payment on this one

@llorllale
Copy link
Contributor

@0crat status

@0crat
Copy link
Collaborator

0crat commented Feb 18, 2018

@0crat status (here)

@llorllale This is what I know about this job, as in §32:

  • The job #120 is in scope for 21days
  • The role is DEV
  • The job is assigned to @llorllale/z for 4days
  • There is a monetary reward attached
  • @llorllale/z will get 50% of their hourly rate for this job
  • The job doesn't have any impediments
  • The budget is 30 minutes/points
  • These users are banned and won't be assigned:
    • @0pdd/z: This user reported the ticket
  • Job footprint (restricted area)

@0crat
Copy link
Collaborator

0crat commented Feb 19, 2018

@llorllale/z this job was assigned to you 5 days ago. It will be taken away from you soon, unless you close it, see §8. Read this and this, please.

@ypshenychka
Copy link

@0crat quality good

@llorllale
Copy link
Contributor

@ypshenychka seems like you must also review the associated PR? (#182). See this points in the policy:

Pull request title explains the problem it is fixing.
Pull request description explains the solution proposed and contains a link to the original ticket it is related to.
The code reviewer found at least three problems in the code.
Comments were mostly about design problems, not cosmetic issues.
All problems found by the code reviewer were addressed by the pull request author.

@0crat
Copy link
Collaborator

0crat commented Feb 19, 2018

QA review completed: +15 points just awarded to @ypshenychka/z, total is +180

@llorllale
Copy link
Contributor

@ypshenychka nevermind

@0crat
Copy link
Collaborator

0crat commented Feb 19, 2018

Order was finished, quality was "good": +35 points just awarded to @llorllale/z, total is +655

@0pdd
Copy link
Collaborator Author

0pdd commented Feb 22, 2018

@0pdd these puzzles are still not solved: 193, 194, 195

@0pdd
Copy link
Collaborator Author

0pdd commented Feb 23, 2018

@0pdd these puzzles are still not solved: 193, 194, 195

1 similar comment
@0pdd
Copy link
Collaborator Author

0pdd commented Feb 23, 2018

@0pdd these puzzles are still not solved: 193, 194, 195

@0pdd 0pdd mentioned this issue May 28, 2018
@0pdd
Copy link
Collaborator Author

0pdd commented Mar 1, 2020

@0pdd 2 puzzles #193, #194 are still not solved; solved: #195.

@0pdd
Copy link
Collaborator Author

0pdd commented Feb 5, 2021

@0pdd 2 puzzles #495, #496 are still not solved.

@0crat
Copy link
Collaborator

0crat commented Feb 5, 2021

@0crat status (here)

@llorllale This is what I know about this job in C7JGJ00DP, as in §32:

@0crat
Copy link
Collaborator

0crat commented Feb 5, 2021

@0crat quality good (here)

@ypshenychka You @ypshenychka/z need to have one of these roles in order to do "Complete QA review": ARC, QA; I'm sorry to say this, but at the moment you've got no roles in this project

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants