Search code examples
sonarqube

SonarQube showing Cognitive Complexity error Even there is only 7 loops


I am review my code using SonarQube. I am receiving the following issue.

Refactor this method to reduce its Cognitive Complexity from 21 to the 15 allowed.

But my method contains only 7 loops. Herewith I attached the code.

private void LayoutTouch(int touchType, int index) {

        if (touchType != -1) { //+1
            try {
                ViewConfiguration vc = ViewConfiguration.get(ctContext);
                mSlop = vc.getScaledTouchSlop();
                parentLayout[index]
                        .setOnTouchListener(new View.OnTouchListener() {
                            @Override
                            public boolean onTouch(View v,
                                                   MotionEvent event) {
                                if(!isValidEvent()){ //+2
                                    return false;
                                }

                                if(checkTouchIndex(index)){ //+3
                                    try{
                                        
                                        // Code here

                                        if (animationStarted) { //+4
                                            return false;
                                        }

                                        final ViewConfiguration vc = ViewConfiguration
                                                .get(getContext());
                                        final int deltaX = (int) (event.getX() + 0.5f)
                                                - mGestureCurrentX;


                                        initiateVelocityTracker();
                                        mVelocityTracker.addMovement(event);

                                        mVelocityTracker
                                                .computeCurrentVelocity(1000);

                                        if(!doSwitchAndNeedToReturn(v, event, index, vc, deltaX)) // +5
                                            return false;
                                    }catch(Exception e){ //+6
                                        setTouchProgressIndex(-1);
                                    }finally{
                                        setTouchProgressIndex(-1);
                                    }
                                }

                                return false;
                            }

                        });

            } catch (Exception e) { //+7
                Log.e("Testing","Exception "+ e);
            }
        }
    }

Why I am getting this issue. Please help me on this.


Solution

  • I agree with SonarQube that the code is overly complex.

    Simplifications possible:

    • combine if statements
    • use a lambda
    • (not done here) use an extra method for the lambda code

    So:

    private void LayoutTouch(int touchType, int index) {
    
        if (touchType != -1) { //+1
            try {
                ViewConfiguration vc = ViewConfiguration.get(ctContext);
                mSlop = vc.getScaledTouchSlop();
                parentLayout[index]
                        .setOnTouchListener((v, event) -> {
                                if (isValidEvent() && checkTouchIndex(index)) {
                                    try{
                                        
                                        // Code here
    
                                        if (animationStarted) { //+4
                                            return false;
                                        }
    
                                        final ViewConfiguration vc = ViewConfiguration
                                                .get(getContext());
                                        final int deltaX = (int) (event.getX() + 0.5f)
                                                - mGestureCurrentX;
    
    
                                        initiateVelocityTracker();
                                        mVelocityTracker.addMovement(event);
    
                                        mVelocityTracker
                                                .computeCurrentVelocity(1000);
    
                                        if (!doSwitchAndNeedToReturn(v, event, index, vc, deltaX)) // +5
                                            return false;
                                    } catch(Exception e) { //+6
                                        setTouchProgressIndex(-1);
                                    } finally {
                                        setTouchProgressIndex(-1);
                                    }
                                }
    
                                return false;
                            });
    
            } catch (Exception e) { //+7
                Log.e("Testing","Exception "+ e);
            }
        }
    }
    

    The extra method:

                        .setOnTouchListener(this::onTouch);
    
    private boolean onTouch(View v,  MotionEvent event) {
        ...
    }
    

    The checked exception handling is very unspecific. If not a specific exception can happen, maybe drop it (at the end).

    Using member variables named with the prefix m, is not conventional in java. These variables indeed seem many, but with mouse, touch and so that might make sense. I mention this, as the calculations seem refactorable.