4

I'm trying to reverse "hello" to "olleh". But the output shows "ooooo".
I think !string:~%back%,1! is the problem, because when I use echo to test the value of back is decreasing or not, it works, but it doesn't work in substring, so it always get the last character of the string (-1,1).

@echo off

set string=hello
set temp_string=%string%
set /a string_length=0

:find_length
if defined temp_string (
    set temp_string=%temp_string:~1%
    set /a string_length+=1
    goto :find_length
)

:loop
setlocal enabledelayedexpansion

set /a back=-1

for /l %%a in (1,1,!string_length!) do (
    set reverse_string=!string:~%back%,1!!reverse_string!
    set /a back-=1
)

echo !reverse_string!
pause >nul
BongBong
  • 71
  • 1
  • 7
  • 2
    I think it's because `%back%` is being used without delayed expansion, so it's always using `-1`. – TripeHound Aug 04 '15 at 12:57
  • So how am I going to make it used with delayed expansion? – BongBong Aug 04 '15 at 13:01
  • You can't directly, because you need the combination of `!..!` and `%..%` to allow a variable index. You'll have to go back to an old-fashioned loop... see answer – TripeHound Aug 04 '15 at 13:05
  • @BongBong If any of the answers below was helpful, please consider marking your preferred answer as accepted. [See this page](http://meta.stackexchange.com/questions/5234/) for an explanation of why this is important. – rojo Aug 04 '15 at 13:45
  • @rojo yeah i'll do it later on – BongBong Aug 04 '15 at 14:52

4 Answers4

4

As TripeHound commented, %back% needs to be delayed. What you should do is use the for /L loop value of %%a to in place of %back%. (No sense decrementing a variable manually when one's already being decremented for you as a part of the for /L loop, right?)

for /l %%a in (%string_length%,-1,0) do (
    call set "reverse_string=!reverse_string!!string:~%%a,1!"
)

goto loops are not very efficient. If you've got a long string you're going to reverse, there'll be a noticeable pause while you count its length if you goto :label for each character. The fastest way I've found to get the length of a string is based on jeb's answer here:

:length <return_var> <string>
setlocal enabledelayedexpansion
if "%~2"=="" (set ret=0) else set ret=1
set "tmpstr=%~2"
for %%I in (4096 2048 1024 512 256 128 64 32 16 8 4 2 1) do (
    if not "!tmpstr:~%%I,1!"=="" (
        set /a ret += %%I
        set "tmpstr=!tmpstr:~%%I!"
    )
)
endlocal & set "%~1=%ret%"
goto :EOF

Put it all together like this:

@echo off
setlocal

set "string=%*"

call :length string_length "%string%"

setlocal enabledelayedexpansion

for /l %%a in (%string_length%,-1,0) do (
    set "reverse_string=!reverse_string!!string:~%%a,1!"
)

echo(!reverse_string!
pause >nul

exit /b 0

:length <return_var> <string>
setlocal enabledelayedexpansion
if "%~2"=="" (set ret=0) else set ret=1
set "tmpstr=%~2"
for %%I in (4096 2048 1024 512 256 128 64 32 16 8 4 2 1) do (
    if not "!tmpstr:~%%I,1!"=="" (
        set /a ret += %%I
        set "tmpstr=!tmpstr:~%%I!"
    )
)
endlocal & set "%~1=%ret%"
goto :EOF

Example output:

command: test.bat The quick brown fox
result: xof nworb kciuq ehT

Community
  • 1
  • 1
rojo
  • 24,000
  • 5
  • 55
  • 101
  • +1 Nice. I was considering merging the role of `count`/`back` in my answer (but left them to keep the overall logic the same) but didn't make the leap that using `%%a` will work because you've still got the `!`/`%` mix _but_ its value gets updated each time round the loop. – TripeHound Aug 04 '15 at 13:28
  • There is no need for `call`… – aschipfl Dec 07 '21 at 07:45
2

The problem is that %back% is being used without delayed expansion, so will always have the value -1. Replacing the end of your code with:

set /a back=-1
set /a count=1
:repeat
if %count% gtr %string_length% goto :report
    set reverse_string=!string:~%back%,1!!reverse_string!
    set /a back-=1
    set /a count+=1
    goto :repeat

:report
echo !reverse_string!

Will do the trick.

You cannot just use !back! because you need the contrast of !...! and %...% to have a variable index, so you'll have to go back to an old-fashioned :loop construct so %back% gets updated each time around.

TripeHound
  • 2,721
  • 23
  • 37
2

As described at this post:

"To get the value of a substring when the index change inside FOR/IF enclose the substring in double percents and precede the command with call. For example:

for /l %%a in (1,1,!string_length!) do (
    call set reverse_string=%%string:~!back!,1%%!reverse_string!
    set /a back-=1
)

Another way to achieve previous process is using an additional FOR command to change the delayed expansion of the index by an equivalent replaceable parameter, and then use the delayed expansion for the substring. This method run faster than previous CALL:

for /l %%a in (1,1,!string_length!) do (
    for %%b in (!back!) do (
        set reverse_string=!string:~%%b,1!!reverse_string!
    )
    set /a back-=1
)

"

However, it is not efficient to first loop thru the string just to count the characters, and then loop again to reverse they. I think the method below should be the fastest one:

@echo off
setlocal EnableDelayedExpansion

set maxLength=80

set string=hello
set "reverse="
for /L %%i in (1,1,%maxLength%) do (
   set "reverse=!string:~0,1!!reverse!"
   set "string=!string:~1!"
   if not defined string goto break
)
:break
echo %reverse%
Community
  • 1
  • 1
Aacini
  • 65,180
  • 12
  • 72
  • 108
  • I thought about this, but my method loops max 13 times + length of string. Yours always loops maxlength iterations, regardless of the length of the string. Still, it's short and sweet, and the performance difference would be infinitesimal I'm sure since `for /L` is a highly efficient method of looping. You get a +1 from me anyway. :) – rojo Aug 04 '15 at 14:08
  • @rojo: Even if the max length would be much larger, this method is very fast because the `goto` command "short-circuit" the `for` iterations. See [this similar method](http://stackoverflow.com/questions/28700043/thousands-separator-of-an-integer-value/28704587#28704587) and dbenham's comment below it. **`:)`** – Aacini Aug 04 '15 at 14:34
  • The `goto` skips execution of the instructions within the `for /L`, but the loop continues to iterate until the end. Like you say, it is very fast, even if you increase the 80 to 8000. Running our scripts 1000 times, my script averages about 3.5 seconds. Yours with maxlength=80 averages about 2.1 seconds, so it is more efficient for length limited strings. Bumping maxlength up to 8000 made the 1000 iteration test jump to 53 seconds, though. I admit that's an extreme example and not likely relevant in practice, but it does illustrate that `goto` doesn't break out of `for /L`. – rojo Aug 04 '15 at 15:12
  • Wow! That 53 seconds result with maxlength=8000 was unexpected! May I ask you to complete one test more with maxlength=2730? Accordingly to dbenham's comment in my previous link, this would take about 10 msec (per time)... – Aacini Aug 04 '15 at 16:35
  • maxlength=2730, 1000 times, test string of `hello` averaged about 19.5 seconds. Interestingly, I discovered that if I have a 502-character string to reverse, if I loop it 1000 times, your script set to maxlength=502 averages ~24 seconds, while mine takes around 14.5. I think your script is the more efficient script for small strings; but for larger, your use of an additional cycle to `set "string=!string:~1!"` counteracts the gains achieved by foregoing calculating the string length. Our scripts are nearly the same speed @~80 - 100 chars with yours set maxlength=stringlength. – rojo Aug 04 '15 at 18:51
  • @rojo: Yes, when the string is large, my method needs to move the entire string - 1 for each char! The best program should check if the string is larger than a certain limit and then choose the appropriate method: larger or shorter string... – Aacini Aug 04 '15 at 20:35
1

Here's another algorithm:

@echo off
call :reverse "The quick brown fox"
echo %output%
pause & exit

:reverse
    setlocal enableDelayedExpansion
    set string=%~1
    set index=0
    :loopchar
        set char=!string:~%index%,1!
        if "!char!"=="" endlocal & set output=%output% & exit /b
        set output=!char!!output!
        set /a index+=1
    goto loopchar

"Tricks":

  • using %index% inside ! to expand its current value instead of call with %% (@Aacini)
  • export of !output! from the local scope by reassigning it on the same line as endlocal.
wOxxOm
  • 65,848
  • 11
  • 132
  • 136
  • Wow, that's short but it works. Can you explain why you use `%~1` instead of `%1`? And also I thought it's going to be like this `!string:~%index%,1!`, why? – BongBong Aug 04 '15 at 13:22
  • 1) `%~1` strips the outer doublequotes. 2) the `string` variable doesn't change its initial value so no need to delay its expansion; instead `%%` through a `call` is used to postpone extraction of the character inside the called statement where `index` will be already expanded before `call`. – wOxxOm Aug 04 '15 at 13:26
  • 1
    The line `call set char=%%string:~!index!,1%%` can be simply `set char=!string:~%index%,1!` because this line is _not_ in a code block. – Aacini Aug 04 '15 at 14:03
  • Oh, thanks, I wouldn't expect it to work, seems counter-intuitive. Welp, batch-syntax isn't intuitive anyway. – wOxxOm Aug 04 '15 at 14:06