[mention]Flipeador[/mention]

[quote]
If the function does not return any value, it makes no sense to use [c]return[/c].
If the function is going to return an empty string, [c]return ""[/c] can be more descriptive (personally I don't like to treat an empty string as "nothing" or as [c]FALSE[/c]).
If your function is going to return only values [c]1[/c]/[c]0[/c] as true/false (boolean), using [c]TRUE[/c]/[c]FALSE[/c] can be more descriptive than [c]1[/c]/[c]0[/c].
[/quote]

This thread is 1 year old, and now I completely agree with your ideas. I want to better understand them.

1. Do you mean that we don't need to use simple [c]return[/c] in the functions and, instead, we should always use [c]return False[/c], [c]return 0[/c], or [c]return ""[/c]?
2. Could you provide an example where we should return the empty string? I want to add it to my notes.

DirExist(FilePattern)
{
    Loop Files, %FilePattern%, D
        return A_LoopFileAttrib
}

- Thanks lexikos. Here's a fix, I'll add it to the library if no-one can see a problem with it.
commands as functions (AHK v2 functions for AHK v1) - AutoHotkey Community
[url]https://autohotkey.com/boards/viewtopic.php?f=37&t=29689[/url]
[code]
DirExist(FilePattern)
{
Loop Files, %FilePattern%, D
return A_LoopFileAttrib
}
[/code]- The previous version was the original from here:
AutoHotkey-Future/DirExist.ahk at master · cocobelgica/AutoHotkey-Future · GitHub
[url]https://github.com/cocobelgica/AutoHotkey-Future/blob/master/Lib/DirExist.ahk[/url]
- Btw I asked here whether the Type backport could be made to identify Floats correctly.
commands as functions (AHK v2 functions for AHK v1) - Page 2 - AutoHotkey Community
[url]https://autohotkey.com/boards/viewtopic.php?f=37&t=29689&p=210822#p210822[/url]

- (Re. True/False. My main point was that 'return True' and 'return', in the OP, should become 'return True' and 'return False'.)
- I'm tempted to indicate 1/0 over True/False because that's what will be 'printed' if you use MsgBox.
- I suppose people could find the value in the documentation, my instinct was that is was sufficiently difficult that a comment might be worthwhile. 'True' is an awkward search term for the documentation, it's hard to find the info. I couldn't remember the page. And even on the right page, you have to search for it repeatedly until you get to the proper entry.
Variables and Expressions - Definition & Usage | AutoHotkey
[url]https://autohotkey.com/docs/Variables.htm[/url]

[quote="lexikos"]... Your implementation is faulty; if you pass a file pattern that matches both a file and a directory, the result may be incorrect.[/quote]
[c]InStr(FileExist(Dir), "D")[/c] is what's outlined in the docs as a substitute for [c]DirExist[/c]. how is it faulty and in which particular case would that fail? a folder cannot have the same name as a file.

[quote="jeeswg"]The use of 'return True' or 'return False' implies that the function only returns 'True' or 'False', i.e. a Bool.
- So I would do:
[...]
return 1 ;True
return 0 ;False
}[/quote]I don't see the connection between what you say and what you show. If anything, wouldn't you want to return [c]true[/c] or [c]false[/c], since the function only returns true or false?
[quote] return True ;1[/quote]I would never comment the value of [c]True[/c]. Comments for code that is self-explanatory are just noise. It takes very little to learn what value [c]True[/c] returns.

As for the DirExist example: DirExist is built-in in v2, and it returns the first matching directory's file attributes, like FileExist. Your implementation is faulty; if you pass a file pattern that matches both a file and a directory, the result may be incorrect.

In AHK v1 it's safe to assume that true means 1 and that false means 0.
A lot of calculations rely on this.
AHK v2 is still open for change however it doesnt seem like anybody wants to change it.

var := 2 + isInteger(3)
; versus the following, better I think
var :=  2
if (isInteger("test") || isInteger(3))
	var := var + 1

isInteger(_prm) {
	if _prm is integer
		return true
	return false
}

As I see it, [c]true[/c] does not even mean the number 1 (that is as real or natural number). The couple [c]true/false|false/true[/c] is not ordered, for example.
[c]true/false[/c] is rather intended for logical (aka boolean) operations - not for arithmetic calculations.
It is a question of design: if your fonction is intended to be used alongside [c]or[/c] and [c]and[/c] the function should return a boolean value (and the name should already inform about it: 'isXX', 'XXexist' etc.) and provide, if necessary, any additional return values using ByRef parameters:

[code]var := 2 + isInteger(3)
; versus the following, better I think
var := 2
if (isInteger("test") || isInteger(3))
var := var + 1

isInteger(_prm) {
if _prm is integer
return true
return false
}[/code]

DirExists(Dir)
{
	if InStr(FileExist(Dir), "D")
		return True ;1
	return False ;0
}

;or:

DirExists(Dir)
{
	if InStr(FileExist(Dir), "D")
		return 1 ;True
	return 0 ;False
}

;function returns 1 (True) or 0 (False)
DirExists(Dir)
{
	return !!InStr(FileExist(Dir), "D")
}

- lexikos's post reminded me of something I meant to say. If you use 'return True' within a function, then the function should only ever return True or False. The use of 'return True' or 'return False' implies that the function only returns 'True' or 'False', i.e. a Bool.
- So I would do:
[code]
DirExists(Dir)
{
if InStr(FileExist(Dir), "D")
return True ;1
return False ;0
}

;or:

DirExists(Dir)
{
if InStr(FileExist(Dir), "D")
return 1 ;True
return 0 ;False
}
[/code]- However, I usually advocate efficient code and good comments, so I might do this if I wanted to be extra clear:
[code]
;function returns 1 (True) or 0 (False)
DirExists(Dir)
{
return !!InStr(FileExist(Dir), "D")
}
[/code]- Personal preference: I would probably use 1/0 explicitly in functions, and perhaps True/False in scripts to be more descriptive. Functions are generally hard to follow, so I would put any important information in comments just above or at the top of the function.
- In some situations, seeing 'return True' might mislead people into thinking that the output value has been defined as a Bool, and that anything you output via return will be converted to True or False.

The [i]effect[/i] of the code is not the most important aspect in the context of best practices (consider comments, for instance). [c]True[/c] does not just mean "the number 1", even though it has that value - the added meaning is the whole reason for having [c]True[/c] and [c]False[/c]. [c]Return True[/c] shows that the function's result should be interpreted as a boolean value, as in [c]if TheFunction(x)[/c], not a number as in [c]if TheFunction(x) = 1[/c]. A beginner reading the code should not need to know that [c]True = 1[/c]; on the contrary, using [c]return 1[/c] with [c]if TheFunction(x)[/c] might only serve to mystify the code.

- Yes, it's all debatable. Thanks for the link.
- This is pretty good:
SplitPath, InputVar [, OutFileName, OutDir, OutExtension, OutNameNoExt, OutDrive]
- I actually think of *filename* as full path, and *file name* as 'name' i.e. full path minus dir.
- Here are my notes on Excel macros:
vPath := oXl.ActiveWorkbook.FullName ;path (dir\name)
vDir := oXl.ActiveWorkbook.Path ;dir
vName := oXl.ActiveWorkbook.Name ;name

[quote="jeeswg"]Btw I would call the variable 'Dir', not 'DirName'. I think that 'dir name' is ambiguous, e.g. for 'C:\Program Files\AutoHotkey', I would say that 'AutoHotkey' is the dir name.[/quote]

Probably this terminology will be interesting for you: https://stackoverflow.com/a/26637489

I might do something like:
[c]return True ;1[/c]
but since it literally returns 1, I might just do:
[c]return 1[/c]

@gregster You have understood perfectly what I was referring to. Yes, I used the wrong word.

Yeah, I meant specifically: " If your function is going to return 1, you must use return 1, and not return TRUE." Here returning 1 and True is just equivalent - it is exactly the same programatically: [c]1[/c] will be returned. Of course, you are right, if the return value is not used as a boolean value afterwards, the reader of the source code might get a wrong impression. Therefore, I wrote [i]could[/i] :). The use of [i]must[/i] in your sentence felt a bit too strong here, I would say [i]should[/i] instead...

@gregster I'm just saying that if the function is intended to return a boolean value, it is more descriptive to use [color=#0080FF]TRUE[/color]/[color=#0080FF]FALSE[/color].
Of course, you can also do things like [c][color=#0040FF]return[/color] [color=#0080FF]TRUE[/color][color=#FF0000]+[/color][color=#FF8000]2[/color][/c], but personally I find it horrible.

[quote="jeeswg"]I do tend to consider that often people looking at code will be experienced programmers, who do not happen to be familiar with AutoHotkey, but who would like to translate the code to their favoured programming language.[/quote]
That is a totally personal decision. If someone has experience in programming (in another language), it is assumed that you already have to know this. You do not need to know about AutoHotkey for these things. Also, it is not difficult to look at the documentation, unless you are allergic to it.

[color=#FF0000][b]Edit*[/b][/color] I'm not going to reduce the clarity of my Script just to make it easier for other users to understand it (so that in the end he ends up using another programming language), watch a tutorial or read the documentation (or ask me, I'll be happy to explain it to you).

[quote]- @gregster: Thanks for pointing that out, I meant to mention that.[/quote]
That was rather a repsonse to Flipeador :D

"If your function is going to return 1", you would normally use [c]return 1[/c] - but you wouldn't have to, you [i]could[/i] [c]return TRUE[/c], too.

- @Flipeador: I do tend to consider that often people looking at code will be experienced programmers, who do not happen to be familiar with AutoHotkey, but who would like to translate the code to their favoured programming language.
- @gregster: Thanks for pointing that out, I meant to mention that.

msgbox % test()
test(){
	return true
}
msgbox % true

But [c]true[/c] evaluates to [c]1[/c]... always. (Of course, you can use other numbers instead.)
[code]msgbox % test()
test(){
return true
}
msgbox % true
[/code]

[quote="jeeswg"]unless you already knew that in AutoHotkey[/quote]
I already answered it above. It's "ambiguous" for new users who do not know anything about programming. Are you going to limit yourself just because there are users who do not know anything about programming?.Has no sense.

[color=#0040FF]TRUE[/color] is not used as [color=#FF8000]1[/color], but as "true". If your function is going to return [color=#FF8000]1[/color], you must use [c][color=#0040FF]return[/color] [color=#FF8000]1[/color][/c], and not [c][color=#0040FF]return[/color] [color=#0080FF]TRUE[/color][/c].

Func1()
{
	return True ;ambiguous (unless you already knew that in AutoHotkey, True = 1)
}
Func2()
{
	return 1 ;unambiguous
}

checkIfDirExists(Dir)
{
	if (InStr(FileExist(Dir), "D"))
		return 1
	else
		return 0
}
checkIfDirExists(Dir)
{
	if (InStr(FileExist(Dir), "D"))
		return 1
	return 0
}

DirExist(FilePattern)
{
    local AttributeString := FileExist(FilePattern)
    return InStr(AttributeString, "D") ? AttributeString : ""
}

JEE_IsDir(vDir)
{
	return !!InStr(FileExist(vDir), "D")
	;return !!DirExist(vDir)
}

- @Flipeador:
[code]
Func1()
{
return True ;ambiguous (unless you already knew that in AutoHotkey, True = 1)
}
Func2()
{
return 1 ;unambiguous
}
[/code]
- @john_c: Btw I would call the variable 'Dir', not 'DirName'. I think that 'dir name' is ambiguous, e.g. for 'C:\Program Files\AutoHotkey', I would say that 'AutoHotkey' is the dir name.

- Of the following two versions, which do people prefer?
[code]
checkIfDirExists(Dir)
{
if (InStr(FileExist(Dir), "D"))
return 1
else
return 0
}
checkIfDirExists(Dir)
{
if (InStr(FileExist(Dir), "D"))
return 1
return 0
}
[/code]
- Here is what I have for a DirExist backport using Coco's coding style. Part of 'AHK v2 functions for AHK v1'.
[code]
DirExist(FilePattern)
{
local AttributeString := FileExist(FilePattern)
return InStr(AttributeString, "D") ? AttributeString : ""
}
[/code]
- Here is a function I wrote in the past.
[code]
JEE_IsDir(vDir)
{
return !!InStr(FileExist(vDir), "D")
;return !!DirExist(vDir)
}
[/code]