SAP / styleguides

This repository provides SAP style guides for coding and coding-related topics.
Other
1.68k stars 444 forks source link

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

Closed BaerbelW closed 6 months ago

BaerbelW commented 8 months ago

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.

ConjuringCoffee commented 8 months ago

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

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

BaerbelW commented 8 months ago

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 commented 8 months ago

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 commented 8 months ago

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 commented 8 months ago

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 commented 8 months ago

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 commented 6 months ago

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