Rekrul
The more I try to write complex scripts, the more I have a burning hatred for whoever designed this broken mess.
I have this code in my script;
Code: Select all
if !width! gtr 640 set width=1280
if !width! gtr 540 (if !width! leq 640 set width=640)
if !width! gtr 500 (if !width! leq 540 set width=540)
if !width! gtr 400 (if !width! leq 500 set width=500)
if !width! gtr 250 (if !width! leq 400 set width=400)
if !width! leq 250 set width=250
Code: Select all
if !width! gtr 640 set width=1280 else (
if !width! gtr 540 (if !width! leq 640 set width=640) else (
if !width! gtr 500 (if !width! leq 540 set width=540) else (
if !width! gtr 400 (if !width! leq 500 set width=500) else (
if !width! gtr 250 (if !width! leq 400 set width=400) else (
if !width! leq 250 set width=250)))))
For reference, I enable Delayed Expansion inside a FOR loop after the filename variables have been defined, then I ENDLOCAL before the start of the next loop. If I don't define the variables outside of Delayed Expansion, it removes "!" from the filenames. If I don't use Delayed Expansion for everything else, nothing works. This has never caused a problem before since it's reset for every loop and there's never a condition where it would start a new loop without ENDLOCAL. Well, until now apparently. roll
The above bit of code is the ONLY thing I changed, and I made sure that there were no unmatched parentheses, and that it didn't improperly jump out of any loops.
The first example works, no matter how many files it's run on. The second fails. Why???
What arcane rule have I violated now?
Last edited by Rekrul (08 Sep 2019 10:44)
----------------------------
#2 08 Sep 2019 17:31
Aacini
Ok. Lets review a simple example:
Code: Select all
if !width! gtr 640 set width=1280 and any string you want...
Code: Select all
if !width! gtr 640 set width=1280 & echo any string you want...
Code: Select all
if !width! gtr 640 set width=1280 else (
Antonio
----------------------------
#3 09 Sep 2019 04:57
Rekrul
Thanks. You're right, I didn't think of that. I still get confused by the fact that spaces only act as delmiters in some situations (like on the command line).Aacini wrote:
It should be obvious that the entire string is assigned to width variable. Why? Because there is not any delimiter in the string. In this case:
I have rather spotty luck using "&" to chain commands. Sometimes it works fine and sometimes it seems to just ignore it. I ended up solving the problem with;Aacini wrote:
Code: Select all
if !width! gtr 640 set width=1280 & echo any string you want...
Code: Select all
if !width! gtr 640 (set width=1280) else (
if !width! gtr 540 (if !width! leq 640 set width=640) else (
if !width! gtr 500 (if !width! leq 540 set width=540) else (
if !width! gtr 400 (if !width! leq 500 set width=500) else (
if !width! gtr 250 (if !width! leq 400 set width=400) else (
if !width! leq 250 set width=250)))))
Code: Select all
set done=1
setlocal enabledelayedexpansion
if "!done!"=="1" echo success
----------------------------
#4 09 Sep 2019 06:59
Aacini
Well, in this segment of code:
Code: Select all
if !width! gtr 640 (set width=1280) else (
if !width! gtr 540 (if !width! leq 640 set width=640) else (
if !width! gtr 500 (if !width! leq 540 set width=540) else (
if !width! gtr 400 (if !width! leq 500 set width=500) else (
if !width! gtr 250 (if !width! leq 400 set width=400) else (
if !width! leq 250 set width=250)))))
In the first IF, the else part is executed when !width! is NOT gtr 640; this means that in this case !width! is leq 640, right? In this way, the if !width! leq 640 command in the second line is entirely superfluous. The same apply for the rest of nested IF's...
You need to rearrange the logic to eliminate redundant cases AND to simplify the nesting. For example:
Code: Select all
if !width! gtr 640 (set width=1280
) else if !width! gtr 540 (set width=640
) else if !width! gtr 500 (set width=540
) else if !width! gtr 400 (set width=500
) else if !width! gtr 250 (set width=400
) else (set width=250
)
Another, simpler way to achieve the same result, is this:
Code: Select all
set prev=1280
for %%a in (640 540 500 400 250 0) do (
if !width! gtr %%a if !width! leq !prev! set width=!prev!
set prev=%%a
)
Code: Select all
@echo off
setlocal EnableDelayedExpansion
set "limits=0 250 400 500 540 640 1280"
:loop
set /P "width=Width: "
if errorlevel 1 goto :EOF
set /A "token=1-(((%limits: =-width)>>31)+((%-width)>>31))"
for /F "tokens=%token%" %%a in ("%limits%") do set index=%%a
echo %index%
echo/
goto loop
Antonio
----------------------------
#5 09 Sep 2019 12:09
Rekrul
You're right again. The two tests in each line were necessary when each was a separate command because it was going through all of them regardless. When I nested it, I simply added the ELSE commands to what I already had.Aacini wrote:
You have redundant tests. Let's review they.
In the first IF, the else part is executed when !width! is NOT gtr 640; this means that in this case !width! is leq 640, right? In this way, the if !width! leq 640 command in the second line is entirely superfluous. The same apply for the rest of nested IF's...
I think your first example is best. The whole reason I nested them in the first place rather than having separate statements was to try and speed up the execution. I didn't want it to run through all six possibilities if it didn't have to. I realize that the difference is probably only milliseconds, but by the time I've accumulated enough files to fill a DVD, it can range in the several thousands and if I'm going to run the script on the whole collection, I want to squeeze every little bit of speed I can out of it. I got a pretty decent speed boost by changing it to only check the width of images that fit a particular pattern, rather than all of them as I had done previously.Aacini wrote:
Another, simpler way to achieve the same result, is this:
Arithmetic expressions were never my strong suit, and it uses a bunch of conventions I'm not familiar with, so I basically understood none of that. sadAacini wrote:
A more interesting simplification could be achieved via a long arithmetic expression. For example:
To be perfectly honest, I've never been a particularly good programmer. The only languages I ever really learned were BASIC and a little assembly on the C64. Which is why I started getting into batch scripts in the first place. The commands are enough like BASIC that it mostly makes sense to me when Delayed Expansion isn't screwing things up. Part of my desire is to write scripts that do useful stuff, but part of it is also to create something, even if it's a script that only myself and maybe a friend or two will use.
BTW, the full script is posted here if you're curious;
oldforum/viewtopic.php?id=2374
I've revised it since then and figured out how to add working counters to it. After a little more testing, I'm going to post the whole thing in that thread just to kind of bookend my struggles with it. smile
Last edited by Rekrul (09 Sep 2019 12:11)
----------------------------
#6 09 Sep 2019 14:12
bluesxman
This seems to be easier to parse mentally (though I've not tested it)
Code: Select all
set "w=1280"
for %%a in (640 540 500 400 250) do if !width! LEQ %%a set "w=%%a"
set /a width=w
EDIT2: Actually this is more like what you're after I think?
Code: Select all
set "w=1280"
for %%a in (640 540 500 400 250) do (
if !width! LEQ %%a (set "w=%%a") ELSE (goto :break_for)
)
:break_for
set /a width=w
cmd | *sh | ruby | chef
original thread: oldforum/viewtopic.php?id=2386
----------------------------
#7 15 Sep 2019 10:08
Rekrul
bluesxman wrote:
This seems to be easier to parse mentally (though I've not tested it)
That seems like it would work, although it has the same problem as my original solution, namely that it runs through all the possibilities each time.
bluesxman wrote:
EDIT2: Actually this is more like what you're after I think?
I originally tried using a GOTO to skip the rest of the comparisons. I used an additional variable as a flag and then added a test for that flag. However, GOTO apparently has a bug in that it breaks parenthesis. Since this whole section is inside a loop, it breaks the loop.
So far, I've used Aacini's first example of nesting the IF/ELSE commands and just eliminating the second test for each one, as the previous command already covers that.
----------------------------
#8 18 Sep 2019 11:00
bluesxman
Ah yes a double nested loop would fail. An alternative may be to do something like this:
for %%s in (your outer loop foo bar) do (
call :subroutine
)
goto :EOF
:subroutine
set "w=1280"
for %%a in (640 540 500 400 250) do (
if %width% LEQ %%a (set "w=%%a") ELSE (goto :break_for)
)
:break_for
set /a width=w
goto :EOF
But you will experience a small time penalty for the :call (which may become noticable if the outer loop processes a lot of items).
cmd | *sh | ruby | chef