1
\$\begingroup\$

I have a byte array and a List of a custom object ByteRule and I'm needing to loop around and perform an operation on both but only as long as one of both is available, the original method is as follows:

private final List<ByteRule> rules;

@Override
public boolean match(final InetAddress addr)
{
    final byte[] bytes = addr.getAddress();
    for(int i = 0; i < this.rules.size() && i < bytes.length; i++)
    {
        final int value = bytes[i] < 0 ? (int) bytes[i] + 256 : bytes[i];
        if(!this.rules.get(i).match(value))
        {
            return false;
        }
    }
    return true;
}

Obviously the this.rules.get(i) on every iteration is inefficient, however as I need to maintain a counter, an enhanced for loop means keeping a separate counter and performing a check on it every time like so (list declaration skipped for space):

@Override
public boolean match(final InetAddress addr)
{
    final byte[] bytes = addr.getAddress();
    int counter = 0;
    for(final ByteRule rule : this.rules)
    {
        if(counter + 1 == bytes.length) break; //In case there aren't enough bytes
        final int value = bytes[counter] < 0 ? (int) bytes[counter] + 256 : bytes[counter];
        if(!rule.match(value))
        {
            return false;
        }
        counter++;
    }
    return true;
}

Is there a cleaner/more efficient solution to this problem that I'm missing?

\$\endgroup\$
2
  • \$\begingroup\$ Obviously the this.rules.get(i) on every iteration is inefficient That is not obvious to me at all. \$\endgroup\$ Jul 24 '13 at 6:36
  • 1
    \$\begingroup\$ This is how you can get rid of the ternary expression. Bitwise AND it with 0xFF. \$\endgroup\$ Jul 24 '13 at 6:44
2
\$\begingroup\$

I would be happy with:

@Override
public boolean match(final InetAddress inetAddress)
{
    final byte[] bytes = inetAddress.getAddress();
    int end = Math.min(this.rules.size(), bytes.length);
    for(int i = 0; i < end; i++)
    {
        final int adressPart = bytes[i] & 0xFF; //byte range from -128 to 127, &0xFF makes it unsigned, therefore positive
        if(!this.rules.get(i).match(adressPart))
            return false;
    }
    return true;
}

If you think, the this.rules.get(i), then test it with a profiler. I would be surprised if this line is the reason for any slowdown, because the length of a InetAddress is probably quite limited and most likely implemented as an ArrayList annyway. If it really matters, you can change it to:

@Override
public boolean match(final InetAddress inetAddress)
{
    final byte[] bytes = inetAddress.getAddress();
    ByteRule[] byteRulesArray = rules.toArray(new String[rules.size()]);
    int end = Math.min(this.rules.size(), bytes.length);
    for(int i = 0; i < end; i++)
    {
        final int adressPart = bytes[i] & 0xFF; //byte range from -128 to 127, &0xFF makes it unsigned, therefore positive
        if(!byteRulesArray[i].match(adressPart))
            return false;
    }
    return true;
}

or if you even fear the array creation, you can use an iterator:

@Override
public boolean match(final InetAddress inetAddress)
{
    final byte[] bytes = inetAddress.getAddress();
    Iterator<ByteRule> rulesIterator = rules.iterator();
    int end = Math.min(this.rules.size(), bytes.length);
    for(int i = 0; i < end; i++)
    {
        final int adressPart = bytes[i] & 0xFF; //byte range from -128 to 127, &0xFF makes it unsigned, therefore positive
        if(!rulesIterator.next.match(adressPart))
            return false;
    }
    return true;
}

Hint: In theory, the iterator solution would be faster than the array creation. However in the real world, because of hardware reasons, the array creation will, most likely, be faster.

\$\endgroup\$
1
  • \$\begingroup\$ I love the third solution, thank you very much for your help. \$\endgroup\$
    – MrLore
    Jul 24 '13 at 20:35

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.