www.digitalmars.com         C & C++   DMDScript  

digitalmars.D.bugs - [Issue 8909] New: is{File,Dir,SymLink} mix return error code and exception

reply d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=8909

           Summary: is{File,Dir,SymLink} mix return error code and
                    exception
           Product: D
           Version: D2
          Platform: All
        OS/Version: All
            Status: NEW
          Severity: normal
          Priority: P2
         Component: Phobos
        AssignedTo: nobody puremagic.com
        ReportedBy: dimitri.sabadie gmail.com



07:46:15 PDT ---
I think a call to .isFile / .isDir / .isSymLink shouln’t throw any exception.
The problem here is that to use, for instance, isFile, we have to use it this
way :

try {
    "path/to/file".isFile;
} catch (const Exception e) {
    /* treat the error here */
}

Such a fonction is excepted — according to its name — to be used by checking
it’s returned value :

if (!"path/to/file".isFile) {
    /* treat the error here */
}

The problem is that the current interface mix both those methods, so the above
codes is just fucking weird for two reasons :

1. if "path/to/file" is not a file, the if statement won’t even be executed
because of the thrown exception, so an if statement is not good here
2. we don’t catch the exception, so we can’t treat the error. An if we do,
we
don’t rely on the returned value.

I propose to remove the exception throw. I’d really love to write this patch,
I’m motivated to contribute to D and all projects around it.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Oct 29 2012
next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=8909


Jonathan M Davis <jmdavisProg gmx.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jmdavisProg gmx.com



PDT ---
I believe that the only reason that they'll throw is if they fail to stat the
file, and it makes perfect sense IMHO for them to throw in that case. Neither
true nor false would be correct if the file doesn't even exist in the first
place. As long as you check that the file exists in the first place, you
shouldn't need to worry about them throwing exceptions.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Oct 29 2012
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=8909




12:02:26 PDT ---


 I believe that the only reason that they'll throw is if they fail to stat the
 file, and it makes perfect sense IMHO for them to throw in that case. Neither
 true nor false would be correct if the file doesn't even exist in the first
 place. As long as you check that the file exists in the first place, you
 shouldn't need to worry about them throwing exceptions.


I think you’re wrong, because when we use a property or function called is*,
we
don’t expect it to throw exception. I personally don’t and I thank it’s
insane
to do so. Furthermore, the fact that is{File,Dir,SymLink} « stats » a file is
an implementation detail thus we don’t have to know anything stat relevant.
Then your argument about stat failure is not out of topic IMHO. If the property
actually fails to stat a file, we don’t really care about why. If we do, I
think a set of more specialized function should be used.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Oct 29 2012
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=8909




12:04:47 PDT ---


 Then your argument about stat failure is not out of topic IMHO. 


I meant « is out of topic ».

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Oct 29 2012
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=8909


Jonathan M Davis <jmdavisProg gmx.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|                            |WONTFIX



PDT ---
I don't think for a second that there's anything special about isX functions
which make it so that they can't throw. It all depends on what they're doing.
If they can't throw, then they're marked with nothrow.

And while the stat call may be an implementation detail, regardless of how
isFile, isDir, or isSymlink are implemented, they rely on the fact that the
file that they're given exists, so _of course_ there's going to be a serious
problem if the file doesn't exist. Throwing is the correct behavior in that
case.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Oct 29 2012
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=8909




13:22:10 PDT ---


 And while the stat call may be an implementation detail, regardless of how
 isFile, isDir, or isSymlink are implemented, they rely on the fact that the
 file that they're given exists, so _of course_ there's going to be a serious
 problem if the file doesn't exist. Throwing is the correct behavior in that
 case.


This case should be test before a call to isFile IMHO. Here isFile has a too
high responsibility.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Oct 29 2012
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=8909




PDT ---
Of course, you test whether the file exists before calling isFile, isDir,
isSymlink. That avoids getting the exception when the file doesn't exist. But
those functions can still be called without having first checked whether the
file existed, and it's perfectly possible for the file to be deleted between
the time that you check that it exists and you call isFile/isDir/isSymlink. So,
those functions must handle the case where they're given a file which doesn't
exist or is otherwise inaccesible (permissions could also make them fail and
could make exists fail in the same way). As such, those functions have only 3
options when they are unable to determine whether the file is a
file/dir/symlink

1. Throw an exception.
2. Return true.
3. Return false.



because that would mean that the program would be being told that the file
wasn't a file/dir/symlink when it doesn't actually know what it is. After a
call to isDir returned false, it could end up assuming that file operations
would work on it (since almost everything is either a file or a directory) and
then do who-knows-what based on incorrect information. If
isFile/isDir/isSymlink can't determine what the file is, then it really doesn't
make sense to return either true or false. So, the correct way to handle it is
an exception. Not to mention, that allows for much better error-handling. If an
exception is thrown, then the program may be able to respond according to what
the error code in the exception was (e.g. it may be able to indicate to the
user that they don't have permissions to read the file).

I think that it's quite clear that throwing an exception is the correct thing
for isFile/Dir/Symlink to do when they are unable to query the file for whether
it's a file/dir/symlink.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Oct 29 2012
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=8909




14:38:19 PDT ---


 Of course, you test whether the file exists before calling isFile, isDir,
 isSymlink. That avoids getting the exception when the file doesn't exist. But
 those functions can still be called without having first checked whether the
 file existed, and it's perfectly possible for the file to be deleted between
 the time that you check that it exists and you call isFile/isDir/isSymlink. So,
 those functions must handle the case where they're given a file which doesn't
 exist or is otherwise inaccesible (permissions could also make them fail and
 could make exists fail in the same way). As such, those functions have only 3
 options when they are unable to determine whether the file is a
 file/dir/symlink
 
 1. Throw an exception.
 2. Return true.
 3. Return false.
 


 because that would mean that the program would be being told that the file
 wasn't a file/dir/symlink when it doesn't actually know what it is. After a
 call to isDir returned false, it could end up assuming that file operations
 would work on it (since almost everything is either a file or a directory) and
 then do who-knows-what based on incorrect information. If
 isFile/isDir/isSymlink can't determine what the file is, then it really doesn't
 make sense to return either true or false. So, the correct way to handle it is
 an exception. Not to mention, that allows for much better error-handling. If an
 exception is thrown, then the program may be able to respond according to what
 the error code in the exception was (e.g. it may be able to indicate to the
 user that they don't have permissions to read the file).
 
 I think that it's quite clear that throwing an exception is the correct thing
 for isFile/Dir/Symlink to do when they are unable to query the file for whether
 it's a file/dir/symlink.


I well listen to what you said, and I still don’t agree. The problem is in
your
own solution :


 […] That avoids getting the exception when the file doesn't exist. But


What the point of avoiding a function to throw? You said there’s function to
check if the file exists (may you tell me which please?), so we can call it
before to call isFile without throwing exception -> it’s a huge design
problem,
because we will never really catch the exception. And it’s quite logic,
because
isFile, isDir or isSymLink, by the responsibility they have, shouldn’t check
if
the file exists. In worst situation, the fact the file exists may appear as a
precondition of the property, but an exception…

I’m afraid to figure out how, nowadays, people want to overuse features where
simple and more adapted ones would be far away enough… You mark this thread as
WONTFIX, it’s a pity, because there’s definitely a problem around those
properties.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Oct 29 2012
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=8909




PDT ---
Checking whether the file exists makes it so that you don't have to take the
performance hit when the exception is thrown and allows you to treat the
exception as the exceptional case rather than expected. But since it makes no
sense to return either true or false when the file can't be checked (due to no
longer existing or a lack of permissions or whatever), there's really no choice
but to throw an exception unless you want to go the C route of returning an
error code and doing the actual return through a pointer or ref argument. But
if you want that, just use the C functions. Regardless, you can't assume that
the function will work, because it can fail even if you check for the file's
existence beforehand.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Oct 29 2012
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=8909




01:24:12 PDT ---
Ok, got you meant. The thing is I’d more likely use a two-steps approach (if
exists then if isFile, for instance). Some guys talk about concurrency, I think
it’s not a good argument here because isFile is not atomic either if used with
exception. Though I understand why it’s marked as WONTFIX.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Oct 30 2012
prev sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=8909




01:31:37 PDT ---
A good link about the topic:

http://www.digitalmars.com/d/archives/digitalmars/D/674.html

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Oct 30 2012