Fork me on GitHub
Subscribe 3

Ticket #596 (fixed enhancement)

Language class tidy

  • Created: 2012-01-14 16:39:56
  • Reported by: Reines
  • Assigned to: Franz
  • Milestone: 2.0-alpha1
  • Component: localization
  • Priority: normal

There are a few parts of the language class which I'm not quite sure about - they may have been designed this way for a reason, but I don't see it.

My main question is, what is the need for a default language and the current language? Is it falling back to the default language for non-existing strings? If so I'm not convinced this is the best approach, especially at runtime.

Why is there a need for a setLanguage function? Wouldn't it be much cleaner to pass the required language into the constructor? Otherwise it becomes possible to have a mix of multiple languages inside 1 Flux_Lang instance, which is horrible.

This class is part of FluxBB and not part of an external module. We can safely hardcode the path to the language directory here, allowing us to remove the setLanguageDirectory function - which similarly would be messy if called at the wrong point in time (potentially even more likely considering it's static).

Minor point - the language list should be cached (even just using the static keyword) for use in the languageExists function.

History

Franz 2012-01-18 13:26:30

Commit 13475fd to fluxbb fluxbb-2.0

#596: Cache list of language packs in Flux_Lang class.

Franz 2012-01-18 13:36:56

So with that commit I have taken care of caching the list of languages. I did it in a static method variable - is that what you meant?

Reines wrote:

Why is there a need for a setLanguage function? Wouldn't it be much cleaner to pass the required language into the constructor? Otherwise it becomes possible to have a mix of multiple languages inside 1 Flux_Lang instance, which is horrible.

Ah, probably a very good point. I could take care of this by simply calling the languageExists() function which itself calls getLanguageList() to make sure the requested language exists. Is that ok or too much overhead?

Reines wrote:

My main question is, what is the need for a default language and the current language? Is it falling back to the default language for non-existing strings? If so I'm not convinced this is the best approach, especially at runtime.

Well, the idea is that, when generating the cached language files for a language, all missing strings are filled with the respective strings from the default language. So it does only do that once at runtime, namely when generating the cache file. Does that answer your question?

Another interesting question regarding this matter is how and when those cache files should be regenerated and whether we can somehow notice that.

Franz 2012-02-15 17:17:15

How would you propose setting the default language then? As static variable?

Franz 2012-02-15 18:30:03

  • Owner changed from Reines to Franz.

Franz 2012-02-21 13:25:49

Commit cb5244f to fluxbb fluxbb-2.0

#596: Move setting of language to constructor of Flux_Lang class.

Franz 2012-02-21 13:28:56

Commit f9c019b to fluxbb fluxbb-2.0

#596: Hardcode language file directory in Flux_Lang class (and use PUN_ROOT to determine correct location).

arw 2012-04-07 19:18:51

Another interesting question regarding this matter is how and when those cache files should be regenerated and whether we can somehow notice that.

the only way I see would be using filemtime to compare it with when cache was done

Why is there a need for a setLanguage function? Wouldn't it be much cleaner to pass the required language into the constructor? Otherwise it becomes possible to have a mix of multiple languages inside 1 Flux_Lang instance, which is horrible.

Ah, probably a very good point. I could take care of this by simply calling the languageExists() function which itself calls getLanguageList() to make sure the requested language exists. Is that ok or too much overhead?

since language common is load just after $lang object, it would still fail on gettext->parse ( if not in cache and file doesn't exist ) so testing if language exist may  no  be needed

Comment edited 1 times (Diff)

Franz 2012-04-07 19:53:34

Commit 718aa2f to fluxbb fluxbb-2.0

#596: Set language in constructor of language object in install.php, too.

Franz 2012-05-23 21:49:50

  • Status changed from open to fixed.

Done for good now.