Make `TableHandle` implement `IDataContainerHandle` (fixes #260)

[prodehghan]

This fixes NullReferenceException when calling Dbosoft.YaNco.Table.GetTypeDescription() method.

[CLAassistant]

All committers have signed the CLA.

[fw2568]

Hello,

thank you for the issue report and the PR.

However just changing ITableHandle to IDataContainerHandle would introduce another issue. IDataContainerHandle is used in all cases where the handle could contain other data (for example in GetStructure, GetTable, GetString, and so on).

Could you please instead introduce another interface ITypeHandle and add it to GetTypeDescription and to IDataContainerHandle and ITableHandle?

Thanks, Frank

[prodehghan]

Hi,

Thanks for the follow up.

Maybe I'm not getting it, but I see some issues here.

There are two interfaces named IDataContainerHandle:

1. Dbosoft.YaNco.IDataContainerHandle
   • Is implemented by
     • Dbosoft.YaNco.Internal.FunctionHandle (indirectly through IFunctionHandle)
     • Dbosoft.YaNco.Internal.StructureHandle (indirectly through IStructureHandle)
     • Dbosoft.YaNco.Internal.TableHandle (indirectly through ITableHandle)
   • Used as a parameter for the many methods in IRfcRuntime:
     • GetTypeDescription, GetStructure, GetTable, GetString, SetString, ...
   • Stored in DataContainer class, and passed to respective IRfcRuntime methods
2. Dbosoft.YaNco.Internal.IDataContainerHandle, which inherits from the other one and:
   • Only implemented by:
     • Dbosoft.YaNco.Internal.FunctionHandle
     • Dbosoft.YaNco.Internal.StructureHandle
   • Used as aparamter for many method in Api, which are equivalent to the ones we have in IRfcRuntime.

In RfcRuntime class, the input parameter of many methods is the interface number 1, which when calling Api method, is being cast (as) to the interface number 2.

First of all, I see having these two separate interfaces problematic, causing unnecessary complications. Second, I see no reason for the second interface to not be implemented by TableHandle, as these two interfaces are exactly being used in similar situations.

[fw2568]

Sorry for the delay, but now I have looked into this problem. I added an experimental PR #264 to solve this problem by adding a new overload, but in the end I have to agree with your analysis. However, the real root problem here is that ITableHandle implements IDataContainer for simplification.

However, this makes no sense in terms of the semantic meaning of the data container, as it should only be used for fields that support reading and setting the underlying field values. However, your solution is the best option for solving the problem.

Again, thanks for your PR and your support. I will merge the PR.

[fw2568]

fixes #260

[prodehghan]

Thanks for approving the PR. I agree with you that solving the underlying issue involves a lot more work, but to just fix the bug, this PR is fine.