DD2480-2022-group-7 / Java

All Algorithms implemented in Java
MIT License
0 stars 0 forks source link

Method LinkList_Sort::isSorted #3

Open samuelsoder opened 2 years ago

samuelsoder commented 2 years ago

The implementation of the Linked List isSorted method in src/main/java/com/thealgorithms/sorts/LinkList_Sort.java has reported cyclomatic complexity 16 by lizard.

Part 1 steps:

Part 2 steps:

tjog commented 2 years ago

The isSorted method is currently not tested by mvn test and therefore does not get counted towards the jacoco report. There is a test file for the class in src/test/java/com/thealgorithms/others/LinkList_Sort_test.java, but that does not get run since it ends in test and not Test (yeah, jUnit not very clear on this). Anyway, when the test file is renamed and moved to mirror the corresponding implementation's package com.thealgorithms.sort it turns out the tests fail. The isSorted method contains lines that always sorts the linkedlist they build, and a couple tests did assertFalse, which then failed of course.

Anyhow, fixing up the test file and removing the try (Scanner sc = new Scanner(System.in)) (it was messing with jacoco somehow), results in the following coverage report for the class and method isSorted:

Class summary ![image](https://user-images.githubusercontent.com/28024277/154723753-fa15c6b4-276c-4ea7-8865-399129172c0a.png)
isSorted ![image](https://user-images.githubusercontent.com/28024277/154726257-96affdd1-8a05-441a-828d-551dcdf5699f.png) ![image](https://user-images.githubusercontent.com/28024277/154726388-2c435254-f076-432d-9a54-4c9c46c78be3.png)
tjog commented 2 years ago

Manual instrumentation can be viewed in https://github.com/DD2480-2022-group-7/Java/commit/e24b6851f1129b023d45d1d1bfba61d8aee58bea . It reported the same result as jacoco at 85% coverage.

tjog commented 2 years ago

The current implementation of the LinkList_Sort::isSorted method can only get its branch coverage increased in the switch case (no default: test case currently) and the other branches are actually unreachable code no matter the input.

Lines https://github.com/DD2480-2022-group-7/Java/blob/45e8a821573e488a4a7a672ebb1318a94a8fa167/src/main/java/com/thealgorithms/sorts/LinkList_Sort.java#L13-L15 reference the same object as the input p is not copied. Then running whatever sort algorithm was chosen on a linked list and then copying it over to "a" actually writes to p. The Arrays.sort(b) afterwards then sorts the same underlying p again, since it already should be sorted. In total, the LinkList_Sort.compare is invoked referencing a and b even though they are exactly the same, which means it will always return true, making the other branch unreachable.

tjog commented 2 years ago

Before being able to increase coverage the unreachable branches needs to be addressed. The commit https://github.com/DD2480-2022-group-7/Java/commit/9b5de875d4d63175ccef595fb6abd4bb264844d6 begins on that work by making the interface/implementation consistent across the different sorting options you can provide. Next steps are hard to imagine without much refactoring. The proposed new purpose of this function is not sort and check it is sorted, but only "sort by this method and return the start Node". The control that something is actually sorted should be part of the test cases, which could call this method with a sorted list.

Next steps:

tjog commented 2 years ago

New tests added in https://github.com/DD2480-2022-group-7/Java/commit/a162699a9ae9fa6fa6cc3bd751aff860c10fcc8d result in 100% coverage, see the following JaCoCo report:

Entire class ![image](https://user-images.githubusercontent.com/28024277/155360459-f4d8d06e-b065-417a-b39c-e4458bda6083.png)
sortWith method ![image](https://user-images.githubusercontent.com/28024277/155360547-45aa1f19-c5eb-49bf-b3ee-d7470d083c13.png)
isSorted method ![image](https://user-images.githubusercontent.com/28024277/155360620-32dc7321-e04e-42ee-b9cc-2755cb52ea16.png)