How to minimize code duplication in sql query?

Viewed 513

I am working on the SQL query code in which I want to avoid the sql query duplication.

Below is the sql query code:

    switch ($y) {
        case 'l.text':
            $query->order('numeric_text ' . (strtolower($x) == 'DESC' ? 'DESC' : 'ASC') .
                ', ' . $y . ' ' . (strtolower($x) == 'DESC' ? 'DESC' : 'ASC'));
            break;
    }

In the above SQL Query code strtolower($x) == 'DESC' ? 'DESC' : 'ASC' is being used at two places. I am thinking to place a varibale there instead.

This is what I have tried:

    $sortOrder = (strtolower($x) == 'DESC' ? 'DESC' : 'ASC');

    switch ($y) {
        case 'l.text':
            $query->order('numeric_text ' . $sortOrder . ', ' . $y . ' ' . $sortOrder);
            break;
    }



Problem Statement:

I am wondering if there is any other better way, we can avoid sql query duplication.

4 Answers

Maybe you can use a function in your model or in helper library to do that job for you, and use it around your project, it could be a good idea to DRY your code.

// Helper

function sort_statment($fields, $sort_order) {
  $sort_statment = '';
  foreach($fields as $field){
     $sort_statment += $field . ' ' . $sort_order 
  }
  return $sort_statment;
}


// Model 
$sort_fields ['numeric_text', 'other_field', $y ....];

switch ($y) {
    case 'l.text':
        $query->order(sort_statment($sort_fields, $sort_order);
        break;
}

Like Nick mentionned in the comments, you can't lower a string and have it equal to 'DESC' or 'ASC', assuming it's just a typo you can just overwrite your variable instead of declaring a new one :

    if(strtoupper($x) != 'DESC') $x = 'ASC';

switch ($y) {
    case 'l.text':
        $query->order('numeric_text ' . strtoupper($x) . ', ' . $y . ' ' . strtoupper($x));
        break;
}

It would also help to know what values $x can take.

if you want to minimize code duplication you can separate code by three parts: determination of sort order, determination of column name and making of a query. For example:

// determination of sort order
$sortOrder = (strtoupper($x) == 'DESC' ? 'DESC' : 'ASC');


// determination of column name
switch ($y) {
    case 'l.text':           
        $column = 'numeric_text';
        break;
}

// making of a query
$query->order($column . $sortOrder . ', ' . $y . ' ' . $sortOrder);

Of course, you can create functions or methods (if you use OOP) with those parts of the code. For example:

/**
 * determination of sort order
 */
function getSortOrder($x)
{
    return strtoupper($x) == 'DESC' ? 'DESC' : 'ASC';
}

/**
 * determination of column name
 */
function getColumn($y)
{
    switch ($y) {
        case 'l.text':           
            return 'numeric_text';
            break;
    }
}

/**
 * making of a query
 */
function addOrder($query, $x, $y) 
{
    $sortOrder = getSortOrder($x);
    $column = getColumn(y);
    $query->order($column . $sortOrder . ', ' . $y . ' ' . $sortOrder);
}

Also, if you have many columns, you can use mapping instead of case statement. It will make your code more comfortable to read. For example:

// determination of column name
$map = [
   'l.text' => 'numeric_text'
];
$column = key_exists($y, $map) ? $map[$y] : 'default_column'

Why don't you use shorthand of ternary operator shorthand

<?php
$sortOrder = $sortOrder ?: 'ASC';

switch ($y) {
    case 'l.text':
        $query->order("numeric_text {$sortOrder},{$y} {$sortOrder}");
        break;
}
Related