Reversing string in PHP

Question

I got this task to evaluate my knowledges in PHP. I was asked to avoid using functions like strrev() or array_reverse().

Apparently they did not like my solution. Can you provide details as to what is wrong with it or how it could be done better?

function reverse_string($str)
{
    if(!strlen($str)){
        return '';
    }
    $reversed = '';
    for($i=strlen($str)-1;$i>=0;$i--){
        $reversed .= $str[$i];
    }
    return $reversed;
}
echo reverse_string('abcdefg3.14');

Output

41.3gfedcba

Answer 1

That is quite a "basic" example, and yet there are different approaches possible.

PSR

There is one things that puts me off a bit: spacing. Spacing makes code more readable, thus easier to understand. Your coding style is too compact.

As already said PHP, like other languages has some widely accepted coding standards like PSR. Observance of which (or the lack thereof) is a telltale sign of how experienced the programmer is.

Coding is often teamwork, which means your colleagues must be able to maintain other colleagues' code (especially after they have left the company...) and adherence to standards facilitates maintenance.

I don´t know about the conditions in which the exercise took place, but a good IDE with linter could reformat the code for you. If you are only allowed to use notepad, then you should still try to write code more or less compliant. At least it would be apparent you are used to those coding standards, and you are just outside of your comfort zone, which they would understand.

Multibyte

Not handling multibyte string could be problematic too, as the code will not always perform as expected.

Concatenation

A minor detail maybe, but as far as I know PHP doesn't have the equivalent of a Stringbuilder, so concatenation is implemented somewhat rudimentary. On longer strings, this could result in poor performance and memory usage. But you don't even need to concatenate. PHP has a yield statement like in Python, so you could simply consume the string character by character, in a generator fashion. Just a thought.

Edge cases

There is one test in your code, but unfortunately it is not useful.

What if I pass a NULL variable to your function? I guess it would return an empty string but this is flawed.

And if a number or a float is passed to the function you could decide to cast it to a string, or reject it. Implementation is up to you, but you can make this function more robust either way.

While it may not be possible to think of every scenario within the allocated time, it is still a good idea to add some comments and TODO list, just to show that you have thought about the possible pitfalls, and you are aware of what is missing to have production-ready code.

Answer 2

correctness

It's unclear what the specification

reverse a string

really means in this context. To reverse US ASCII like "hello world" is straightforward, and properly handled by the OP code. But the internet is global, and an input like "«déjà vu»" would fare poorly. Is it "a string"? Or would a sensible specification keep calling out that this or that field is "a multibyte string"?

For a text processing solution that can handle "internet text", prefer to call mb_str_split().

special case

    if (!strlen($str)) {
        return '';
    }

It's hard to see why that's even part of the solution. Surely running the for loop through zero iterations should suffice, producing the same result.

linear complexity

You used the .= catenation operator -- good! It results in \$O(n)\$ linear performance. In order to use it, it was necessary to traverse the source string in reverse order.

Definitely do not traverse characters in forward order to build a result using this:

    $reversed = "{$character}{$reversed}";

It is observed to suffer from \$O(n^2)\$ quadratic performance, taking several seconds for a task that should complete within 20 msec. The trouble is we keep copying "the string so far" into a new result string. In contrast .= requests a "big enough" buffer and then keeps extending the result string, without re-reading and re-re-reading partial results. And even with reallocation copying, amortized cost using a constant growth factor will still remain linear.

Answer 3

Can you provide details as to what is wrong with it or how it could be done better?

Starting the counter length doesn't consider multi-byte characters

As was mentioned in a comment, the first expression of the for loop contains a call to strlen($str) to calculate the first value of $i. Per the documentation of strlen():

Note:

strlen() returns the number of bytes rather than the number of characters in a string.

For multi-byte strings this will not work well as some characters may be broken up. One could use mb_strlen() to get the length of a multi-byte string and mb_substr() to get multi-byte characters at given indexes.

Use idiomatic spacing

While this is purely subjective, most idiomatic PHP follows standards like PSR-12, which has many recommendations on styling and other code conventions. The line which stands out as the most incongruent with such conventions is the loop declaration:

for($i=strlen($str)-1;$i>=0;$i--){

Idiomatic PHP code would have more spaces after the keyword for, the semi-colons and the closing parenthesis, as well as surrounding binary operators:

for ($i = strlen($str)-1; $i>=0; $i--) {

The php.net documentation often has idiomatic code. For example, the documentation for the for keyword contains this example (one of four):

<?php
/* example 1 */

for ($i = 1; $i <= 10; $i++) {
    echo $i;
}
...

Answer 4

function reverse_string($str)

could be

function reverse_string(string $str) : string

Then it would enforce that the input has to be a string immediately rather than failing when strlen is called.

This is an example of something that might fail you on a culture fit basis. Some organizations prefer strong typing even if they use a language like PHP that is traditionally weakly typed.

It's also worth noting that some of these interviews expect you to ask questions to clarify the problem. Not asking questions can fail the interview even if the code complies with the basic description. I.e. the goal of the interview may be to see how you handle ambiguous requirements.