• Post Reply Bookmark Topic Watch Topic
  • New Topic

Is there a better way?  RSS feed

 
Tom Sullivan
Ranch Hand
Posts: 72
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
I am still pretty new to java. I am trying to get the hang of not only
OO but also some semblance of good design and practice.

Below is the code that I have a question about... I think that what I
have done is simplistic (maybe even dumb but I am not sure) and while
it works, I want to know if there is a better way and are there
any dangers to the way that I am going about solving my problem.



The reason for the infinite loop above is that the composite
doc is actually created in constructor code above the preceding lines:



If I don't go into an infinite loop, I get a nullpointerexception
because my ParseHTML object is looking for a file that does not yet
exist. So, the infinite loop insures that the file is actually in
the directory before ParseHTML goes looking for it. Is the method I
have employed reasonable or is there a better way? If the method I
am using is valid, then should I have other parameters around my loops
so that the object will stop running the loop at some point if there
is some problem and then report that problem or is it safe to assume
that the file must be created at some point so the loop will not
run forever?

Thanks,
Tom

[ January 14, 2006: Message edited by: Tom Sullivan ]

[ January 14, 2006: Message edited by: Tom Sullivan ]
[ January 14, 2006: Message edited by: Tom Sullivan ]
 
Mark Spritzler
ranger
Sheriff
Posts: 17309
11
IntelliJ IDE Mac Spring
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator


You could simply write the above code as



Usually you don't want to call a method or such in the () of a while. But you would have had that inside the while {} anyway.


Why do you even have the second while loop for canRead?



Well it depends on your IDE, and how you like things. but you could do this



I changed the value to -1 since if the value is not in the String indexOf returns -1. So even though you assigned first and second to 0, it would be changed to either the actual indexOf if found, or -1 if not found. and first and second would only equal 0 if the value was found in the begining.

Also the second one with two lines for the return helps when you use a debugger to get a particular value, in most cases, in yours you just see what the value of first and second is before the return and is easy to determine. Plus most debuggers also have an expression watch.

Mark
[ January 14, 2006: Message edited by: Mark Spritzler ]
 
Tom Sullivan
Ranch Hand
Posts: 72
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Sorry, I did leave out a little detail in my question.

I do know that I can be less verbose in the writing of the code. I simply did what I did for readability (not that the convention is good which I appreciate feedback on as well).

The file is created by an external app. But, it all happens in the same constructor where the file is later trying to be read by the parser object. Because the external app can not create the file before the software then calls to it, I have to make the object wait a bit before it passes the file path info to the parser object. I test the existance of the file first. Next I make sure that there the app that created the file does not still have a lock on it in through the canRead() method.

This is the way that the software works:

1. View presents HTML file in HTML editor to client.
2. Client edits the file
3. New file is created in temp dir.
//here is where model kicks off
4. Model tests existance of new file (not shown above)
5. Model passes new file path and old file path to diff program in constructor (shown above)
6. Diff program creates composite file (mark up of changes with strike through of deleted text and highlighted new text).//rt.exec(other program)
7. The other app creates the composite of HTML changes from old to new doc creating yet another doc (composite doc which I need to get).
8. Constructor calls file to parse the HTML (happens too fast hence the loop).
9. HTML is cleaned up (excess tags removed) and changes are logged in XML file for presentation to other users who can accept or reject changes...(methods not shown).

I don't have the stream to read until the file is created. I don't actually create the file... I simply take the file that is created and do stuff with it.
[ January 14, 2006: Message edited by: Tom Sullivan ]
 
Tom Sullivan
Ranch Hand
Posts: 72
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Just before you wrote that, I looked at the API and found that the sb will
return -1 if !found. Glad that was discovered... The code worked as
expected during my tests but I would hate to see it return a false value due to a mistake...
[ January 14, 2006: Message edited by: Tom Sullivan ]
 
  • Post Reply Bookmark Topic Watch Topic
  • New Topic
Boost this thread!