Issue #2617 add lastIndexOf(byte[] array, byte[] target)

[fdesu]

Hi, I hope this issue still makes sense. I wanted to attempt to add the lastIndexOf method to the Primitive's utility classes to fix #2617.

To try it out I've started with simple implementation of Bytes#lastIndexOf to gather initial feedback. I want to implement lastIndexOf for other classes in the same manner. Do you think it is ok or I need to consider any other approach/put some annotations/etc. ?

[xindoo]

    for (int i = array.length - target.length; i >= 0; i--) {
        int j = 0;
        while (j < target.length && array[i + j] == target[j]) j++; 
        if (j == target.length) 
            return i;
    }

it's clearer

[fdesu]

@xindoo thanks for taking a look.
Do you think it is really easier to read? For me double for is kind of easier/clearer.

[xindoo]

labeled statement is easy to make me confusing, and I think it’s same to some of the others

[fdesu]

@xindoo yep, but problem with the label might be solved with simple break statement instead of continue to a label, right? I've sticked to the way how other methods were implemented, so I wanted to be consistent in regards with the labels.

[Maaartinus]

@fdesu No, breaking out would hit return i;. You could do

    for (int i = array.length - target.length; i >= 0; i--) {
      for (int j = 0; ; j++) {
        if (j == target.length) {
          return i;
        } else if (array[i + j] != target[j]) {
          break;
        }
      }
    }
    return -1;

but I don't claim it's nicer.

[fdesu]

I'm quite flexible in this regard and don't have strong preferences, but e.g. in java.lang.String.lastIndexOf(char[],int,int,char[],int,int) they do

startSearchForLastChar:
        while (true) {
            ...
            while (j > start) {
                ...
                    continue startSearchForLastChar;
                ...
            }
            ...
        }
    }

and it's pretty clear, so I think marks are fine when they are properly named.

What do you @Maaartinus @xindoo think?

[Maaartinus]

IMHO the last occurrence of an empty string starts at array.length. AFAIK that's how String.lastIndexOf works and also what you get when you leave this test out.

[fdesu]

Yep, you're right, in this case array.target should be a return value

[szarnekow]

I think you meant array.length, didn't you?

[fdesu]

@szarnekow shouldn't have published the comment in hurry, yes, array.length

[fdesu]

I've rebased this branch against latest master