Code error in _StringInsert() - but in my opinion it should not be

[amarcruz]

In Line# 270 of String.au3 (6 of _StringInsert())

ElseIf $s_InsertString = "" Or (Not IsString($s_String)) Then

should be read:

ElseIf $s_InsertString = "" Or (Not IsString($s_InsertString)) Then

by the way,
like this, many functions in Standard UDFs check its parameter type; this is NOT CONSISTENT with AutoIt philosophy, I think.

AutoIt allows expressions like this:
$x = 123 & 456 ; x="123456"
with implicit type convertion, no error.

Why _StringInsert("123", 456, 3) should not working and set @error?
Why _StringInsert("123", "", 3) set @error?
...should not.

OK, this is my opinion, of course.
I have a 17 years base in Assembler, C, FoxPro/VFP, HTML, Javascript, and VBScript, yet I may be wrong.

...and Sorry for my horrible "English" :)

[amarcruz]

_StringInsert, corrected & minor improved version

[anonymous]

I forgot, another inconsitence:
in _StringInsert, $i_position parameter is base 0, not 1.

Help don't say that.

[BrewManNH]

Why _StringInsert("123", 456, 3) should not working and set @error?
Why _StringInsert("123", "", 3) set @error?

The first one works without any errors for me. Are you saying it should or it shouldn't give you an error?

The second one doesn't make a lot of sense, "" may be regarded as a string, but why would you ever try to insert a string with nothing in it? I think that if the string is sent as "", it should be reported as an error because the length of the string is 0, which means you sent the function nothing to work with. Also, if you send it the source string as "" you're not adding anything to an existing string you might as well just use the second parameter as the string, because that's what you'd get back if it worked as you wanted it to.

[jchd]

Inserting an empty string should definitely NOT set @error. An empty string is not "nothing to qork with" but a perfectly legitimate string. That's IMHO.
I'd even would rather the function be:

; #FUNCTION# ====================================================================================================================
; Name...........: __StringInsert
; Description ...: Inserts a string within another string.
; Syntax.........: __StringInsert($s_String, $s_InsertString, $i_Position)
; Parameters ....: $s_String   - Original string
;                  $s_InsertString   - String to be inserted
;                  $i_Position - Position to insert string (negatives values count from right hand side)
; Return values .: Success - Returns new modified string
;                  Failure - Returns original string and sets the @error to the following values:
;                  |@error = 3 : Invalid position
; Author ........: Louis Horvath <celeri at videotron dot ca>
; Modified.......: jchd
; Remarks .......: Use negative position values to insert string from the right hand side.
; Related .......:
; Link ..........:
; Example .......: Yes
; ===============================================================================================================================
Func __StringInsert($s_String, $s_InsertString, $i_Position)
	$s_String = String($s_String)
	$i_Position = Int($i_Position)
	Local $i_Length = StringLen($s_String) ; Take a note of the length of the source string
	If Abs($i_Position) > $i_Length Then
		Return SetError(3, 0, $s_String) ; Invalid position
	EndIf

	If $i_Position >= 0 Then
		Return(StringRegExpReplace($s_String, "(?s)\A(.{" & $i_Position & "})(.*)\z", "${1}" & String($s_InsertString) & "$2"))
	Else
		Return(StringRegExpReplace($s_String, "(?s)\A(.*)(.{" & -$i_Position & "})\z", "${1}" & String($s_InsertString) & "$2"))
	EndIf
EndFunc   ;==>__StringInsert

I find this much more consistent with generic AutoIt expressions using Variant "variance" (e.g. like True & "abc" & 456 which doesn't bark about True and 456 not being strings).

[BrewManNH]

How do you insert nothing (e.g. "") into a string? If it sets @error and tells you that the string was empty, then you can use error checking to see that your insert was empty and the result is the original string. This will at least tell the coder that they are attempting something that didn't work as expected because their string was empty. I can see having it accept an empty source string, but I don't see why you'd want an empty insert string to be legitimate. Differing opinions I suppose, but with @error, you're getting feedback that something went wrong.

[Valik]

Should we modify AutoIt so that the following generates a syntax error?

Local $s = "abc" & "" & "def"

That would be stupid, right? So why then should _StringInsert() treat inserting an empty string as an error? It's a harmless - albeit pointless - operation.

[jchd]

True, since in many cases, inserted string comes as a programatic result which may be the (perfectly valid) empty string, e.g. like a partial result from a regexp where some capturing group may capture nothing. I see no "error" here nor any reason to force complication in the code, only convenience.

If we start flagging all identity operations like $c = $a * $b or $i += $j where $b may be 1 and $j may be zero, we end up with a useless language in practice (IMVHO).

$s = "abc"
$s &= (1 = 2)
gives abcFalse and _StringInsert has no reason to behave differently.

[amarcruz]

Valik, jchd, I agree.
_StringInsert(123, 456, 3) should return "123456"
This is consistent with Autoit.

[guinness]

Removed by revision [7230] in version: 3.3.9.5