Is this a design pitfall for the batch operations?

edwinsn · 2016-03-19 10:29:32

Hi Arnaud,

I was "trapped" by a case, and I'm wondering if it's worth posting here for your information

Problem: BatchDelete() might have no effects at all, while the results and TSQLRestClientURI.OnFailed() will not report anything about it.

Steps to reproduce the issue:

1 - Use BatchStartAny, but NOT BatchStart();

2 - Use BatchDelete(ID: TID), but NOT BatchDelete(Table: TSQLRecordClass; ID: TID).

3 - Now BatchSend will just return HTML_Success and OnFailed will not report anything.

Thanks for your time!

ab · 2016-03-21 07:42:30

BatchDelete(ID: TID) should return -1 in such case.
This is up to the caller to check the returned value.

edwinsn · 2016-03-21 07:53:47

Hi ab,

If you allow me, I'd like to suggest that this design is error-prone - as you can see in my reported case.

Raising exception is a better way for catching errors, because, quoting from SO user Ned Batchelder:

if you forget to do what you should be doing with status returns, you ignore errors. If you forget to do what you should be doing with exceptions, the exception bubbles up to the outer layer of the software where it becomes visible.

Raising exceptions is a modern software design for error handle, people says.

You can check this StackoverFlow discussion for further info: http://stackoverflow.com/questions/4670 … error-code

Just my 2 cents

ab · 2016-03-21 09:34:22

AFAIR all CRUD operations of TSQLRest do not throw exceptions, but returns error codes.
It is by design.
So Batch methods follows the same pattern.

Issues may happen due to a primary key violation... which is more like an input problem, and not a fatal error.
IMHO exceptions should be... exceptional. Not due to some logic or input issue, but due to an Access Violation, an I/O issue, or some other unrecoverable problem...
See http://blog.synopse.info/post/2015/04/1 … -good-idea

Your SO question is from 2011... whereas my own preference is the more "modern" software design approach of https://golang.org/doc/faq#exceptions

Of course, it is a matter of taste, and you are free not to like it.
It may be misleading for you, I agree, and feel sorry about that.
The point is that it is coherent at TSQLRest level.
I do not see any real benefit of changing the whole API after so much time and use in production...

edwinsn · 2016-03-21 13:09:59

Hi ab,

I'm not just trying to argue, but I view it as **exceptional** if you use BatchStartAny() and then BatchDelete(ID: TID) without providing a TSQLTable class, but because it's not expected to be so.

mORMot Open Source

#1 2016-03-19 10:29:32

Is this a design pitfall for the batch operations?

#2 2016-03-21 07:42:30

Re: Is this a design pitfall for the batch operations?

#3 2016-03-21 07:53:47

Re: Is this a design pitfall for the batch operations?

#4 2016-03-21 09:34:22

Re: Is this a design pitfall for the batch operations?

#5 2016-03-21 13:09:59

Re: Is this a design pitfall for the batch operations?

Board footer