On 10/11/06, Spark Shen <[EMAIL PROTECTED]> wrote:
Hi All: After skimming through the source code of java.io.File, I found two tweaked method names: private native boolean isReadOnlyImpl(byte[] filePath); private native boolean isWriteOnlyImpl(byte[] filePath); Take isWriteOnlyImpl as an example, canRead -> exists && !Java_java_io_File_isWriteOnlyImpl() -> => this method is in native\luni\shared\file.c getPlatformIsWriteOnly() => this method is in native\luni\linux\helpers.c, and the follow is the code snippet if (buffer.st_uid == geteuid ()) return (buffer.st_mode & S_IRUSR) == 0; else if (buffer.st_gid == getegid ()) return (buffer.st_mode & S_IRGRP) == 0; return (buffer.st_mode & S_IROTH) == 0; The name isWriteOnlyImpl is confusing, negate the return value of isWriteOnlyImpl and uses it in canRead is confusing, in native method getPlatformIsWriteOnly(), prob S_IRXXX bit is also confusing and can not express the meaning writeOnly. Correct me if I am wrong. Since isWriteOnlyImpl is only utilized by canRead, I suggest the following: change isWriteOnlyImpl to isReadableImpl, getPlatformIsWriteOnly to getPlatformIsReadable and do the negation in getPlatformIsReadable: canRead -> exists && Java_java_io_File_isReadOnlyImpl() -> getPlatformIsReadable() if (buffer.st_uid == geteuid ()) return (buffer.st_mode & S_IRUSR) != 0; else if (buffer.st_gid == getegid ()) return (buffer.st_mode & S_IRGRP) != 0; return (buffer.st_mode & S_IROTH) != 0; This way, code is more readable and the method name getPlatformIsReadable semantically matches it implemantation. (The original name getPlatformIsWriteOnly seems not consistent with its implmentation, correct me if I am wrong).
Yes, you're right and it's better to refactor. Just be careful about the native-refactoring, due to that no enough tool support for it. :-) If no one objects, I want to supply a patch to do the enhancement.
Best regards -- Spark Shen China Software Development Lab, IBM --------------------------------------------------------------------- Terms of use : http://incubator.apache.org/harmony/mailing.html To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
-- Robert Hu China Software Development Lab, IBM