StringSplit bug?

[ybouan]

The following works great in the stable version but not in the the beta (83):

Func ParseData($line)
    If StringLeft($line,1) = ';' Or StringStripWS($line,8) = '' Then
        Return ''
    EndIf
    Local $cmd = StringStripWS($line,8)
    Local $cmd =  StringSplit($cmd, ']')
    Local $cmd =  StringSplit($cmd[1], '[')
    If $cmd[0] >= 2 And StringIsXDigit($cmd[2]) Then Return $cmd[2]
    Return ''
EndFunc

I get the following error:

Subscript used with non-Array variable.:

Local $cmdA = StringSplit($cmdA[1], '(')

The problem seems to be the repetition of "local"

If I do that only once it works.

Maybe this should not be allowed and was thus fixed in the newer version...

Edited October 20, 2005 by ybouan

[Josbe]

The following works great in the stable version but not in the the beta (83):

Func ParseData($line)
    If StringLeft($line,1) = ';' Or StringStripWS($line,8) = '' Then
        Return ''
    EndIf
    Local $cmd = StringStripWS($line,8)
    Local $cmd =  StringSplit($cmd, ']')
    Local $cmd =  StringSplit($cmd[1], '[')
    If $cmd[0] >= 2 And StringIsXDigit($cmd[2]) Then Return $cmd[2]
    Return ''
EndFunc

I get the following error:

Subscript used with non-Array variable.:

Local $cmdA = StringSplit($cmdA[1], '(')

The problem seems to be the repetition of "local"

If I do that only once it works.

Maybe this should not be allowed and was thus fixed in the newer version...

Why not try to change different variable names for every StringSplit? (no test, only a question)

[/dev/null]

The following works great in the stable version but not in the the beta (83):

Func ParseData($line)
    If StringLeft($line,1) = ';' Or StringStripWS($line,8) = '' Then
        Return ''
    EndIf
    Local $cmd = StringStripWS($line,8)
    Local $cmd =  StringSplit($cmd, ']')
    Local $cmd =  StringSplit($cmd[1], '[')
    If $cmd[0] >= 2 And StringIsXDigit($cmd[2]) Then Return $cmd[2]
    Return ''
EndFunc

I get the following error:

Subscript used with non-Array variable.:

Local $cmdA = StringSplit($cmdA[1], '(')

The problem seems to be the repetition of "local"

If I do that only once it works.

Well, I guess the problem is that you're trying to use $cmd as a string variable and as an array. That won't work.

Local $cmd = StringStripWS($line,8) ; <== the local variable $cmd is a STRING

Local $cmd = StringSplit($cmd, ']') ; <== StringSplit returns an array, thus the local variable $cmd must be an array, which conflicts with your string definition above.

If you omit the second "local", $cmd refers to a global variable with the same name as the local one. Then it's no problem to have a local string variable $cmd and a global array $cmd.

Solution: You cannot define a variable as a string AND as an array. Use different names and your problem is gone. Whether this is a bug or not? I don't know. I guess the error message could be more descriptive...

Cheers

Kurt

[Valik]

How can anybody diagnose this? The code and the error message are clearly not the same. I see no variables or in fact no lines in the example as compared to the error. Please show the real code or show an error message generated by the example code.

[w0uter]

well its a fact that he shouldnt have 3x local.

@kurt what do you mean that he cannot use the same variabels.

i have never had problems doing something like this:

dim $x = "var"
$x = stringSplit($x, 'a')
consoleWrite($x[1] & @LF)

[/dev/null]

How can anybody diagnose this? The code and the error message are clearly not the same. I see no variables or in fact no lines in the example as compared to the error. Please show the real code or show an error message generated by the example code.

see code below. I would like to know why there is an error message at line 18. If it's not O.K. to define the same variable twice with local, then there should be a different error message....

Func ParseData_OK($line)
    If StringLeft($line,1) = ';' Or StringStripWS($line,8) = '' Then
        Return ''
    EndIf
    Local $cmd = StringStripWS($line,8)
    $cmd =  StringSplit($cmd, ']')
    $cmd =  StringSplit($cmd[1], '[')
    If $cmd[0] >= 2 And StringIsXDigit($cmd[2]) Then Return $cmd[2]
    Return ''
EndFunc

Func ParseData_Problem($line)
    If StringLeft($line,1) = ';' Or StringStripWS($line,8) = '' Then
        Return ''
    EndIf
    Local $cmd = StringStripWS($line,8)
    Local $cmd =  StringSplit($cmd, ']')
    Local $cmd =  StringSplit($cmd[1], '[')
    If $cmd[0] >= 2 And StringIsXDigit($cmd[2]) Then Return $cmd[2]
    Return ''
EndFunc

$retval = ParseData_OK("Hex [0ac9ff] String")
msgbox(0,"",$retval)

$retval = ParseData_Problem("Hex [0ac9ff] String")
msgbox(0,"",$retval)

Shorter version of the "problem":

$retval = StringSplit("Hello World"," ")
msgbox(0,"",$retval[1])  ; this one works
global $retval = StringSplit($retval[1],"l"); this one does NOT. The global keyword seems to trigger the error message

Cheers

Kurt

Edited October 24, 2005 by /dev/null

[Valik]

The minimum test-case is probably:

Local $v = "Test"
Local $v = $v
MsgBox(4096, "", $v)

Likely, the expected output in the message box is "Test" but instead it is empty. My guess is that when AutoIt sees a declaration and assignment operation needs to occur, it goes ahead and declares the variable being assigned to and initializes its value to the default. In the example above and as demonstrated by the OP, when this happens to be re-declaring a variable already in scope, that variable is destroyed first. Internally the code is probably trusting that the user isn't doing something stupid.

I wouldn't necessarily call this a "bug". It's definitely unexpected and probably not desirable behavior, but I have the feeling AutoIt is simply doing exactly what it was written to do in this situation. In reality, AutoIt should of been throwing the same error that it does when a user tries to re-declare a function argument as a local variable.

However (A very strong however), I'm pretty sure trying to make the above code throw an error will break code written like this:

For $i = 0 To 200
    Local $v = GetDataFromSomewhere()
  ; Use $v
Next

Since AutoIt only has function scope and not loop scope, the example above is technically redeclaring $v in the same scope for every iteration of the loop. The only difference in this case is that we aren't trying to use $v in the re-initialization of $v. I also want to point out that from a code reading perspective, the code above is correct. While it is true that $v is still at function scope even though it is declared in the scope, for purposes of reading the code, it is implied that any variable declared in a block is only visible inside that block and not outside of it. Even if this isn't enforced by the language, it's still a good "rule" to self-enforce.

The best resolution I see is to detect that the variable already exists when it is being re-declared but rather than throw an error, if the scope is the same as the previously declared variable, just silently drop the scope specifier. That would mean the two snippets are functionally equivalent (The second example currently works):

Local $v = "Test"
Local $v = $v
MsgBox(4096, "", $v)

Local $v = "Test"
$v = $v; No scope specifier
MsgBox(4096, "", $v)

[/dev/null]

However (A very strong however), I'm pretty sure trying to make the above code throw an error will break code written like this:

For $i = 0 To 200
    Local $v = GetDataFromSomewhere()
 ; Use $v
Next

that's probably true, but it will only break code that's not correct anyway (according to the rule of variable declaration). If we can only define variables at function or global scope the interpreter should throw an error if we do that twice: Variable $v already defined at function/global scope.

The best resolution I see is to detect that the variable already exists when it is being re-declared but rather than throw an error, if the scope is the same as the previously declared variable, just silently drop the scope specifier. That would mean the two snippets are functionally equivalent (The second example currently works)

I would prefer the error message above, even if it breaks scripts (that are not correct anyway). However I think it's much more convenient to simply drop the second scope specifier, as this is most probably what the programmer wanted to do.

Cheers

Kurt

[Valik]

I think you missed some of my objection, Kurt. Take this simple loop for example:

For $i = 0 To 1
    Local $v = "Test"
    UseV($v)
Next

As far as the interpreter is concerned, it sees this code as:

Local $v = "Test"
UseV($v)
Local $v = "Test"
UseV($v)

The interpreter doesn't have any way to establish that the variable is declared over and over inside a loop which is the distinguishing characteristic that makes the code correct. Instead, what the interpreter sees is like the OP demonstrated where the same variable is being declared over and over.

There are probably several solutions but the easiest is to probably just silently drop the scope specifier. Au3Check should probably throw a warning in this situation, though.

[Celeri]

Valik, dev/null, wOuter, Josbe, you already know this.

For the others, pleas read.

This might seem unrelated (not in this case however!) but since I've read Jos' suggestions on writing UDFs, I found that some of my problems disappeared. So here's a few pointers.

It's rough for the first 15 minutes but after that you'll be grateful and have easier programs to parse trough:

ALWAYS declare locally if used within a UDF

and ALWAYS declare Globally if used elsewhere.

You can check if you left something behind by writing AutoItSetOption("MustDeclareVars",1) as one of the first lines in your script.

Then run Tidy and track down variables beginning with "####" in the resulting report. They are most likely stray variables.

Adhere to a clear naming scheme for all variables:

$a<letter> - Array (the following letter describes the data type taken from the rest of the data types below)

$b - Binary data

$h - File or window handle

$i - Integer

$f - Boolean

$n - Floating point number

$s - String

$v - Variant (unknown/variable type of data)

Give clear, significant names to your variables

Make variables anyone can recognise easily. Don't make the name too long either. Usually 5-10 letters is more than enough to explain what a variable represents.

Don't use reserved keywords

I don't have a list handy but even if using a keyword as a variable works, it's never smart. Anyone with a list we can post somwhere?

Examples ...

A command line string with all the switches and all could look like this:

..... $s_Cmd = "c:\cmd /f /whatever"

An array containing different filenames with their path

..... $as_FullFilename[0]

A simple loop

..... $i_Loop = 1 to 50

An array of possible command lines

..... $as_Cmd[0] = "c:\cmd /f /whatever"

NEVER DO THIS

..... $a=jj($b*$c/$d)

... it's impossible to read and even harder to debug ...

Conclusion

As you can see with this system, variables CAN have (almost) the same name.

You also reduce the chance of using a variable both locally and globally (and having some nasty surprises.

If someone picks up the code he/she will know a variable is supposed to be a string, an array or whatever. It makes debugging much easier and it helps find solutions faster.

Edited October 26, 2005 by Celeri

[ybouan]

Valik, after reading my post I realize I did not provide enough information in the post above and I am sorry.

My problem is not really that I cannot get this to work, but rather that the behavior was different in the beta version. Since I had solved the issue I went on to other things and lost the exact code I was using so I cannot rerun the test. I will try to reproduce the behavior and post it back to you guys if I confirm an issue actually exist with the code and note my head :-)

As you guys have mentioned it is not really correct to assign a string into an array of the same name. (I have a nasty habit of trying to use as little variable names as possible...)

The possible fixes are:

- Make only the first assignement local but this is VERY unclean

- Use different variable names for your arrays.

[Valik]

- Make only the first assignement local but this is VERY unclean

Come again? How is it unclean? Variables should be declared only once. Its flat out wrong to declare a variable multiple times, we just can't enforce the error due to the way AutoIt is designed (see my previous replies about why that is).

It sounds to me like you've developed a warped perspective on what is "clean" or "good" code, at least as far as the usage and naming of variables is concerned.

[LxP]

ALWAYS declare locally if used within a UDF

and ALWAYS declare Globally if used elsewhere.

Hmm... I have to say that I never ever use Global in my scripts (or at least haven't yet). If I write a function that needs access to a variable from my main code, I'll just pass it as an argument. If I need to modify that variable then generally I'll just pass it ByRef. I'm not a huge fan of Global variables at all.