*
The moose likes Java in General and the fly likes The ULTIMATE!  From the creator of FileMonitor comes DirectoryMonitor :) Big Moose Saloon
  Search | Java FAQ | Recent Topics | Flagged Topics | Hot Topics | Zero Replies
Register / Login


Win a copy of Android Security Essentials Live Lessons this week in the Android forum!
JavaRanch » Java Forums » Java » Java in General
Bookmark "The ULTIMATE!  From the creator of FileMonitor comes DirectoryMonitor :)" Watch "The ULTIMATE!  From the creator of FileMonitor comes DirectoryMonitor :)" New topic
Author

The ULTIMATE! From the creator of FileMonitor comes DirectoryMonitor :)

Ivan Jouikov
Ranch Hand

Joined: Jul 22, 2003
Posts: 269
Hey everyone!

I just finished working on the greatest thing of all times: DirectoryMonitor. This bad boy can monitor your entire file system (all you have to do is set it on your "/" or "C:/" (with my 35GB full of stuff and 1.7 ghz Java runs at 80% CPU (using 1 second checks that is) )...

Check out these bad boys:

This file demonstrates how you would use directory monitoring:




This file is the FileMonitor - Directory Monitor itself, all 418 lines of it ...




Here's the FileChangeListener:




And the FileChangeEvent:






Also, I wanna know what you guys think of my coding style and my "ability" in general. Please be as critical as possible

BTW, for those who don't know, I wrote this thing because I couldn't find any FileMonitors out there, so I thought, what the hell, I'll just write my own one. This thing simply let's you know if there are any changes inside a file/directory that you're monitoring, such as stuff getting modified, deleted added. 1-line use
Ivan Jouikov
Ranch Hand

Joined: Jul 22, 2003
Posts: 269
QUICK FIX ALREADY!!!

I've been debugging it extensively, but just now I noticed 1 more bug. And fixed it

Here's the new version:

John Smith
Ranch Hand

Joined: Oct 08, 2001
Posts: 2937
Also, I wanna know what you guys think of my coding style and my "ability" in general. Please be as critical as possible

OK, you asked for it. First, do not use all caps, especially in the subject line. This is not a forrest in Russia, we can all hear you well.

public FileReloaderTest() throws Exception

Throwing a general Exception is not a good idea. Instead, list all of them explicitely. That will allow the caller to do some meaningful processing.

while(true)Thread.sleep(1000);

That's not a very thrilling solution to whatever it is that you are trying to do. Did you look into wait()/notify()?

public static long checkInterval = 1000;

"public static final long CHECK_INTERVAL = 1000;"
is more expressive and is in line with the Sun's Java Coding conventions.

protected Timer daemon;

I am not happy about the name "daemon". That's the implementation detail. What are you going to do when you want to change it to non-daemon? I suggest "timer", "monitor", or something of that sort, to indicate what it does, not how it does it.

protected FileMonitor()

You indicated that FileMonitor is a singleton, yet it has a protected constructor, so nothing stops the classes in the same package from creating 200 instances of this class. The constructor should be private. More generally, the abundance of the protected fields and methods is somewhat unsettling.

for ( Iterator i = fileMap.values().iterator(); i.hasNext();

Although your fileMap is a synchronized collection, it doesn't mean that you have thread safety when iterating through it. If another thread modifies your fileMap while you are iterating, you are in trouble.


FileMonitor.instance.debug("..");
FileMonitor.getInstance().fileMap.remove(file);


So is it instance or getInstance()?



This doesn't look very solid. If the exception is thrown and caught, your code proceeds to notify the listeners, which will probably confuse the hell out of them.
[ June 23, 2004: Message edited by: Eugene Kononov ]
Ivan Jouikov
Ranch Hand

Joined: Jul 22, 2003
Posts: 269
First, do not use all caps, especially in the subject line. This is not a forrest in Russia, we can all hear you well.

I only used caps cuz I was really thrilled about my achievements I rarely USE THEM LIKE AN IDIOT


Throwing a general Exception is not a good idea. Instead, list all of them explicitely. That will allow the caller to do some meaningful processing

Hmm, every time I was doing throws Exception, this was on the top of my head... It's just that often you have same exception travleing through all of your methods, and you have to write the same stuff for javadoc comment over and over. How do you deal with it? You just tought it out and write it?

That's not a very thrilling solution to whatever it is that you are trying to do. Did you look into wait()/notify()?

That's just the tester class, it doesn't server any purpose other than demonstrate how you would register a file with FileMonitor. I expect the user to terminate it using "Ctrl+Z" (on Linux) or using their IDE...

is more expressive and is in line with the Sun's Java Coding conventions.

The reason I didn't use final is because i wanted the user to be able to change the interval. But now, come to think about it, changing that INT won't change the interval on the timer, so might as well make it final. Thanks!

I am not happy about the name "daemon". That's the implementation detail. What are you going to do when you want to change it to non-daemon? I suggest "timer", "monitor", or something of that sort, to indicate what it does, not how it does it.

Good point. Thanks.

You indicated that FileMonitor is a singleton, yet it has a protected constructor, so nothing stops the classes in the same package from creating 200 instances of this class. The constructor should be private. More generally, the abundance of the protected fields and methods is somewhat unsettling.

Well, when I was designing it, I was thinking that people could extend it. But now come to think about it, you can't really extend a singleton can you? I'd have to use a factory pattern if I wanted it to be extendable, I guess...

Although your fileMap is a synchronized collection, it doesn't mean that you have thread safety when iterating through it. If another thread modifies your fileMap while you are iterating, you are in trouble.

So you're saying I'd have to use "synchronized"? Then what's the point of making a synchronized map in the first place, if it doesn't synchronize it?

So is it instance or getInstance()?

I wanna say that I am 100% sure that using instance would be safe in that case, but my 100% insurance failed many times, so I'll change that to getInstance()

This doesn't look very solid. If the exception is thrown and caught, your code proceeds to notify the listeners, which will probably confuse the hell out of them.

How would you propose to solve that exception? Basically, what's happening there, is we JUST FOUND THE FILE, and there's NO WAY I could get a FileNotFoundException... It's like doing

MyObject b = new MyObject();

void blah(){ // do nothing }

try{ b.blah() }
catch( NullPointerException e ) {...}

I have NO idea how to approach this kind of "problem". Suggestions would be useful.

Thanks for your criticizm, it's very useful. Hope I get more.
Ivan Jouikov
Ranch Hand

Joined: Jul 22, 2003
Posts: 269
Wow already found another bug thanks to ur advices
 
wood burning stoves
 
subject: The ULTIMATE! From the creator of FileMonitor comes DirectoryMonitor :)
 
Similar Threads
file change notification
Apache Common VFS
What you think of this style?
I wrote my own FileMonitor. Check it out.
java.util.Collections is a huge load of ****?