If statement in for loop not filtering out items

Question

 // @param physicalAddress - the actual address of the home a host wants to list (not the ethereum address)
    // @return _id - list of ids for homes
    function listHomesByAddress(string _physicalAddress) public returns(uint [] _id ) {
        uint [] results;
        for(uint i = 0 ; i<homes.length; i++) {
            if(keccak256(homes[i].physicalAddress) == keccak256(_physicalAddress) && homes[i].available == true) {
                results.push(homes[i].id);
            }
        }
        return results;

    }

The result is supposed to be a list of ids which match the physical address entered however it does not filter through but returns all the available homes. When I change to using String utils nothing changes.

Here is the whole code:

pragma solidity ^0.4.0;

import "browser/StringUtils.sol";

// @title HomeListing

contract HomeListing {

struct Home {
    uint id;
    string physicalAddress;
    bool available;
}
Home[] public homes;
mapping (address => Home) hostToHome;
event HomeEvent(uint _id);
event Test(uint length);
constructor() {

}

// @param physicalAddress - the actual address of the home a host wants to list (not the ethereum address)
function addHome(string _physicalAddress) public {
    uint _id = uint(keccak256(_physicalAddress, msg.sender));
    homes.push(Home(_id, _physicalAddress, true));
}

// @param physicalAddress - the actual address of the home a host wants to list (not the ethereum address)
// @return _id - list of ids for homes
function listHomesByAddress(string _physicalAddress) public returns(uint [] _id ) {
    uint [] results;
    for(uint i = 0 ; i<homes.length; i++) {
        string location = homes[i].physicalAddress;
        if(StringUtils.equal(location,_physicalAddress )) {
            results.push(homes[i].id);
        }
    }
    return results;

    }
}

Answer 1

Your issue with the listHomesByAddress function is that you're attempting to instantiate a dynamic array in memory, which is impossible.

Since you don't know ahead of time how large an array you'll need, you'll either need to count it first (which means looping twice which is wasteful & expensive in terms of gas) or use an array in storage. When using an array in storage however, it costs gas to clear it afterwards, which is also wasteful.

You could instead use a public storage array to copy IDs into, counting the number of entries found as it progresses. You can then use this counter as the index of the last relevant entry in the results array. Now we know the length of the array in memory we need, we can go ahead and create the final array with the IDs in via a getArray function:

uint[] public results;

function listHomesByAddress(string _physicalAddress) public returns(uint) {
    uint counter = 0;
    for(uint i = 0; i < homes.length; i++) {
        string location = homes[i].physicalAddress;
        if(StringUtils.equal(location, _physicalAddress)) {
            counter++;
            results[i] = homes[i].id;
        }
    }
    return getArray(counter);
}

function getArray(uint _length) public returns (uint[]) {
    uint[] memory arr = new uint[](_length);
    for (uint i = 0; i < _length; i++) {
        arr[i] = results[i];
    }
    return arr;
}

If the array is not required for a smart-contract, and instead is for a front-end DApp, you can simply return the counter and the front end can use that to loop over the results array to gather the correct IDs instead.