CODEWITHSUNDEEP
coldfusionerror-handlingcoldfusion-8
Vincent P
Vincent P

Reputation: 426

Wrap CFTRY tags around everything

We have a system that lets a user upload a file, we loop through that file and then make another file. The user that uploads the file is a logged in user.

Problem is that the files contain sensitive data so we have to delete them. As you can imagine there are a few places that write more info to the file and read the file. Sometimes an error happens on this page (normally something to do with CFFILE).

So my question is, is it fine to place all the code (most of it anyway) in a giant CFTRY? Then catch any exception that happens, then run another CFTRY inside the CFCATCH to delete the 2 files?(Read the update) I'm not too worried about performance as this process is not done a million times a day, maybe 3 times a month.

Is this acceptable practice for making sure files are deleted?

UPDATE I wont be deleting the files in the CFCATCH. I'll first check if the exist. Then delete them.

Upvotes: 1

Views: 705

Answers (4)

Vincent P
Vincent P

Reputation: 426

I had to write a new section of code that almost did the same thing as what I asked. Instead of writing individual lines to the file and then wrapping a big CFTRY around that. I instead wrote each line to a variable and ended each line with a new line character, in my case (Windows) the new line characters where Chr(13) & Chr(10). But you should rather use the following line of code

<cfset NL = CreateObject("java", "java.lang.System").getProperty("line.separator")>

Which will make the variable NL equal to the current system new line character.

Then you can have a small CFTRY where you can write the entire variable to a file.

Upvotes: 0

genericHCU
genericHCU

Reputation: 4446

The only thing I would say about a huge try/catch block is that it stops all processing in the try block so if you have stuff that could still be done, stopping the whole train just because there is a quarter on the track may be overkill.

I have a similar process that works with a bunch of files, we put each process in a separate try / catch block so they don't interfere with each other. i.e. a broken first file doesn't screw up the next 3 perfectly good files. The catch block simply adds the error message to a string then notifies the user of a bad format (or whatever) in the file(s) that were bad but the good files processed as expected.

<!--- file one --->
<cftry>
  some stuff
  <cfcatch>
    <cfset errors = errors & "file one did not work because #cfcatch.message#">
  </cfcatch>
</cftry>

<!--- file 2 --->
<cftry>
  some stuff
  <cfcatch>
    <cfset errors = errors & "file two did not work because #cfcatch.message#">
  </cfcatch>
</cftry> 
<cfetc...>

If you're looping over a dynamic set you can put the try/catch block INSIDE the loop so the try/catch doesn't stop the loop and the other stuff can process. of course this doesn't work if file 2 depends on file 1...

<cfloop index = "i" ...>
  <cftry>
    some stuff
    <cfcatch>
      <cfset errors = errors & "file #i# did not work because #cfcatch.message#">
    </cfcatch>
  </cftry>
</cfloop>

Upvotes: 3

Dan Bracuk
Dan Bracuk

Reputation: 20804

We have a similar situation regarding files and take a different approach.

Step 1 is to limit access to the directories containing the files.

Step 2 is scheduled cleanup. We have a ColdFusion job that runs every day. It inspects various directories and deletes any file more than x days old. The value of x depends on the directory.

This approach may or may not suit your situation.

Upvotes: 1

Adam Cameron
Adam Cameron

Reputation: 29870

It's fine to use try/catch whenever it's warranted. There are no CFML police which will come and drag you away in the middle of the night if you put the try/catch around 101 lines of code instead of the permissable 100 lines of code.

However - as @Tomalak says - your wording kinda suggests that the code could stand some refactoring. You say you can't refactor the code, but adding exception handling is already refactoring, so clearly you can do it. So do it properly. Isolate bits of functionality, and put them into separate modules (I don't mean like as called by <cfmodule>, I mean the term generically), be they UDFs, methods in one or more CFCs (they're probably disparate, so probably not appropriate for a single CFC), or even just include files. They can be refactored better later on. Development is iterative and cyclical, remember: you do not need it to be perfect every time you make changes. For one thing, the definition of "perfect" changes as requierments change. But you should aim to always improve code when you maintain it. And I don't think simply slapping one try/catch around the whole thing suggests an improvement, more like "this code is out of control".

Another thing I can suggest is to make your improvements and perhaps post it to https://codereview.stackexchange.com/, and find out what others think. I dunno how many CFers inhabit that site, so it perhaps might be good to post something on Twitter marked with #ColdFusion when you've done so.

Upvotes: 3

Related Questions