Lucas_Werkmeister_WMDE added a comment.

  In T345083#9357025 <https://phabricator.wikimedia.org/T345083#9357025>, 
@Michael wrote:
  
  > In T345083#9356947 <https://phabricator.wikimedia.org/T345083#9356947>, 
@Michael wrote:
  >
  >> In T345083#9356750 <https://phabricator.wikimedia.org/T345083#9356750>, 
@Lucas_Werkmeister_WMDE wrote:
  >>
  >>> [...]
  >>> Perhaps it would be more honest to just have a boolean flag for “is this 
language name used for terms or not” – after all, that’s all we would use the 
context for, too. We can still change it to a more fine-grained context later 
if we really need it.
  >>
  >> I guess either that or have a `TermLanguageNameLookup` produced by the 
factory. But for this single problem, I think I would still go for the boolean 
flag. We can always refactor it later when we get an usecase that warrants it.
  >
  > Ah, I would have expected this to just be an evil `public function getName( 
string $languageCode, bool $forTerms = false ): string`. (Which could be 
slightly less evil if we were already on PHP 8)
  
  That’s what I had in mind at first too, but then I realized that a constant 
could make it more readable.
  
  > Looking at Change name of mul language when used for terms (I7dbd94be) 
<https://gerrit.wikimedia.org/r/c/mediawiki/extensions/Wikibase/+/977258>, I 
wonder if it would not be better to have two new public methods on 
`LanguageNameLookup` like `getNameForTerms( string $languageCode )` and 
`getNameNotForTerms( string $languageCode )`. `getNameNotForTerms()` would then 
just directly call the now private `getName()` method and nothing else. 
`getNameForTerms()` would check if `$languageCode === 'mul'` and based on that 
either return a special name or also directly return the result of `getName()`.
  >
  > What do you think?
  
  Perhaps `getNameForTerms()` is nicer than `getName( $languageCode, 
LanguageNameLookup::FOR_TERMS )`, yeah. (It’s shorter, at least.) 
`getNameNotForTerms()` sounds strange, though. What if we just keep `getName()` 
public and add `getNameForTerms()` on top of it? This doesn’t force callers to 
choose between one or the other like the other approaches do (callers that use 
`getName()` can keep using it without even realizing the interface changed), 
but I think that’s okay (the meaning of `getName()` would be the same as 
before, and we already have Gerrit changes updating all the callers, so I don’t 
think we’re missing any).

TASK DETAIL
  https://phabricator.wikimedia.org/T345083

EMAIL PREFERENCES
  https://phabricator.wikimedia.org/settings/panel/emailpreferences/

To: Lucas_Werkmeister_WMDE
Cc: Michael, Lucas_Werkmeister_WMDE, Epidosis, Amire80, Mahir256, Nikki, 
Lydia_Pintscher, Sarai-WMDE, ItamarWMDE, Aklapper, Manuel, 
Danny_Benjafield_WMDE, Isabelladantes1983, Themindcoder, Adamm71, Jersione, 
Hellket777, LisafBia6531, Astuthiodit_1, 786, Biggs657, karapayneWMDE, 
Invadibot, maantietaja, Juan90264, Alter-paule, Beast1978, Un1tY, Akuckartz, 
Hook696, Kent7301, joker88john, CucyNoiD, Nandana, Gaboe420, Giuliamocci, 
Cpaulf30, Lahi, Gq86, Af420, Bsandipan, GoranSMilovanovic, QZanden, 
LawExplorer, Lewizho99, Maathavan, _jensen, rosalieper, Neuronton, Scott_WUaS, 
Wikidata-bugs, aude, Mbch331
_______________________________________________
Wikidata-bugs mailing list -- [email protected]
To unsubscribe send an email to [email protected]

Reply via email to