Accounting-Companion / TallyConnector

You can use Tally Connector to Connect your desktop/Mobile Applications with Tally seamlessly.
47 stars 29 forks source link

GetLedger should return error instead of exception #33

Closed reach2rv closed 10 months ago

reach2rv commented 10 months ago

Hi @saivineeth100

I think GetLedger or any other get method should return error instead of object not found exception this will help in first checking if object exist and create if not there, while doing integration with other systems.

Please suggest

saivineeth100 commented 10 months ago

Hi @reach2rv how can we return error instead of ledger?

the intention for throwing exception is to communicate if object does not exist (if this exception thrown, no need to check again if object exists or not) We need to surround get ledger with try catch block and handle what to do if object exists and if not.

reach2rv commented 10 months ago

Hi @reach2rv how can we return error instead of ledger?

the intention for throwing exception is to communicate if object does not exist (if this exception thrown, no need to check again if object exists or not) We need to surround get ledger with try catch block and handle what to do if object exists and if not.

Yes currently I am doing the same using try catch block. What we do is return null if there is no value so validation becomes easy and can be written without try catch block. As value not found can not be an error or exception. It's same like SQL query if query has no values you get empty array or list in ORM.

saivineeth100 commented 10 months ago

Previously I used to do so(return null), but it's not good practice and I also faced some problems in prod apps I created.

There can be more than one reason that GetLedger or any get method fails like tally not opened or Query resulted empty or xml parsing fails. If I return null in those 3 cases, it's not possible trace because it returned null.

Since these methods are expected to get at least one object and not array we are throwing exception, if returned xml doesn't have min single object. but GetLedgers method which expects a list as result we will not throw exception if list is empty, we will return an empty array.

I Leaned somewhere in Internet that throwing Exception is better than returning null and using this practice I am also able to trace error fastly Return null or Throw Exception - Best Practice? Clean Code Tip: throw exceptions instead of returning null when there is no fallback

If you think writing without, try and catch block is easy, it makes tracing why it returned null is hard, instead surround with try catch block and catch exceptions based on type and act. accordingly

reach2rv commented 10 months ago

Agreed. Thank you!