What's where?

sebastian.galczynski

Well, the author of this code certainly didn't know what's WHERE. Or JOIN.
The following code is actually from a production system managing conference rooms in some office building in Poland. In case you wonder: No, old reservations were not automatically deleted. Also this code probably is still there, because that client didn't pay for any upgrades. I hope he learned how to delete them by hand.

    /**
     * Checks for currently reserved rooms
     * @param Request $request
     * @return \Symfony\Component\HttpFoundation\JsonResponse
     */
    public function checkCurrentReservedAjaxAction(Request $request)
    {
        $em = $this->getDoctrine()->getManager();

        /*
         * rooms
         */
        $rooms = $this->getDoctrine()->getRepository('ReservationsSecuredBundle:Rooms')->findAll(array(), array('name' => 'ASC'))->getQuery()->getResult();

        /*
         * status
         */
        $status = $this->getDoctrine()->getRepository('ReservationsSecuredBundle:StatusReservation')->findAll();

        /*
         * reservations
         */
        $reservations = $this->getDoctrine()->getRepository('ReservationsSecuredBundle:Reservations')->findAll();

        /*
         * reservations in rooms
         */

        $currDate = new \DateTime('now');

        $currentlyReserved = array();
        foreach ($rooms as $key => $room)
        {
            foreach ($reservations as $kr => $reservation)
            {
                foreach ($reservation->getRoom() as $reservationRoom)
                {
                    if ($room == $reservationRoom->getRoom())
                    {
                        if ($reservation->getFrom() < $currDate && $reservation->getTo() > $currDate && $reservation->getIdStatusReservation()->getId() == 2)
                        {
                            $currentlyReserved[$room->getId()] = true;
                        }
                    }
                }
            }
        }

        return new \Symfony\Component\HttpFoundation\JsonResponse(array('currentlyReserved' => $currentlyReserved));
    }

Zenith

@sebastian-galczynski What's with the backslashes in the class names?

Gąska

@sebastian-galczynski said in What's where?:

What's where?

What's on second.

HardwareGeek

@Gąska Who?

sebastian.galczynski

@Zenith said in What's where?:

What's with the backslashes in the class names?

PHP namespace separators? If you ask why didn't he import these classes at the top of the file - I have no idea. Maybe he didn't know how.

loopback0

@HardwareGeek said in What's where?:

@Gąska Who?

Who's playing first.

TheCPUWizard

If an ORM is in use, those MAY be dealing with that and the underlying SQL being generated... Did not see enough code in that snipped to be 100% sure.

dkf

@TheCPUWizard said in What's where?:

If an ORM is in use, those MAY be dealing with that and the underlying SQL being generated... Did not see enough code in that snipped to be 100% sure.

In theory yes. But those three nested foreachs don't make me at all hopeful. It looks a lot more like someone who isn't hoisting the query into the DB at all…

BernieTheBernie

@sebastian-galczynski said in What's where?:

$reservation->getIdStatusReservation()->getId() == 2

That's really number 2.

TheCPUWizard

@dkf said in What's where?:

@TheCPUWizard said in What's where?:

If an ORM is in use, those MAY be dealing with that and the underlying SQL being generated... Did not see enough code in that snipped to be 100% sure.

In theory yes. But those three nested foreachs don't make me at all hopeful. It looks a lot more like someone who isn't hoisting the query into the DB at all…

Not sure.... If I have

class Master
{
   int Id;
   public List<Slave> Slaves { get; set; }
}

and I have a lazy loading ORM... Then

foreach (var slave in Masters[42].Slaves)
{
}

Will generate and execute something like (I am typing raw, so not real SQL)

Select * from Slaves Join Master where Master.Id = 42

But you would have no indication by looking at the code.

Zenith

@sebastian-galczynski That was a dumb question from me. I write my PHP without (official) namespaces. I guess the backslashes make sense if you think of namespaces like directories.

sebastian.galczynski

@Zenith said in What's where?:

I guess the backslashes make sense if you think of namespaces like directories.

They debated for a long time, eventually settling for \. Some proposals were even weirder.

sebastian.galczynski

@TheCPUWizard

Yes, there is lazy loading. If he did $room->getReservations() or so it would be even slower, because of network latency / forking threads in the db etc. The point is that current reservations are a very small subset of all reservations, so you should just do

SELECT room.id FROM room JOIN reservation_room
JOIN reservation WHERE reservation.from <= :now AND reservation.to >= :now.

preferably after putting an index on the datetime columns. The query planner will deal with it. And the ORM in question lets you do exactly this (it has it's own query language very similar to SQL) in a one-liner.

TheCPUWizard

@sebastian-galczynski

Using JUST THE CODE POSTED.... please prove to me that (and other similar)

$status = $this->getDoctrine()->getRepository('ReservationsSecuredBundle:StatusReservation')->findAll();

Actually goes to the database at all. IT could return something like an IQueryable which simple captures the initial information.

While it is highly improbable that what I am saying is actually the case, I am confident that with sufficient motivation (say $100K USD) I would write supporting code so that those exact lines actually performed highly optimized Db operations... Perhaps not even hitting the database at all until the last line were actual data is needed for the Json response.

dkf

@TheCPUWizard said in What's where?:

with sufficient motivation (say $100K USD)

You'd use PHP for that little?

sebastian.galczynski

@TheCPUWizard said in What's where?:

While it is highly improbable that what I am saying is actually the case, I am confident that with sufficient motivation (say $100K USD) I would write supporting code so that those exact lines actually performed highly optimized Db operations

What it effectively means is transpiling arbitrary PHP (or some other procedural-functional language) code into SQL. I wouldn't bet on $100k as a price for that.

TimeBandit

@Zenith said in What's where?:

I guess the backslashes make sense if you think of namespaces like directories.

only in the Windows world

Tsaukpaetra

@sebastian-galczynski said in What's where?:

@Zenith said in What's where?:

I guess the backslashes make sense if you think of namespaces like directories.

They debated for a long time, eventually settling for \. Some proposals were even weirder.

I'm partial to the smileys.

Gąska

@Tsaukpaetra you're partial to many weird things that serve no useful purpose.

Zecc

I have expressed my opinion previously.

@Zecc said in The nerdy jokes thread (bonus original title mode!):

@ben_lubar said in The nerdy jokes thread (bonus original title mode!):

https://twitter.com/willkirkby/status/1028682822956007427

If I ever make an OS....

Tsaukpaetra

@Gąska said in What's where?:

@Tsaukpaetra you're partial to many weird things that serve no useful purpose.

I'm trying to become more human!

Gąska

@Tsaukpaetra on a programming forum?

Tsaukpaetra

@Gąska said in What's where?:

@Tsaukpaetra on a programming forum?

.... Don't judge me!

Applied Mediocrity

@Gąska said in What's where?:

@Tsaukpaetra on a programming forum?

A what now?