Parse a clean slug URL

Question

I have to implement a "user-friendly" URL for an employment website. The URL /jobs{parameters?} may be composed of up to three searching criteria separated by hyphens (i.e. -); each criterion is separated by a tilde (i.e. ~). The parameters string has the following structure: -keywords-employmentTypes-locations. keywords and employmentTypes are optional. location is mandatory unless there are no parameters, the route should handle all these cases:

/jobs
/jobs-canada /jobs-canada~paris~usa
/jobs-full_time-canada /jobs-full_time~part_time-canada~paris
/jobs-engineer-canada /jobs-engineer~c++-canada~paris
/jobs-engineer-full_time-canada /jobs-engineer~c++-full_time-canada~paris

The following class parses the string parameters and fill all criteria in variables:

class JobUrlBuilder
{
 const DEFAULT_LOCATION = 'france';
 public $keywords;
 public $employmentTypes;
 public $locations;
 public $allEmploymentTypes;
 function __construct($allEmploymentTypes = [])
 {
 $this->setAllEmploymentTypes($allEmploymentTypes);
 }
 /**
 * @param string $parameters
 */
 public function setParameters($parameters)
 {
 $parameters = explode('-', ltrim($parameters, '-'));
 if (empty($parameters[0])) {
 array_shift($parameters);
 }
 if (count($parameters) === 3) {
 $this->keywords = $parameters[0];
 $this->employmentTypes = $parameters[1];
 $this->locations = $parameters[2];
 } else if (count($parameters) === 2) {
 $areEmploymentTypes = $this->areEmploymentTypes(explode('~', $parameters[0]));
 $this->keywords = $areEmploymentTypes ? null : $parameters[0];
 $this->employmentTypes = $areEmploymentTypes ? $parameters[0] : null;
 $this->locations = $parameters[1];
 } else if (count($parameters) === 1) {
 $this->keywords = null;
 $this->employmentTypes = null;
 $this->locations = $parameters[0];
 } else {
 $this->keywords = null;
 $this->employmentTypes = null;
 $this->locations = self::DEFAULT_LOCATION;
 }
 $this->keywords = $this->keywords ? explode('~', $this->keywords) : [];
 $this->employmentTypes = $this->employmentTypes ? explode('~', $this->employmentTypes) : [];
 $this->locations = $this->locations ? explode('~', $this->locations) : [];
 return $this;
 }
 /**
 * @param array $items
 */
 public function areEmploymentTypes($items)
 {
 $jobTypes = array_map(function ($v)
 {
 return mb_strtolower($v);
 }, $this->allEmploymentTypes);
 return ! array_diff($items, $jobTypes);
 }
 /**
 * @param array $employmentTypes
 */
 public function setAllEmploymentTypes($employmentTypes)
 {
 $this->allEmploymentTypes = $employmentTypes;
 }
 public function hasDefaultLocation()
 {
 return in_array(self::DEFAULT_LOCATION, $this->locations);
 }
}

I don't like this code (spaghetti code, no separation of concerns...). I think there is a better implementation. I'm also open to other URL structures - I know I can simply use input parameters and call it a day, but I should implement a clean URL. I have added unit tests to handle all cases. I think it might help you understand the code.

• Simon Brahan: Are keywords and employmentTypes individually optional? If so, You'll struggle to tell the difference between /jobs-fulltime-canada and /jobs-php-canada for example. FWIW I think you should just use input parameters, it's not like anyone is going to type this out.
• Razor: I think you meant /jobs-fulltime-canada and /jobs-fulltime-php, yes it's ambiguous but I don't mind in this case. Probably I'm over-engineering, I may reconsider my code, thanks.

Answer 1

The greatest problem with this is that you want to make these elements optional without actually marking whether they are present. There are at least two alternatives to this or normal GET parameters that allow for all three to be optional.

Mark whether they are present

Consider a URI that looks like /jobs/keyword/engineer/c++/type/full_time/part_time/location/canada or /jobs-keyword-engineer-c++-type-full_time-part_time-location-canada (if you want everything in the first slot for search engine optimization). You may prefer something like kword, etype, and loc to make those less likely to show up as keywords themselves. I would dislike those as variable names, but the exigencies here may make them better as parameter names. Ideally those would be things that make no sense as keywords.

This is still slug based. It simply has some extra information to help you parse.

Look up everything

You currently have a PHP-based list of valid employment types. Instead, put every valid employment type and location in a database table. Then you can say something like

SELECT parameter_id, parameter_name, parameter_type 
FROM parameters 
WHERE parameter_name IN (?, ?, ?, ?, ?)

where you then pass (for example)

['engineer', 'c++', 'full_time', 'part_time', 'canada']

as the things to select.

I would prefer to store employment types in the database, as they are data to me. Storing them in code is itself a spaghetti code problem in my opinion.

The results will be all the employment types and locations, which you can then easily represent by objects or IDs. Whatever remains will be the keywords, which presumably aren't bounded in the same way that employment types and locations are.

• Razor: The first URI isn't restful, the second one is interesting though, thanks. I'm already storing employment types and locations in the database, I just don't query the table in my unit tests

Answer 2

Perhaps Simon is right with the comment:

FWIW I think you should just use input parameters, it's not like anyone is going to type this out.

But if you don't agree with that, then perhaps the simplification below will help.

Possible simplification

One consideration I had was to use a regular expression with named subpatterns- something like this:

preg_match('#-?((?P<keywords>[^-]+)-)?((?P<employmentTypes>[^-]+)-)?(?P<location>.+)#', $parameters, $matches);

Which works well for the case where all three parameters are supplied - e.g. "-engineer~c++-full_time-canada~paris":

$matches => Array
(
 [keywords] => engineer~c++
 [employmentTypes] => full_time
 [location] => canada~paris
) //removed numeric keys

So then you can set the properties accordingly:

foreach(['keywords', 'employmentTypes', 'location'] as $key) {
 if ($matches[$key]) {
 $this->$key = $matches[$key];
 }
}

However if either keywords or employment types are omitted, there would still need to be logic to determine which of those is still present... So perhaps it would be better to give more generic names for the first two - like groupA and groupB...

preg_match('#-?((?P<groupA>[^-]+)-)?((?P<groupB>[^-]+)-)?(?P<location>.+)#', $parameters, $matches);

And you would still need the logic to determine if groupA had keywords or employment types... but at least you should likely be able to simplify assignment of the locations parameter.

Simplifying the array_map() call

The call to array_map() in areEmploymentTypes():

 $jobTypes = array_map(function ($v)
 {
 return mb_strtolower($v);
 }, $this->allEmploymentTypes);

Can be simplified by simply passing the function name mb_strtolower as a string literal:

$jobTypes = array_map('mb_strtolower', $allEmploymentTypes);

While there may not be a noticeable improvement in speed, it eliminates the excess closure.

• Razor: Using regex is an interesting alternative, I like it. Assuming keywords and/or employmentTypes are omitted, I'm calling setAllEmploymentTypes then setParameters separately, I'm not sure this is the right thing to do, I even think I shouldn't set $allEmploymentTypes in the class, do you think I should add it as a second parameter setParameters($parameters, $allEmploymentTypes) ?
• Sᴀᴍ Onᴇᴌᴀ: Hmm... I guess if it doesn't end up being used in any other methods then $allEmploymentTypes could just be a second parameter... but hopefully that method would still adhere to the S.R. principle...