Find qualifications that are expiring [closed]

Question

I have the following function that returns StaffQualifications where either the qualification is expiring or the qualification type is expiring, but this section doesn't seem to sit well with me as it splits up the SQL string... Should this be two separate functions perhaps even though 95% of the query is the same?

    if ($qualification instanceof Qualification) {
        $sql .= "AND q.id = :qualificationId ";
    } elseif ($qualificationType instanceof QualificationType) {
        $sql .= "AND q.qualification_type_id = :qualificationTypeId ";
    }

The above doesn't quite feel right, but not sure how to better it.

/**
 * @return StaffQualification[]
 */
public function getStaffQualificationsExpiringOnQualificationOrQualificationType(?Qualification $qualification, ?QualificationType $qualificationType): array
{
    if ($qualification === null && $qualificationType === null) {
        throw new \Exception('Qualification or QualificationType must not be null');
    }
    if ($qualification instanceof Qualification && $qualificationType instanceof QualificationType) {
        throw new \Exception('Only one of Qualification or QualificationType can be present');
    }

    $sql = "
    SELECT *
    FROM staff_qualification AS sq ";
// Complicated SQL ensues that I've omitted from this SE question...

    if ($qualification instanceof Qualification) {
        $sql .= "AND q.id = :qualificationId ";
    } elseif ($qualificationType instanceof QualificationType) {
        $sql .= "AND q.qualification_type_id = :qualificationTypeId ";
    }

    $sql .= "
    GROUP BY sq.id
    ORDER BY sq.end_date
    ;
    ";

    $rsm = new ResultSetMapping();
    $rsm->addEntityResult(StaffQualification::class, 'sq');
    $rsm->addJoinedEntityResult(Staff::class, 's', 'sq', 'staff');
    $rsm->addFieldResult('sq', 'id', 'id');
    $rsm->addFieldResult('s', 'staff_id', 'id');
    $query = $this->getEntityManager()->createNativeQuery($sql, $rsm);
    if ($qualification instanceof Qualification) {
        $query->setParameter('qualificationId', $qualification->getId());
    } elseif ($qualificationType instanceof QualificationType) {
        $query->setParameter('qualificationTypeId', $qualificationType->getId());
    }
    $result = $query->getResult();
    return $result;
}

Answer 1

I think the best thing to do would be to extract the logic outside of the string building and pass it in. You could do this either inline as per below or with sprintf or Heredoc.

/**
 * @param ?Qualification $qualification
 * @param ?QualificationType $qualificationType
 * @return StaffQualification[]
 * @throws Exception
 */
public function getStaffQualificationsExpiringOnQualificationOrQualificationType(
    ?Qualification $qualification,
    ?QualificationType $qualificationType
): array
{
    if ($qualification === null && $qualificationType === null) {
        throw new Exception('Qualification or QualificationType must not be null');
    }
    if ($qualification instanceof Qualification && $qualificationType instanceof QualificationType) {
        throw new Exception('Only one of Qualification or QualificationType can be present');
    }

    if ($qualification instanceof Qualification) {
        $filterSql = "q.id = :qualificationId ";
    } elseif ($qualificationType instanceof QualificationType) {
        $filterSql = "q.qualification_type_id = :qualificationTypeId ";
    }
    $sql = "
    SELECT *
    FROM staff_qualification AS sq
    WHERE 1=1
    AND $filterSql
    GROUP BY sq.id
    ORDER BY sq.end_date
    ;
    ";

    $rsm = new ResultSetMapping();
    $rsm->addEntityResult(StaffQualification::class, 'sq');
    $rsm->addJoinedEntityResult(Staff::class, 's', 'sq', 'staff');
    $rsm->addFieldResult('sq', 'id', 'id');
    $rsm->addFieldResult('s', 'staff_id', 'id');
    $query = $this->getEntityManager()->createNativeQuery($sql, $rsm);
    if ($qualification instanceof Qualification) {
        $query->setParameter('qualificationId', $qualification->getId());
    } elseif ($qualificationType instanceof QualificationType) {
        $query->setParameter('qualificationTypeId', $qualificationType->getId());
    }
    $result = $query->getResult();
    return $result;
}