Update language code parsing

[johnml1135]

Refactor for some edge cases and for cleaner understanding of what is going on.

This should align with the documentation at https://github.com/sillsdev/serval/wiki/Language-Tag-Resolution-for-NLLB%E2%80%90200.

---

This change is 

[ddaspit]

Reviewed 3 of 3 files at r1, all commit messages.
Reviewable status: all files reviewed, 8 unresolved discussions (waiting on @johnml1135)

---

src/SIL.Machine.AspNetCore/Services/LanguageTagService.cs line 12 at r1 (raw file):

    private readonly Dictionary<string, string> _threeToTwo;

    private readonly Regex _langTagPattern;

This can be static.

---

src/SIL.Machine.AspNetCore/Services/LanguageTagService.cs line 26 at r1 (raw file):

    }

    private Dictionary<string, string> InitializeDefaultScripts()

This can be static.

---

src/SIL.Machine.AspNetCore/Services/LanguageTagService.cs line 82 at r1 (raw file):

        if (!langTagMatch.Success)
            return languageTag;
        string parsed_language = langTagMatch.Groups["language"].Value;

Use camel case for variable names.

---

src/SIL.Machine.AspNetCore/Services/LanguageTagService.cs line 90 at r1 (raw file):

        // If they gave us the ISO code, revert it to the 2 character code
        if (_threeToTwo.ContainsKey(language_subtag))

SIL.WritingSystems already has a method for this purpose. It is called StandardSubtags.TryGetLanguageFromIso3Code.

---

src/SIL.Machine.AspNetCore/Services/LanguageTagService.cs line 94 at r1 (raw file):

        // There are a few extra conversions not in SIL Writing Systems that we need to handle
        if (StandardLanguages.ContainsKey(language_subtag))

You should use TryGetValue. It saves a lookup and is more idiomatic C#.

---

src/SIL.Machine.AspNetCore/Services/LanguageTagService.cs line 97 at r1 (raw file):

            language_subtag = StandardLanguages[language_subtag];

        if (StandardSubtags.RegisteredLanguages.Contains(language_subtag))

Use TryGet here.

---

src/SIL.Machine.AspNetCore/Services/LanguageTagService.cs line 103 at r1 (raw file):

        Group scriptGroup = langTagMatch.Groups["script"];
        string? script = null;
        if (_defaultScripts.ContainsKey(language_subtag))

I would reverse these three if statements and use else if instead. It will save unnecessary lookups. Also use TryGetVaue.

---

tests/SIL.Machine.AspNetCore.Tests/Services/LanguageTagServiceTests.cs line 93 at r1 (raw file):

    [Test]
    public void ConvertToFlores200Code_English()

I would rename this test. It isn't specific to English. It is really testing an invalid language tag where the ISO 639-3 code is used instead of the ISO 639-1 code.

[johnml1135]

src/SIL.Machine.AspNetCore/Services/LanguageTagService.cs line 12 at r1 (raw file):

Previously, ddaspit (Damien Daspit) wrote…

This can be static.

Done.

[johnml1135]

src/SIL.Machine.AspNetCore/Services/LanguageTagService.cs line 26 at r1 (raw file):

Previously, ddaspit (Damien Daspit) wrote…

This can be static.

Wouldn't you need to Sldr.InitializeLanguageTags(); first?

[johnml1135]

src/SIL.Machine.AspNetCore/Services/LanguageTagService.cs line 82 at r1 (raw file):

Previously, ddaspit (Damien Daspit) wrote…

Use camel case for variable names.

Done.

[johnml1135]

src/SIL.Machine.AspNetCore/Services/LanguageTagService.cs line 90 at r1 (raw file):

Previously, ddaspit (Damien Daspit) wrote…

SIL.WritingSystems already has a method for this purpose. It is called StandardSubtags.TryGetLanguageFromIso3Code.

Done.

[johnml1135]

src/SIL.Machine.AspNetCore/Services/LanguageTagService.cs line 94 at r1 (raw file):

Previously, ddaspit (Damien Daspit) wrote…

You should use TryGetValue. It saves a lookup and is more idiomatic C#.

Done.

[johnml1135]

src/SIL.Machine.AspNetCore/Services/LanguageTagService.cs line 97 at r1 (raw file):

Previously, ddaspit (Damien Daspit) wrote…

Use TryGet here.

Done.

[johnml1135]

src/SIL.Machine.AspNetCore/Services/LanguageTagService.cs line 103 at r1 (raw file):

Previously, ddaspit (Damien Daspit) wrote…

I would reverse these three if statements and use else if instead. It will save unnecessary lookups. Also use TryGetVaue.

Done.

[johnml1135]

tests/SIL.Machine.AspNetCore.Tests/Services/LanguageTagServiceTests.cs line 93 at r1 (raw file):

Previously, ddaspit (Damien Daspit) wrote…

I would rename this test. It isn't specific to English. It is really testing an invalid language tag where the ISO 639-3 code is used instead of the ISO 639-1 code.

Done.

[codecov-commenter]

Codecov Report

Attention: 3 lines in your changes are missing coverage. Please review.

Files	Coverage Δ
....Machine.AspNetCore/Services/LanguageTagService.cs	92.53% <90.90%> (+3.17%)	⬆️

📢 Thoughts on this report? Let us know!.

[ddaspit]

Reviewed 2 of 2 files at r2, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @johnml1135)

---

src/SIL.Machine.AspNetCore/Services/LanguageTagService.cs line 26 at r1 (raw file):

Previously, johnml1135 (John Lambert) wrote…

Wouldn't you need to Sldr.InitializeLanguageTags(); first?

It will still be called first. Making the method static just means that the method is associated with the class instead of an instance. In any case, I would move Sldr.InitializeLanguageTags(); inside InitializeDefaultScripts.

[ddaspit]

Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @johnml1135)

---

src/SIL.Machine.AspNetCore/Services/LanguageTagService.cs line 10 at r2 (raw file):

    private readonly Dictionary<string, string> _defaultScripts;

    private static readonly Regex _langTagPattern = new Regex(

I would rename this to LangTagPattern, since it is now a constant.

[johnml1135]

src/SIL.Machine.AspNetCore/Services/LanguageTagService.cs line 10 at r2 (raw file):

Previously, ddaspit (Damien Daspit) wrote…

I would rename this to LangTagPattern, since it is now a constant.

Done.

[johnml1135]

src/SIL.Machine.AspNetCore/Services/LanguageTagService.cs line 26 at r1 (raw file):

Previously, ddaspit (Damien Daspit) wrote…

It will still be called first. Making the method static just means that the method is associated with the class instead of an instance. In any case, I would move Sldr.InitializeLanguageTags(); inside InitializeDefaultScripts.

Done.

[ddaspit]

Reviewed 1 of 1 files at r3, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @johnml1135)