[erlang-questions] Why it is a illegal guard expression?

[Richard O'Keefe]

On Feb 24, 2010, at 4:42 PM, 钱晓明 wrote:

> Hi, I am a newer of erlang. I wrote a module to find all files(no  
> directory)
> under a directory, including its subdirectory:
> -module(lib_test).
> -export([list_files/1]).
>
> list_files(Path)->
>    {ok, FileList} = file:list_dir(Path),
>    process_filelist(FileList, []).
>
> process_filelist([File|FileList], Files)->
>    if
>       filelib:is_dir(File) ->
>        {ok, NewFileList} = file:list_dir(File),
>        process_filelist(NewFileList, Files);
>       true ->
>        process_filelist(FileList, [File|Files])
>    end;
> process_filelist([], Files) ->
>    Files.
>
> But I fail to compile this file, because " filelib:is_dir(File) ->" is
> illegal guard expression. So Why it is illegal? What should I write?

What did you find when you looked in the reference manual?
What does it say you are allowed to use in a guard?

You should write

     process_file_list([Path|Paths], Files) ->
         case filelib:is_dir(Path)
           of true ->
	     {ok, Children} = file:list_dir(Path),
	     Children_Paths = [filename:join(Path, Child) || Child <-  
Children],
	     process_file_list(Paths,
		process_file_list(Children_Paths, Files))
	   ; false ->
	     process_file_list(Paths, [Path|Files])
	end;
     process_file_list([], Files) ->
	Files.

There are four issues.
(1) If your function is processing [A,B,C] and A is a directory,
     it will completely forget about B and C.  I suspect that you
     don't want that.  If you do, you really really need to make
     that explicit in a comment.
(2) The documentation for file:list_dir/1 does not make it clear
     whether you get just the simple name of each child or the
     simple name with the directory prefix.  The second is what
     you *need* if you want to recursively walk the directory
     tree; the first is what you *get*.
(3) Not everything that isn't a directory is a file.
     If you simply define a file to be anything that is not a
     directory, OK, but if you think a "file" is what UNIX calls
     a "regular file", that's not what you get.  Arguably you
     should be using
	case <look for it yourself>(Path)
	  of directory -> ...
	   ; file -> ...
	   ; socket -> ...
	   ; device -> ...
	   ; other -> ...
	   ; {error,_} -> ...
	end
     This is an unexpected benefit of the limitations of guards;
     Booleans-and-ifs are very often the wrong thing to do anyway.
(4) I dare say that you understand perfectly well that a single
     directory you're not allowed to search inside will bring your
     whole function crashing down rather than just skipping that
     directory, but it's definitely worth a comment to remind
     less savvy readers.