Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add ALV table for BAL message details #57

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

chriss158
Copy link

This PR adds the possibility to show a ALV table for message details of a BAL message.

tempsnip

image

Adds the possibility to show a ALV table for message details of a BAL message
@fabianlupa
Copy link
Owner

Hi Chris,

thanks for the PR! I'll review it this weekend.

@fabianlupa
Copy link
Owner

Hi @chriss158, some thoughts:

  • Is prepare_table-iv_table_struc_type_name necessary? Is a DDIC structure technically needed? Otherwise as RTTS is already used it should be quite easy to just take the line type of it_table.
  • Can the callback form / function module be replaced by something more modern, like an OO event or an interface implementation? As kind of a facade for the standard callback. What do you think?
  • Do the other new methods need to be public / in the bal logger class at all? If it's display logic it could also just be moved to a local class in the function group. Or is it supposed to be reused by some other implementation using a non default callback?

@chriss158
Copy link
Author

Thanks for the review @fabianlupa

Is prepare_table-iv_table_struc_type_name necessary? Is a DDIC structure technically needed? Otherwise as RTTS is already used it should be quite easy to just take the line type of it_table.

Yes, sadly. Because i need to import the exported table data. See here:

IMPORT mt_msg_details TO <lt_msg_details> FROM DATABASE bal_indx(al) ID lv_lognumber.

I need to define a data type before i can get the data itself. The IMPORT/EXPORT is hard to use when you work with ABAP generics.

Can the callback form / function module be replaced by something more modern, like an OO event or an interface implementation? As kind of a facade for the standard callback. What do you think?

Yeah that's also something i don't like about the callback feature of BAL. I need to use the function module, because the BAL callback feature only supports form callbacks and function module callbacks. Maybe we can use the function module as an interface to a OO object. But i need to think about that one.

Do the other new methods need to be public / in the bal logger class at all? If it's display logic it could also just be moved to a local class in the function group. Or is it supposed to be reused by some other implementation using a non default callback?

I made it public because i need to create the table type in the callback function module and also in the class object itself. Maybe we can create some kind of utility class for such globally used methods.

It would be much easier if i could get the current class object reference into the function module. But right now i have no idea how i could achieve that.

@fabianlupa
Copy link
Owner

fabianlupa commented Oct 17, 2023

I need to define a data type before i can get the data itself. The IMPORT/EXPORT is hard to use when you work with ABAP generics.

I see what you mean. I had a go at a POC but you'd have to serialize the type descriptors and import them first from memory, do the CREATE DATA and then import the actual data. I don't think that's worth pursuing.

I made it public because i need to create the table type in the callback function module and also in the class object itself. Maybe we can create some kind of utility class for such globally used methods.
It would be much easier if i could get the current class object reference into the function module. But right now i have no idea how i could achieve that.

Some refactoring could be done but I would suggest doing so after the initial PR. The feature is quite niche and I doubt this library sees much use nowadays anyway. What I do not want to lose is 7.40 compatibility though. I'll mark some lines where adjustment is required. Sorry for the delays.

Copy link
Owner

@fabianlupa fabianlupa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually it's just this one line I think.

CLEAR mr_table.
CLEAR mv_table_struc_type_name.
mv_add_table_to_next_message = abap_false.
RAISE EXCEPTION NEW zcx_alog_logging_failed( ix_previous = lx_bal
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

7.40 requires RAISE EXCEPTION TYPE

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants