Reduce number of for loops in method

Question

I'm trying to reduce the number of for loops in this method, but I'm not sure how to do that while still keeping the logic in tact and not moving to another method. I want to do this for the sake of reducing the McCabe Cycolmatic Complexity, which increases with each loop by 1. So I'd like to reduce the method by 1.

  private void method(int page)
    {
        for (int i = 0; i < LINES_PER_PAGE; i++)
        {
            nextLine[i] = null;
        }
        try
        {
            Scanner temp = new Scanner(fileToPrint);
            for (int i = 0; i < page - 1; i++)
            {
                skipAPage(temp);
            }
            for (int i = 0; (i < LINES_PER_PAGE) && (temp.hasNext()); i++)
            {
                nextLine[i] = expandTabs(temp.nextLine());
            }
        } catch (FileNotFoundException e)
        {
            e.printStackTrace();
        }
    }

There is no reason to refactor this code, it's as readable as it gets. But I would advise always leaving white space before and after any conditional or loop statements, it may add +2 more lines, but it is lot easier on the eyes. — The Law, Oct 07 '15 at 19:11
Why can't you move part of the code to other method? You could at least extract first part into some `clearLines(nextLine, LINES_PER_PAGE)` which is quite logical thing to do in this context. — bezmax, Oct 07 '15 at 19:36

YoungHobbit · Accepted Answer · 2015-10-07T20:21:32.583

4

    for (int i = 0; i < LINES_PER_PAGE; i++) {
        nextLine[i] = null;
    }

This is completely unnecessary, because default value for any object is null in java.

In the rest of two loops, you are calling two different methods and your second loop also depends on the argument, so I am not sure they can be merged into one loop.

As it appears that nextLine is declared outside the method, I would suggest you to use List<Sometype> and add the elements into it, coming from expandTabs(temp.nextLine());, every time you enter the method clear the list list.clear().

Try this code. Use the appropriate type for declaring the List<String>. You can access the element from the List using myList.get(index) method.

List<String> myList = new ArrayList<>();

private void method(int page) {
  try {
    myList.clear();            // Clear the list.
    Scanner temp = new Scanner(fileToPrint);
    for (int i = 0; i < page - 1; i++) {
      skipAPage(temp);
    }
    for (int i = 0; (i < LINES_PER_PAGE) && (temp.hasNext()); i++) {
      myList.add(expandTabs(temp.nextLine()));    // Add the elements to the list.
    }
  } catch (FileNotFoundException e) {
    e.printStackTrace();
  }
}

edited Oct 07 '15 at 20:21

answered Oct 07 '15 at 19:10

YoungHobbit

13,254
9
50
73

This isn't true - nextLine is declared outside the method, and could therefore be populated before this method. The for loop isn't guaranteed to fully populate nextLine, and therefore nulling out the elements is reasonable. – Sbodd Oct 07 '15 at 19:24
@Sbodd your observation seems to be correct. I would suggest to use List in that case, instead of an array. Then he won't need to loop. – YoungHobbit Oct 07 '15 at 19:36
@YoungHobbit could you show me what this would look like for this method? I haven't worked with list. not sure how to clear the lines with something like this: `List myList = new ArrayList<>(Arrays.asList( expandTabs(temp.nextLine()) ));` – Carlo Otto Oct 07 '15 at 20:09
@CarloOtto what is return type of `expandTabs(temp.nextLine())` and type of `nextLine[]`. – YoungHobbit Oct 07 '15 at 20:11
@YoungHobbit both are String. (expandTabs is `return temp.toString();` – Carlo Otto Oct 07 '15 at 20:21
@CarloOtto Please check the answer. – YoungHobbit Oct 07 '15 at 20:22

score 0 · Answer 2 · answered Oct 07 '15 at 19:16

I recommend to skip the skipAPage step as I assume another loop in there:

        Scanner temp = new Scanner(fileToPrint);
        int linesToSkip = page * LINES_PER_PAGE;
        int i = 0;
        while ( temp.hasNext() ) {

             if ( i >= linesToSkip && i < linesToSkip  + LINES_PER_PAGE)
                  nextLine[i] = expandTabs(temp.nextLine());
             else
                  temp.nextLine();
             i++;
        }

score 0 · Answer 3 · answered Oct 07 '15 at 19:28

0

You could replace

for (int i = 0; i < LINES_PER_PAGE; i++)
    {
        nextLine[i] = null;
    }

with a call to Arrays.fill.

answered Oct 07 '15 at 19:28

Sbodd

11,279
6
41
42

Or just skip it entirely -- by default, an array of `Object`s is initialized to contain nothing but `null`. – Nic Oct 07 '15 at 21:02
1

@QPaysTaxes what if `method()` were to be called twice? – Sbodd Oct 08 '15 at 21:00

Reduce number of for loops in method

3 Answers3