How can I reduce Cognitive Complexity in this block ? By the way how can this be bigger than the 15 allowed?

Question

I have this code:

 private void processMedia(Integer mediaId, List<String> hiresPhysicalPaths) {
        final AtomicBoolean isHls = new AtomicBoolean(false);
        for (String hiresPhysicalPath : hiresPhysicalPaths) {
            REPORT.info("Process media version {} for media {} ", hiresPhysicalPath, mediaId);
            String folderName = StringUtils.substringBefore(hiresPhysicalPath, "/");
            File dir = new File(MediaRepositoryTools.getCurrentMediaPhysicalRootPath(App.getApplicationSession()), folderName);
            REPORT.info("Dir : {}", dir);
            File[] directoryListing = dir.listFiles();
            if (directoryListing == null) {
                REPORT.warn("Dir is not really a directory");
                return;
            }
            String mediaIdString = mediaId.toString();
            // loop pour savoir si le média est HLS ou pas
            for (File child : directoryListing) {
                String ext = FilenameUtils.getExtension(child.getName()).toLowerCase();
                String childFileName = FilenameUtils.getBaseName(child.getName());
                //If m3u8 file ex : 3180734-9u83wns9eh-m3u8.m3u8
                try {
                    if (isMediaHls(mediaIdString, childFileName, ext)) {
                        miseAJourFichierHLS(child);
                        REPORT.info("Le fichier m3u8 a été mis à jour pour la version : {} du média {} ", hiresPhysicalPath, mediaId);
                    } else {
                        REPORT.info("Le fichier m3u8 n'a pas été mis à jour pour la version : {} du média {} ", hiresPhysicalPath, mediaId);
                    }
                } catch (IOException e) {
                    REPORT.error("Unable to update the HLS file for media " + mediaId, e);
                }
                isHls.set(true);

                if (childFileName.startsWith(mediaIdString) && isFileNeededToBeDeleted(child.getName(), isHls.get())) {
                    this.copyOrDelete(child);
                }
            }
        }
        App.getService(TransactionService.class).withTransaction(() -> this.updatePreviewGeneratedFlagForMedia(mediaId, isHls.get()));
    }

Sonar told me to reduce the Cognitive Complexity from 16 to 15, I don't know how can it be that much since there are no nested condition in there, maybe i'm wrong.

EDIT : I edited the code in order to show you the original one with nothing missing, I'll take into account your concerns especially about the code complexity

One way is to extract the inner blocks of code into separate methods. — Hulk, Jul 15 '22 at 08:50
Well, you've got an `if` inside a `try`, inside a `for` inside another `for`. That seems a lot of nesting to me. And this method feels to me like it's about 3x longer than it should be. Have you thought about factoring the content of the inner loop into another method? So everything from `String ext =` down to `this.copyOrDelete(child)`. Except possibly `isHls.set(true);` - that might be best staying where it is. — Dawood ibn Kareem, Jul 15 '22 at 08:50
Also, I think some of your code is missing from the question - just before the `else` you have a left parenthesis with no matching right parenthesis. — Dawood ibn Kareem, Jul 15 '22 at 08:53
And not what you asked, but I don't see the point of creating an `AtomicBoolean` inside a method. There's only ever one thread that can see that value. — Dawood ibn Kareem, Jul 15 '22 at 08:55
@DawoodibnKareem there's no missing right bracket, only a very misleading indentation style (the right bracket under the `if` actually belongs to the `else`!) — Klitos Kyriacou, Jul 15 '22 at 08:56
@KlitosKyriacou I'm pretty sure you're mistaken. Look just to the left of the `else`. — Dawood ibn Kareem, Jul 15 '22 at 08:58
@DawoodibnKareem I see what you mean, there is indeed a missing parenthesis. (As well as the very misleading indentation style) — Klitos Kyriacou, Jul 15 '22 at 08:59
And `isHls` is set but never read, so what's the point of it? — Klitos Kyriacou, Jul 15 '22 at 08:59
@KlitosKyriacou I'm pretty sure you're mistaken about that too. Scroll to the right and look near the bottom. — Dawood ibn Kareem, Jul 15 '22 at 09:00
`isHls` it is read twice, but in one of those places it is always `true`... — Hulk, Jul 15 '22 at 09:00
Sorry about the logs, I deleted the content because I have to modify it, et when I deleted it i probably deleted the right parenthesis. — JhinKazama, Jul 15 '22 at 09:03
@GeertPt I don't think you should have done that. We don't know what else OP has missed out after the orphaned parenthesis. It would be better if we could all see the code exactly as OP first posted it. If OP wishes to correct it, they can do so. But unless you've seen the original code, you shouldn't correct it for them. — Dawood ibn Kareem, Jul 15 '22 at 09:09
@GeertPt that kind of changes can be misleading (changing the question) -- are you sure there was nothing else deleted on that part? Maybe some conditional or whatever. (typos and spelling in TEXT are OK, but code is very difficult, unless it is just formatting) ((+10k user should be aware of that... [When should I make edits to code?](https://meta.stackoverflow.com/q/260245/16320675): "Fix syntax (...) **if you are sure that it is not relevant to the question**" {emphasis **not** mine})) — user16320675, Jul 15 '22 at 09:14
I saw that OP has reputation 1, so I assume he couldn't edit his question, so I just applied what he said in comments. And from the question I know the original code would have compiled, and I trust he would not omit essential parts of the question. — GeertPt, Jul 15 '22 at 09:21

Nico · Accepted Answer · 2022-07-15T09:10:34.107

I think you're thinking a little bit wrong about Cognitive Complexity.

The general idea is the following:

Ignore structures that allow multiple statements to be readably shorthanded into one
Increment (add one) for each break in the linear flow of the code
Increment when flow-breaking structures are nested

So what happens in your code?
Every declaration of a variable is +1 for the score.
Every for loop declaration is +1.
Every try catch and multiple catches are +1.
You can read about it more here

What could you do to fix this?

Well you could check if every variable is necessary.
You could try to get rid of the try and catch or if clauses.
What I would do is trying to create another method for this part:

    for (File child : directoryListing) {
            String ext = FilenameUtils.getExtension(child.getName()).toLowerCase();
            String childFileName = FilenameUtils.getBaseName(child.getName());
            //If m3u8 file ex : 3180734-9u83wns9eh-m3u8.m3u8
            try {
                if (isMediaHls(mediaIdString, childFileName, ext)) {
                    miseAJourFichierHLS(child);
                    REPORT.info(" TODO "     } else {
                    REPORT.info(" TODO  ", hiresPhysicalPath, mediaId);
                }
            } catch (IOException e) {
                REPORT.error("TODO " + mediaId, e);
            }
            isHls.set(true);

            if (childFileName.startsWith(mediaIdString) && isFileNeededToBeDeleted(child.getName(), isHls.get())) {
                this.copyOrDelete(child);
            }
        }

Also you should try to split your method in more smaller parts which you can easy understand which part does what. Your processMedia method does too much. Make it more simple and devide it. This is an approach from clean code. You can read a little bit more about it here

Thanks for you explanation. I was effectively wrong about Cognitive Complexity — JhinKazama, Jul 15 '22 at 09:26

How can I reduce Cognitive Complexity in this block ? By the way how can this be bigger than the 15 allowed?

1 Answers1