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

Fan-in and Fan-out Metrics Seem to be Swapped #16

Closed
git-afsantos opened this issue May 22, 2018 · 5 comments · Fixed by #17
Closed

Fan-in and Fan-out Metrics Seem to be Swapped #16

git-afsantos opened this issue May 22, 2018 · 5 comments · Fixed by #17
Assignees
Labels

Comments

@git-afsantos
Copy link
Contributor

Hello,

The definitions for fan-in and fan-out in the CCCC User Guide are as follows.

For a given module A, the fan-out is the number of other modules which the module A uses, while the fan-in is the number of other modules which use A.

These definitions are very close to the common definitions for Afferent Coupling (fan-in) and Efferent Coupling (fan-out), and I was using CCCC (version 3.1.4) to gather such metrics.

In the top-level XML report (cccc.xml) it seems to me that, either I did not understand the metrics definitions correctly (and, if so, I would ask for some clarification), or the reported values are actually swapped.

Consider the following example code.

#include <string>

class A {
  std::string name;
  A(const std::string& _name) : name(_name) {}
};

class B {
  A a;
  B(const std::string& name) : a(name) {}
};

int main(int argc, char **argv) {
  B b1("1"), b2("2");
  return 0;
}

For class A the fan-in and fan-out are both 1, which is to be expected (A uses string and is used by B). For class B, however, I get a fan-out of 0 and a fan-in of 2, which is backwards.

The report for module B (B.xml) correctly reports class B as having two suppliers, A and string.

It seems to me that the problem is in the counting for fan-in and fan-out. Fan-in is being counted based on the suppliers and fan-out is being counted based on the clients. Since a supplier is used by the module, it should be fan-out, while a client uses the module, and thus should be fan-in.

If you agree with my assessment, and are still accepting bug fixes, I can even provide the pull request to swap these two values.


Also, I initially tried this example with the struct keyword, instead of class (which is more or less equivalent in C++), and in this case the classes are not counted as modules, even with the --lang=c++ option. But I understand that this is a separate issue.

@sarnold
Copy link
Owner

sarnold commented Aug 11, 2018

Sorry, another one lost in the notification flood... It's been a while since i looked at this code, but your assessment does sound valid; if you still want to submit a PR I'll give it a once (or twice) over. Thanks!

@sarnold
Copy link
Owner

sarnold commented Apr 25, 2019

bump I spent a little more time looking at this, and I still agree the definitions/code seem "swapped" from what they should be (mainly because of the human interpretation of client/supplier and uses/used-by). Hopefully @tim-littlefair can still chime in, but I'm inclined to test this and push a new release if things look good.

@sarnold sarnold self-assigned this Apr 25, 2019
@sarnold sarnold added the bug label Apr 25, 2019
git-afsantos added a commit to git-afsantos/cccc that referenced this issue Apr 26, 2019
@tim-littlefair
Copy link

tim-littlefair commented Apr 28, 2019 via email

@tim-littlefair
Copy link

I've looked at the code, and gone back and refreshed the definitions of FI and FO on the web (not the original Henry/Kafura paper unfortunately, but enough places to check there is a consensus), and it's clear that this is a bug. Mine. More than 20 years old. I hope no-one was relying on this over all this time...

On the topic of this observation from André ...

Also, I initially tried this example with the struct keyword, instead of class (which is more or less equivalent in C++), and in this case the classes are not counted as modules, even with the --lang=c++ option. But I understand that this is a separate issue.

...
I accept that the implementation of a type declared using the struct keyword in C++ is similar to the implementation of a type declared using the class keyword, but the two keywords have history and designed purpose associated with them which changes our expectation of how they should be used. I remember thinking about this issue when I was writing this code all those years ago, and I decided (rightly or wrongly) that the class keyword was intended by the designers of C++ to define types which had both data content and operations, and which would normally conform to an OO convention of having a public interface and private implementation details, whereas struct was inherited from ANSI C, allowed to gain some extra syntax in C++ for consistency with classes, but was not intended to be used for types which were intended to conform to the OO paradigms (information hiding, data private by default etc).

André (or anyone else) has a body of code to analyze in which this decision is turning out to be inconvenient, I suspect that a small change to the CCCC_Module::is_trivial() method would change behaviour so that structs would be treated as if they were classes.

@git-afsantos
Copy link
Contributor Author

Thanks for the reply, Tim.

this is the first time I've seen anyone take an interest in the Henry/Kafura information flow metrics

And CCCC is probably the only free tool measuring these, which I also find surprising, as they seem to convey useful information. Just so you know, I am contributing to a group that is making some efforts to improve quality of robotics software. These metrics belong to the set that the group has agreed upon.

the two keywords have history and designed purpose associated with them which changes our expectation of how they should be used

Agreed, and I understand your stance. It is a valid decision, just one that I was not expecting at first, so I figured I would report it as well.

I suspect that a small change to the CCCC_Module::is_trivial() method would change behaviour so that structs would be treated as if they were classes.

Thanks for the pointer. Might be useful if the need for this change arises. 👍

EelcoPeacs pushed a commit to EelcoPeacs/cccc that referenced this issue Jan 16, 2023
Adjusted the test reference files for the changes made for issue sarnold#16 in merge sarnold#17.
Also changed for new html files footer with new link to GitHub project
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants