1

This question is a bit confusing so I will try to explain through code and example.

How can I decrement a counter if it meets a condition? Sounds simple right? But what if that method continuously gets evoked from within a for loop so that values passed to it, get reset?

Eample: I have a document with several pages, however some of those pages are blank and I don't want to count them.

For a 5 page document with the first page being blank. I want to accomplish the following behavior:

page 1: nothing...
page 2: 1 of 4
page 3: 2 of 4
page 4: 3 of 4
page 5: 4 of 4

Currently it's including the blank page as part of the page count.

Here is a code sample:

static main() {
    // et cetera
    for(Page page : pages) {
        int pageNumber = page.getPageNo();
        int pageCount = page.getTotalPageCount();
        form.getTemplate().writePageNumber(page, pageNumber,pageCount);
    }
}

So in the main routine I'm calling a method called writePageNumber() from within a for loop where I pass in currentPageNumber and totalPage count.

public boolean writePageNumber(Page page, int currentPageNo, int totalPageCount) {
    if ( !page.isBlank() ) {
        this.write(currentPageNo, totalPageCount);
    }
    else if ( page.isBlank() ){
        currentPageNo--;
        totalPageCount--;
    }

    // et cetera
}

Now here in the writePageNumber() method, I get page context and I process. I check to see if the page is not blank then I write the currentPage of totalPages, if it is blank then I decrement the currentPage No and totalPage count...so as to not count the blank page...is this the right way to do it? Not sure

Since the method is called from within a for loop, the values after I decrement them get reset, how to maintain decremented values?

There probably is a better way to do this, any help would be appreciated.

Edit:

Just for clarity, the problem is that writePageNumber() is called from a for loop so the next time writePageNumber is called, the values are reset, so during the second iteration the else if condition is no longer true and the first condition is true so the wrong page number is written

arabian_albert
  • 708
  • 1
  • 11
  • 24
  • Sorry, but doesn't understand where is the problem... anyway if you need to return two values you can wrap it with class – AsfK Apr 17 '18 at 04:50
  • try this: if ( !page.isBlank() ) { this.write(page.getPageNo(), page.getTotalPageCount()); } else if ( page.isBlank() ){ page.getPageNo()--; page.getTotalPageCount()--; } – dkb Apr 17 '18 at 04:50
  • @AsfK I edited my question and added detail, hopefully it should be clearer now – arabian_albert Apr 17 '18 at 04:51
  • @dkb problem is that is called from a for loop so the next time writePageNumber is called, the values are reset, so during the second iteration the else if condition is no longer true and the first condition is true so the wrong page number is written – arabian_albert Apr 17 '18 at 04:52
  • @arabian_albert, The values are reset because you declare the variables in the loop, define them before the loop! – AsfK Apr 17 '18 at 04:55
  • @AsfK yes it's due to the loop, that is the problem – arabian_albert Apr 17 '18 at 04:59
  • In your code sample, where is `writePageNumber` being called? That would be a relevant piece to include in the question. – Martin Tuskevicius Apr 17 '18 at 05:02
  • 1
    In your updated version, the arguments to the method call don't match its parameters. – Martin Tuskevicius Apr 17 '18 at 05:05
  • @MartinTuskevicius nice catch. I corrected it. `writePageNumber` should be Called from within the for loop – arabian_albert Apr 17 '18 at 05:05

2 Answers2

2

The problem is that in your current code, the variables currentPageNo and totalPageCount are on the stack, which means that their values get thrown away after the method returns. One way to solve your problem would be to extract the body of writePageNumber into your for loop. So rather than having something like,

Page page = /* ... */
int totalPages = 5;
for (int currentPage = 1; currentPage <= totalPages; i++) {
    writePageNumber(page, currentPage, totalPages);
};

Your code could look like:

Page page = /* ... */
int totalPages = 5;
for (int currentPage = 1; currentPage <= totalPages; i++) {
    if (!page.isBlank()) {
       // ...
    } else {
      currentPage--;
      totalPages--;
    }
    // ...
}

Edit: You mentioned not breaking code patterns, but you may have to compromise in this case. Currently, you are expecting writePageNumber to be a function that takes two integer arguments, modifies them, and returns their updated values for the next function call somehow. You can't return multiple values from a function in Java. You need to have those values persist across method calls, so they need to be declared outside of the method's scope (or, like I suggested, you extract the method into the loop), which means using a field.

Martin Tuskevicius
  • 2,590
  • 4
  • 29
  • 46
  • Thanks. However I'd like to maintain my current pattern as this code I'm working with is very legacy, I fear breaking something... +1 for the nice idea – arabian_albert Apr 17 '18 at 05:02
  • writePageNumber is located in a different class. The caller should not know about implementation of how that class implements writing the page no. The sample I have given is a simple example to illustrate the problem. I think breaking a design pattern for fixing a bug is not good practice and breaks the fundamental computer science principle of separation of concerns – arabian_albert Apr 17 '18 at 18:41
1

Here's what I believe the problem is:
You have a number (5) page objects; every object has its own currentPage and totalPageCount as follows:
Object 1: Page: current = 1; total = 5;
Object 2: Page: current = 2; total = 5;
Object 3: Page: current = 3; total = 5;
Object 4: Page: current = 4; total = 5;
Object 5: Page: current = 5; total = 5;

In your code, you are updating current & total (not in the object but only locally in the context they're being used), but these values are not updated in the remaining Objects; the for loop is looping over these un-updated remaining objects.
I suggest you do something like this:

private int blanks = 0;
public boolean writePageNumber(Page page, int currentPageNo, int totalPageCount) {
    if ( page.isBlank() ) {
        blanks++;
        // etcetera
    } else {
        this.write(currentPageNo - blanks, totalPageCount - blanks);
    }

    // et cetera
}

Edit

You should first iterate over all pages to get total blank pages, in order to get a fixed totalPageCount, then remove the - blanks in the totalPageCount - blanks above, orelse you'll get data like this:

Object 1: Page: current = 1; total = 5;
Object 2: Page: current = BLANK; total = 5;
Object 3: Page: current = 2; total = 4;
Object 4: Page: current = BLANK; total = 4;
Object 5: Page: current = 3; total = 3;

Notice how the total keeps changing.
One more thing to note is that totalPageCount is not a property of a page and thus should not belong to the Page object, but rather to the book object. See how it's complicating things?!

Ahmad Tn
  • 360
  • 3
  • 16
  • if you want to update the `page` Object, I suggest defining setter methods, like `setPageNo(int no)` and `setTotalPageCount(int count)` – Ahmad Tn Apr 17 '18 at 14:13
  • 1
    I like your subtract blanks idea however it might print the wrong page no if say there were a blank in the middle of the document stream. then `blank` would increment to 2 and then `this.write(currentPageNo - 2, totalPageCount - 2)` and also the totalPageCount might be different as blanks gets incremented when it encounters a blank – arabian_albert Apr 17 '18 at 16:57
  • Also it looks like the value of blanks won't persist after the method `writePageNumber` returns to caller, so in the next call `blanks` will be `0` again – arabian_albert Apr 17 '18 at 18:40
  • As for your 1st point, I agree, which is why I edited my answer. Even your code will have the same issue I believe. To overcome this, you'll have to iterate over your pages once to count all blank pages first. – Ahmad Tn Apr 18 '18 at 18:11
  • If a single object is created from the class, in which the method `writePageNumber` is defined, and re-used in your loop then `blanks` should not lose its value. I can help you better if I can see both classes: a github repo maybe? A workaround (that works, but may be logically wrong depending on you classes structure) is to make `blanks` static like this `static int blanks = 0` – Ahmad Tn Apr 18 '18 at 18:15