Each table is on separate pages. Is it considered duplication (written in Angular)?
The DevOps team ran SonarQube and detected the th
tags as code duplication. How can I explain to them that it's not duplication, but rather the "use" of that component (table)? Wrapping the table with a component would result in "over-abstraction," leading to more complex and less maintainable code. Is there any official coding principle I can refer to?
from my point of view, each page consider it's own ui and i assamble the components/ui-elements and display what i need. it's give me more flexable to change the column in one page and in another page to use another column.
<!-- Table for Users -->
<table>
<thead>
<tr>
<th>ID</th>
<th>Name</th>
<th>Date</th>
</tr>
</thead>
<tbody>
<tr *ngFor="let user of users">
<td>{{ user.userId }}</td>
<td>{{ user.userName }}</td>
<td>{{ user.lastLogin }}</td>
</tr>
</tbody>
</table>
<br>
<!-- Table for Products -->
<table>
<thead>
<tr>
<th>ID</th>
<th>Name</th>
<th>Date</th>
</tr>
</thead>
<tbody>
<tr *ngFor="let product of products">
<td>{{ product.productId }}</td>
<td>{{ product.productName }}</td>
<td>{{ product.dateAdded }}</td>
</tr>
</tbody>
</table>
-
How you can explain it to them depends on their ability to understand. If they are competent, they will see that using the same label in different UIs isn't code duplication and will ignore this finding as a false positive. If they're incompetent, then good luck convincing them!Hans-Martin Mosner– Hans-Martin Mosner06/06/2024 10:02:32Commented Jun 6, 2024 at 10:02
-
There is no official coding principle that can help you. In fact there is no such thing as "official" coding principle to begin with. This is a matter of communication and cooperation. You need to convince them that it is fine. SonarQube can be set to ignore this issue, so it is not a technical problem at all.freakish– freakish06/06/2024 10:39:10Commented Jun 6, 2024 at 10:39
-
Are they really suggesting that SQ is right and you should write a component with the th ID,Name,Date in?Ewan– Ewan06/06/2024 17:35:07Commented Jun 6, 2024 at 17:35
3 Answers 3
What you are seeing is a false-positive outcome from the algorithm used by the tool to detect possible code duplication.
A tool like SonarQube cannot, by definition, reliably tell if two seemingly identical pieces of code are the result of code duplication that needs to be addressed, or if those pieces of code are just the same by accident.
There is a case of code duplication when two pieces of code (larger than N lines each) are the same and when one of them is changed, the other should be changed in the same way to keep fulfilling all requirements.
If the two pieces of code are expected to change independently when the requirements change, then it is just similar code, but not duplicated code.
A tool like SonarQube cannot know if seemingly identical code needs to remain identical or if it is expected to diverge over time, so it can only flag areas of code that need to be investigated for duplication.
In this case, your response to the DevOps team should be along the lines of
Yes, we have some tables that currently use the same column headings. However, these tables are completely independent and for either the number of columns or the column headings can be changed by our stakeholders without affecting the other table. For that reason, the code is not a duplicate and the warning from SonarQube should be marked as a false-positive.
Sonarqube has a button you can click to say "accepted/ignored" etc for each thing it raises.
So for stuff like this where you realise there is technically duplication, but think its better that way. You can just mark it accepted.
Note: this isn't just for show, as it will change your overall score. So if you click it before you show people your PR they will just see the good score and you wont have to explain yourself
-
For someone like me who does not know Sonarqube - will that tool remember the acceptance and not complain again at the next run?Doc Brown– Doc Brown06/06/2024 16:00:48Commented Jun 6, 2024 at 16:00
-
yup, seems to im not an expert with SQ but i do have a lot of experience telling it that i know better :)Ewan– Ewan06/06/2024 16:30:15Commented Jun 6, 2024 at 16:30
-
What does that mean? "Yes- it will remember the choice and not ask again", or "No - it will not remember", or "you dont know"?Doc Brown– Doc Brown06/06/2024 16:40:37Commented Jun 6, 2024 at 16:40
-
yes, i have clicked it and its not nagged me again. No I havent checked the manual to see exactly what will cause a recompute or how long it lasts etcEwan– Ewan06/06/2024 17:29:26Commented Jun 6, 2024 at 17:29
-
I like this answer because it's just a naïve tool. I've had security scanners mark every single
foreach
loop in our C# project as a security flaw because it is an infinite loop. Marking it resolved or accepted and moving on with life is the way to go.Greg Burghardt– Greg Burghardt06/06/2024 23:54:41Commented Jun 6, 2024 at 23:54
Although Bart van Ingen Schenau gives a good explanation about what SonarQube is telling you and Ewan succinctly addresses how to handle it in SonarQube, there's an underlying systemic issue here.
It sounds like an external team is running various scans on your system and reporting errors to your team. You need to either fix them or provide a justification for not fixing them.
In some respects, I can understand having a centralized team own and manage tools, like SonarQube for static analysis and code quality, Black Duck for software composition analysis, JFrog Artifactory for build artifacts, etc. In my experience, it can be cheaper to manage these globally across an organization, and having a central team lets that team gain expertise in managing and using these tools to guide the teams using them as part of their work.
The issue you're experiencing is that this type of team, who doesn't have expertise in the system being built, is trying to enforce rules on the teams with that expertise. It could be good for this "DevOps" team to own the tools, help the teams configure them for each system under development, and make sure that reports are made available to the teams and that the teams understand what the reports say. However, no development team should have to justify to this team when a finding is a false positive or not. The teams themselves should be in a position to mark findings as false positives or otherwise ignore them if they are irrelevant.
There may be exceptions to this for security findings, for example. A global team that runs application vulnerability scans may be better positioned to help a team understand a vulnerability and then justify why a detected vulnerability is a false positive or lower risk than the scanner says. However, this would require that the centralized team have security expertise and not just tool expertise.