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

[Use | to assemble text] Is a caveat needed in case of translatable texts? #349

Closed
BaerbelW opened this issue Mar 6, 2024 · 7 comments
Closed
Labels
Clarity of Text The issue or pull request helps to improve the clarity of the text clean-abap

Comments

@BaerbelW
Copy link

BaerbelW commented Mar 6, 2024

Relevant sections of the style guide
https://github.com/SAP/styleguides/blob/main/clean-abap/CleanABAP.md#use--to-assemble-text

Question
I've tried applying this rule but can only do so rarely as we need our texts to be translatable. This does work with either the CONCATENATE keyword or using "&&" to string the bits and pieces together but not with the Clean ABAP rule as far as I can tell. Should this perhaps be spelled out as a restriction of when it can be used and when not?

Note: in most cases I've been working with this format for texts: 'Some text...'(001)' and not text-001 so that at least the English text is used when a translation is missing.

@BaerbelW BaerbelW added Clarity of Text The issue or pull request helps to improve the clarity of the text clean-abap labels Mar 6, 2024
@ConjuringCoffee
Copy link
Contributor

ConjuringCoffee commented Mar 6, 2024

Maybe I'm missing something, but does this not work...?

DATA(lv_test) = |{ 'Some text'(001) } more text|.

@BaerbelW
Copy link
Author

BaerbelW commented Mar 6, 2024

Thanks for the quick reply! That might have been a combination I hadn't tried but did now and it seems to work in our NW 7.50 system. Just not really sure, what the advantage over concatenation with "&&" is which is - to me - much faster to type and interpret than the version with "|" and "{ }".

How about expanding the code example with the version for translatable text?

@bjoern-jueliger-sap
Copy link
Member

Translatable text often is a bit different, I'd argue, in that you probably shouldn't assemble it with either of those methods in the first place, but use message texts with placeholders instead, because in other languages the order of the text fragments might be different or certain grammatical constructions work differently.

For instance, non-translatably I might construct a text like

  data(str) = |Customer { customer }'s total amount owed is { amount }.|.

but if you want to make this translatable, the correct way isn't to make translatable texts of the fragments

  " Don't do this
  data(str) = |{ 'Customer'(001) } { customer }{ '''s total amount owed is'(002) } { amount }.|.

but instead to create a message text Customer &1's total amount owed is &2. and then use that with

  message ... with customer amount into data(str).

This isn't really about readability of the code but primarily about getting good translations: While in English the possessive marker 's appears only in text-002, and text-001 bears no case marker, in many languages the "Customer" in text-001 would need to be declined in agreement with the possessive case of the customer's name - but someone just translating text-001 can't know that (and you might even be tempted to reuse text-001 in other contexts where "Customer" appears in English but is not related to a possessive at all).

@BaerbelW
Copy link
Author

BaerbelW commented Mar 6, 2024

Hi Björn,
thanks for the detailed feedback! Yes, translatable texts and whether or not a simple concatenation will do or a proper message is better will depend a lot on context. I often use the concatenation for things like header information in ALV-output and there a messages would be a bit of an "overkill" as grammer or sentence structure doesn't play a role in something like this:

CONCATENATE '# of sales-orgs:'(c01) lv_cnt_txt INTO lv_text.

For other kind of text, I am usually working with messages where that makes sense.

@bjoern-jueliger-sap
Copy link
Member

Yeah, I think in such a simple case there isn't a lot of reasons to strictly follow the rule about using |. Still, I personally would do it because it's good to get into the habit of using it, anyway.

@pokrakam
Copy link
Contributor

Just a thought: You can try to find a suitable DDIC domain for ALV headers and the likes, or there's no harm in creating one. It's possibly more efficient and definitely more reusable than a translatable text element.

@bjoern-jueliger-sap
Copy link
Member

Since there haven't been any other arguments for a while, people seem to be generally fine with the rule in the guide staying as it is - feel free to reopen the discussion if there aren't any other edge cases to consider

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Clarity of Text The issue or pull request helps to improve the clarity of the text clean-abap
Projects
None yet
Development

No branches or pull requests

4 participants