views:

62

answers:

2

Is this bad practice and should I be shot for coming up with this code?

function get_business_addresses($business_id) {

    $query = $this->db->get_where('contact_business_addr_rel', array('business_id'=> $business_id));

    if ($query->num_rows() > 0) {
        foreach ($query->result() as $row) {
            $address_id = $row->address_id;
            $address_type_id = $row->address_type_id;
            $this->db->select('type');
            $q = $this->db->get_where('contact_business_address_type',array('id'=> $address_type_id));

            $query = $this->db->get_where('contact_business_addresses',array('id'=> $address_id)); 

            if ($query->num_rows() > 0) {
                foreach ($query->result() as $row) {

                    $row2 = $q->row();
                    $obj_merged = (object) array_merge((array) $row, (array) $row2);
                    $data[] = $obj_merged;
                }

            }
        }
    }
    return $data;
}
+6  A: 

Probably. I can't say for certain, but that looks like it should be significantly slower than just using a JOIN.

I would say benchmark it to be sure, but the rule of thumb I go by is "if it can be done cleanly in the SQL, the DB engine can probably do it better than I can".

zebediah49
+1  A: 

In short, yes this is bad. For a few rows (e.g. 10) this doesn't make much of a difference. However, as soon as your table happens to have 10 000 rows it means it first has to fetch 10 000 rows, then loop through each row and merge all the stuff. A simple JOIN in this case will be much faster.

Yorick Peterse