• Post Reply
  • Bookmark Topic Watch Topic
  • New Topic

NX:Create/Delete invalid data

 
Tony Collins
Ranch Hand
Posts: 435
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Just wonder what the the wisdom is regarding the reporting of invalid data
for create and update methods. Say the incorrect amount of fields or a field with a larger length then allowed is passed to the createRecord or upDateRecord.
I see that there are 4 options
1) Pass a negative number back to caller if data is invalid( though updateRecord is void)
2) throw Invalid Data Exception chained in Duplicate Key Exception/RecordNotFoundException.
3) Pad data out/chop data off, report nothing though log fact data was lost.
4) Throw Runtime Exceptions.
4, I think is a no no.
1, is OK but could not work with upDateRecord as it doesn't have a return
value. I wouldn't want inconsitent error reporting methods for updateRecord and createRecord, so would rule out.
3) Is a bit shoddy and not really the way API's work.
4) Chaining duplicatekeyException and RecordNotFoundException for invalid data passed to createRecord and updateRecord seems wrong.
Any ideas past experiences.
Tony
 
Philippe Maquet
Bartender
Posts: 1872
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Hi Tony,
My preferences go to (in decreasing order) 3-4-2-1.
Regards,
Phil.
PS: I think that you'll get as many different opinions as answers
 
Tankred Smult
Greenhorn
Posts: 16
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Hi.
I've decided to throw a
IllegalArgumentException in such cases, which is a runtime exception.
As long as you document what this method accepts as valid input in the javadoc, I do not think you should require the clients catch exceptions thrown if they violate this contract.
Even in suns api they don't. Imagine having to write this each time you create an integer:

Luckily NumberFormatException is a RuntimeException, so you don't have to.
 
Philippe Maquet
Bartender
Posts: 1872
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Hi Tankred,
Very good argument !
My new prefered sequence is 3==4-2-1
I really hesitate between 3 and 4 if field length is the only issue. All our fields are just simple text fields, and chopping data off in case of overflow seems defendable to me.
Best,
Phil.
 
Jim Yingst
Wanderer
Sheriff
Posts: 18671
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
I prefer IllegalArgumentException as well. Especially if the number of fields is incorrect - that indicates a programming error so bad, I want to force the coder to fix it. If the programmer is getting that stuff wrong, I don't trust them to check return values or log files either. And using a DuplicateKeyException is just wrong, semantically - the problem has absolutely nothing to do with a duplicate key. Throwing a DuplicateKeyException feels like lying to me; I won't do it.
If the field length is wrong - well if it's too short, that's no problem at all; I simply pad with spaces, and logging of this should probably be at Level.FINER or FINEST, if at all. If it's too long, I use an IllegalArgumentException, as I consider that this should have been validated at the client before sending it to the server. However I feel less strongly about this than I did about incorrect field count; using option 3 might be acceptable here.
 
Tankred Smult
Greenhorn
Posts: 16
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
I don't think it is appropriate to just cut off the string at the end. The data class isn't capable of deciding the importance of the field, and whether or not it is ok to just cut off the end of the string, as it has no knowlegde of the data it is inserting.
In my assignment I've got a field called "Houry charge", which is 8 bytes. If a Contractor were charging say "$123456789.00", I don't think it would be ok if the database were to say he would charge only "$1234567".
Imagine the look on my face when I discover that the plumber I thought I got at the bargain price of $1234567/hour in fact demands $123456789.00.
This example is maybe not very realistic, but my point is, as the Data class has no knowlegde of the importance of the data, it should leave the decision up to it's client, not make one without even letting the client know.
So the way I see it, an exception is the only acceptable solution.
 
Vlad Rabkin
Ranch Hand
Posts: 555
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Hi All,
I use IllegalArgumentException if request has incorrect number of fields (elements in array) and NullPointerException if request value is null.
I suppose it was Jims words:
"I like to write code so that if something fails, it fails spectacularly"

And it must that way in these cases...
Cheers,
Vlad
 
Jim Yingst
Wanderer
Sheriff
Posts: 18671
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
This example is maybe not very realistic, but my point is, as the Data class has no knowlegde of the importance of the data, it should leave the decision up to it's client, not make one without even letting the client know.
Good point. I think it's not really a very important concern for the actual application we are dealing with, but it could well be signifcant for future enhancements. Especially for those of us who are taking pains to keep the Data code free from any assumptions about the specific Contractors application, to maximize possible re-use.
 
S Bala
Ranch Hand
Posts: 49
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Runtime exceptions and logging seem to the most suitable choices. Initially I had used IORecordNotFoundException - subclass of RecordNotFoundException for the read() method. But, my create() and find() methods are calling the read() method and I am forced to chain/log the IORecordNotFoundException. Chaining the IORecordNotFoundException with DuplicatekeyException seems ridiculous. So I am going to throw specific runtime exception which will be caught in the server itself, and a client specific exception will be thrown to the user. Eg. - The server site is having problems, please contact the admin.
we should not propagate many program specific runtime exceptions to the client. The client has no clue where to handle and what to handle. They will have to wrap every method call in a runtime exception try - catch block.
- SB.
 
Manoj Gundawar
Ranch Hand
Posts: 169
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Hi,
I feel all these apporaches will fail in real life situation. As a programmer these are understandable approaches, but as a user of the system, these are not. User wont like an application to crash or an exception to be trhown just because he entered a wrong data. to avoid this possibility, is it not possible to design GUI such a way that user wont be able to enter more digits then acceptable limit?
I see that as the best solution rather than allowing user to enter unlimited number of digits and then rejecting it, or showing error on the screen.
(I have learned this from my past 4 years of programming experience. Users always want a very friendly system, where they need minimum keystrokes or mouse clicks. And what they hate most is to see that the data is accepted and error is thrown after few seconds)
Just a thought.
Thanks,
Manoj
 
Tony Collins
Ranch Hand
Posts: 435
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
I supose if you throw a runtime exception you have to ensure it is never thrown, unless there is a bug.
Tony
 
Vlad Rabkin
Ranch Hand
Posts: 555
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Hi,
Tony is absolutely right. That is why I throw RuntimeException.
The client application should never send an incorrect request.
If does the RuntimeException will signal about this bug.
Cheers,
Vlad
 
Jim Yingst
Wanderer
Sheriff
Posts: 18671
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
to avoid this possibility, is it not possible to design GUI such a way that user wont be able to enter more digits then acceptable limit?
Absolutely, that's what we would want. But the create() and update() methods are on the server, not the GUI - so what we're considering is, what if the GUI did not do its job correctly (either because we screwed up or because some junior programmer changed the code), and invalid data is submitted to the server, even though the GUI was supposed to validate it? In this case, IllegalArgumentException makes it clear that the person who coded the GUI client has screwed up. Assuming that we provided proper documentation of what arguments are considered legal.
 
Tony Collins
Ranch Hand
Posts: 435
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
The negative point of throwing a runtime exception, I supose, is the server could crash as a result of the client.
Which can't be right ? That's got to be a security problem.
Also it doesn't make the server very flexible.
 
Tony Collins
Ranch Hand
Posts: 435
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
I think SUN might be tempting us to throw a Runtime exception, which in every book seems to be a NO NOooo.
Tony
 
Jim Yingst
Wanderer
Sheriff
Posts: 18671
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
The negative point of throwing a runtime exception, I supose, is the server could crash as a result of the client.
Why? The server throws the exception to the client, just like it does for a SecurityException or FileNotFoundException. I sure hope those don't crash the server...
I think SUN might be tempting us to throw a Runtime exception, which in every book seems to be a NO NOooo.
Ask yourself WWJD? (What Would Joshua Do?)
The Book of Joshua has Item 40: Used checked exceptions for recoverable conditions and run-time exceptions for programming errors. I suppose this could be considered a recoverable programming error, in the sense that if we catch it the client could try something else I suppose. But I'd rather they didn't - they should complain loudly to the programmer and make them fix it. Passing invalid arguments the server can't deal with is most definitely a programming error. There is ample justification for throwing an IllegalArgumentException in this case - that's what the exception is there for.
 
Manoj Gundawar
Ranch Hand
Posts: 169
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Now I got it correct.
1. GUI wont allow to enter invalid argument lengths.
2. But as the update code is on the server side, it has to make an assumption that it can happen and has to handle it.
But if we see in real life applications, this would happen:
If some wrong data sneaks into the system, dont crash the system. Instead log it in some exception report. In this case it would be logged to logger.
But this would be applied to many other scenarios. For ex what if user enters non numeric values for the fields which only needs numeric values. I guess this would be handled at the client end. But does server code also need to handle it?
In that case there would be duplication of code for all the validation that are already done on the client side.
Thanks,
Manoj
[ August 07, 2003: Message edited by: Jmannu gundawar ]
 
Andrew Monkhouse
author and jackaroo
Marshal Commander
Pie
Posts: 12007
215
C++ Firefox Browser IntelliJ IDE Java Mac Oracle
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Hi Manoj
This is far more common in real life situations than you realise. It is only because we are coding both the client and server in this assignment that it appears we are duplicating our error handling.
In my last job, I was frequently writing middleware solutions where we received data (in an EDI standard) from clients somewhere in the world and had to translate that into commands the back end host would accept. Now even though we were using EDI standards, we still had clients leaving out mandatory fields, and putting alphanumeric values in numeric only fields and other "silly" mistakes. 70% of our code was error handling. And from what I have seen around the world, this is fairly normal when you are working on middleware or back end solutions.
The client code you write should never send invalid data to the back end, but if someone else sends invalid data, you should handle it before your database becomes corrupt. Sending an exception to the client seems like a reasonable solution. Logging it on the server side is also very useful (gives you a way of explaining what happened).
Regards, Andrew
 
Manoj Gundawar
Ranch Hand
Posts: 169
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Agreed!
manoj
 
Tony Collins
Ranch Hand
Posts: 435
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
So say a programming error produces a checked exception from one of the API's etc. Is it acceptable to place an assertion in the catch block for that checked exception. Or is this frowned upon.
I agree with the 70% error checking, I worked in emmbedded real time systems and I would say it's 90%( Exceptional case) 10% Normal Behaviour.
 
Andrew Monkhouse
author and jackaroo
Marshal Commander
Pie
Posts: 12007
215
C++ Firefox Browser IntelliJ IDE Java Mac Oracle
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Hi Tony
Assertions can be great.
If they get triggered during testing, you get a very clear indication of what when wrong and why.
When someone else reads your code, you have a line that serves as a comment telling them what you expect never to happen.
And when the customer runs your code, they should never be aware that assertions exist, as they should never be triggered in live situations.
Regards, Andrew
 
Jim Yingst
Wanderer
Sheriff
Posts: 18671
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
[Manoj]: If some wrong data sneaks into the system, dont crash the system. Instead log it in some exception report. In this case it would be logged to logger.
Well I'd certainly log to the logger. But fro the server, I'd also throw an exception; this won't crach the server, that's for sure. It also won't crash my client because I do explicitly catch RuntimeExceptions from the client and treat them basically the same as I do RemoteExceptions:
"An unexpected error occurred which prevented your request from being processed. Please e-mail tecnical support at [...]. For detailed information about the error, consult the log at [...]."
[Manoj]: But this would be applied to many other scenarios. For ex what if user enters non numeric values for the fields which only needs numeric values. I guess this would be handled at the client end. But does server code also need to handle it?
It could. We're not obligated to check for everything we possibly could; I think it's a question of how much work do we want to put in checking for a given type of error, and how serious would the problem be if it were not prevented. In my code, the server is very generic - the server doesn't know or care anything about what the different fields inthe DB mean; it just knows what the header file tells it. That is, it knows the numbre of fields, and the name and length of each field. That's it. So, it's pretty easy for me to check number of fields and length of field. It would be more work for me to insert checks of data type, syntax, etc - and they would tend to reduce the reusability of my code. It's certainly possible to do this and still have reusable code - I'm just saying it's more work. And how important is this? If the field count is wrong, or the field exceeds its allowed length, then at a minimum that field data cannot be stored as it was submitted. Worse, dependig on how the code is written, you may corrupt the data of neighboring fields, or neighboring records. That's pretty bad, and it's easy to prevent from the server, so I check for it on the server. On the other hand, if someone wants to put "foo" into a numeric field - well, at least that won't affect any other fields, or records. And we are inserting the data exactly as requested, without changing it. It's not our fault if someone's choosing to enter invalid data. And we don't have an explicit requirement to prevent this. And it would be more work to identify the proper format of each field and validate it on the server. So in this case, the cost/benefit ratio is significantly higher than it was for the field length issue, and I choose not to put effort into this type of validation. In a "real" system it would probably be a good thing to have, even necessary - but I see it as out of scope here.
 
Jim Yingst
Wanderer
Sheriff
Posts: 18671
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
[Tony]: So say a programming error produces a checked exception from one of the API's etc. Is it acceptable to place an assertion in the catch block for that checked exception. Or is this frowned upon.
I agree with all Andrew's comments about assertions. In general they should be perfectly acceptable in your assignment. But I'm not sure exactly how you intend to use the in this particular case - could you elaborate?
 
Tony Collins
Ranch Hand
Posts: 435
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
I've had to type this three times by the way, but it's the hotest day in Liverpool for 100 years and the pubs are open.
Say Interuptted Exception I'm never going to intterupt an exception, so if my code ends up in that particular catch block, should I assert.
???
How can you lot deal with weather like this every day, back in the colonies ?
 
Andrew Monkhouse
author and jackaroo
Marshal Commander
Pie
Posts: 12007
215
C++ Firefox Browser IntelliJ IDE Java Mac Oracle
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Hi Tony
Say Interuptted Exception I'm never going to intterupt an exception, so if my code ends up in that particular catch block, should I assert.

Yes, that could be a valid place to use an assertion.
The way I see it, if you own all the threads, you know that none of your threads will ever call Thread.interupt(), so you know that this exception will never be thrown.
Far too often, I see code in publications where the coder decides just to swallow the exception, which is not good.
Putting an assertion in there lets anyone looking at your code know that you looked at the possible exception, and determined why it is not relevant to process it past the assertion.
How can you lot deal with weather like this every day, back in the colonies ?

We are just hardier
Actually the problem is not the heat, it is the cold. We can handle the nice warm days of 30 or so, and can still be happy when it gets up to 40 or 45 (dont get that in Sydney very often - used to get it in Melbourne ). But now that we are getting all the way down to 7 at night, my girlfriend and I are feeling the cold - the houses just aren't designed to hold heat.
Regards, Andrew
[ August 08, 2003: Message edited by: Andrew Monkhouse ]
 
Jim Yingst
Wanderer
Sheriff
Posts: 18671
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Say Interuptted Exception I'm never going to intterupt an exception, so if my code ends up in that particular catch block, should I assert.
Assert what? There are a lot of things you could assert - in this case, it sounds like you want to assert that the catch clock will never be run. For this, something like

is appropriate. If the statement is exacuted at all (and assertions are enabled), it will throw AssertionError. You could also add a log() statement in case assert is disabled.
 
Don't get me started about those stupid light bulbs.
  • Post Reply
  • Bookmark Topic Watch Topic
  • New Topic