-
Notifications
You must be signed in to change notification settings - Fork 102
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
Added calculation for relational cohesion metric #747
base: master
Are you sure you want to change the base?
Conversation
NodeJS v18 is not compatible with Ubuntu 16.04, due to the GLIBC mismatch.
Build CodeCompass in Release mode in CI
Log the start of each C++ metric computation
Debug info upon C++ parser finished a TU
Update Tarball generation for Ubuntu 20.04 Focal Fossa
Fix segmentation fault due to access of file ID on non-loaded file. Eliminated use of size() on ODB query result in SourceManager. Added isSingletonResult instead to dbutil.
…eCompass into RelationalCohesion
@kmkristof The PR was marked ready, but the description says the tests are still missing. (And there are indeed no tests in the changeset.) Please add the missing unit tests. |
@kmkristof The PR contains various commits unrelated to this PR for some reason. This makes it hard to review the PR. |
@@ -18,6 +20,7 @@ | |||
#include <util/odbtransaction.h> | |||
|
|||
#include <memory> | |||
#include <iostream> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why was this required? fstream
should be enough for file management.
object(CppAstNode : CppRecord::astNodeId == CppAstNode::id) \ | ||
object(File : CppAstNode::location.file) \ | ||
object(CppMemberType : CppMemberType::memberAstNode) | ||
struct RelationalCohesionRecordView |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the same view is used in #757. In this case a more generic name should be used and code duplication should be avoided.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A common view could probably be used however the same applies to "CohesionCppRecordView" for example ("RelationalCohesionRecordView" contains this + typeHash).
Maybe this should be looked at separately to unify the commonly used views?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You started working concurrently on a second issue before finishing this one. I don't advise to merge in duplicated code segments so close to each other in time, and then refactor them in a follow-up issue.
plugins/cpp_metrics/model/include/model/cpprelationalcohesion.h
Outdated
Show resolved
Hide resolved
plugins/cpp_metrics/parser/include/cppmetricsparser/cppmetricsparser.h
Outdated
Show resolved
Hide resolved
|
||
void CppMetricsParser::relationalCohesion() | ||
{ | ||
std::unordered_set<std::string> filepaths; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be filePaths
, following naming guidelines.
typeDefinitionPaths.insert(std::make_pair(record.typeHash,record.filePath)); //save where the type is defined to avoid self relation | ||
} | ||
|
||
std::unordered_map<std::string,std::unordered_set<std::uint64_t>> relationsFoundInFile; //store the type relations already found for each file |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is what unordered_multimap
is for. Wouldn't that simplify the code?
@mcserep I added the necessary data and definitions for unit testing however i ran into a problem. |
Closes #680
Added -m flag to the parser which can be used to specify modules. The user is expected to input a path to a file in which the paths to the modules are listed. If no modules are specified the default behaviour is to treat every directory under the input paths as a module.