• Post Reply Bookmark Topic Watch Topic
  • New Topic
programming forums Java Mobile Certification Databases Caching Books Engineering Micro Controllers OS Languages Paradigms IDEs Build Tools Frameworks Application Servers Open Source This Site Careers Other Pie Elite all forums
this forum made possible by our volunteer staff, including ...
Marshals:
  • Campbell Ritchie
  • Jeanne Boyarsky
  • Ron McLeod
  • Paul Clapham
  • Liutauras Vilda
Sheriffs:
  • paul wheaton
  • Rob Spoor
  • Devaka Cooray
Saloon Keepers:
  • Stephan van Hulst
  • Tim Holloway
  • Carey Brown
  • Frits Walraven
  • Tim Moores
Bartenders:
  • Mikalai Zaikin

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

 
Ranch Hand
Posts: 269
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
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
Posts: 269
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
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:

 
Ranch Hand
Posts: 2937
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
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
Posts: 269
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
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
Posts: 269
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Wow already found another bug thanks to ur advices
 
reply
    Bookmark Topic Watch Topic
  • New Topic