HiveBrain v1.2.0
Get Started
← Back to all entries
patternphpMinor

Verb conjugator for French

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
frenchforconjugatorverb

Problem

I had written with very few PHP skills a conjugator script.
It was not good, so I decided to start it from scratch with a help from a friend. I'm using the code formatter from eclipse.

Now I need your reviews and improvement tips.

I know all my long arrays with the all verbs and many more long arrays aren't a good way,but I have no clue how to put them all in a database and how then to use them.

Now a specific question:

word_stem.php In this file, the script gets the correct word_stem to building in conjugate.php function in the line:

Compiling a conjugated verb

$conjugated_verb = word_stem($infinitiveVerb, $person, $tense, $mood) . ending($person, $tense, $mood, $endingwith, $exceptionmodel, $infinitiveVerb);


They are many exception rules for many verb groups.
That's why I had to writte somethings long and complicated if conditions to find the needed changed mood-tense-person combinations, which should use the correct word_stem.

Excerpt 1 from word_stem()

if (in_array($exceptionmodel->getValue(), [
    ExceptionModel::Eler_Ele,
    ExceptionModel::Eter_Ete
]) && (($mood->getValue() === Mood::Indicatif && $tense->getValue() === Tense::Present && in_array($person->getValue(), [
    Person::FirstPersonSingular,
    Person::SecondPersonSingular,
    Person::ThirdPersonSingular,
    Person::ThirdPersonPlural
]) || $tense->getValue() === Tense::Futur) || ($mood->getValue() === Mood::Subjonctif && $tense->getValue() === Tense::Present && in_array($person->getValue(), [
    Person::FirstPersonSingular,
    Person::SecondPersonSingular,
    Person::ThirdPersonSingular,
    Person::ThirdPersonPlural
])) || ($mood->getValue() === Mood::Conditionnel && $tense->getValue() === Tense::Present) || ($mood->getValue() === Mood::Imperatif && $tense->getValue() === Tense::Present && $person->getValue() === Person::SecondPersonSingular))) {
    $word_stem = substr_replace($word_stem, 'è', - 2, 1);
}


Excerpt 2

```
if ($exceptionmodel->getValue() === Exceptio

Solution

Wrap conditions to be visible

The first order of business when having long if conditions is to wrap lines at each and every && or ||, and in general to wrap the lines
so that you are able to see the conditions without scrolling. If you have to scroll, you'll loose context straight away. This will render your code into the following:

if ($exceptionmodel->getValue() === ExceptionModel::VALOIR
    && (  ($mood->getValue() === Mood::Indicatif 
           && $tense->getValue() === Tense::Present
           && in_array($person->getValue(), 
                     [ Person::FirstPersonSingular,
                       Person::SecondPersonSingular,
                       Person::ThirdPersonSingular
                     ]))
        || $tense->getValue() === Tense::Futur
        || ($mood->getValue() === Mood::Conditionnel
            && $tense->getValue() === Tense::Present) 
        || ($mood->getValue() === Mood::Imperatif
            && $tense->getValue() === Tense::Present
            && $person->getValue() === Person::SecondPersonSingular
           )
       )
   )
{
    $word_stem = word_stem_length($infinitiveVerb, 4) . 'u';
}

if ($exceptionmodel->getValue() === ExceptionModel::VALOIR 
   && ($mood->getValue() === Mood::Subjonctif 
       && $tense->getValue() === Tense::Present
       && in_array($person->getValue(), 
                   [ 
                     Person::FirstPersonSingular,
                     Person::SecondPersonSingular,
                     Person::ThirdPersonSingular,
                     Person::ThirdPersonPlural
                   ])
      )
   ) 
{
    $word_stem = word_stem_length($infinitiveVerb, 4) . 'ill';
}


In addition to making them all visible, I've indented the different blocks as best I can. Possible bug: Do you have an parenthesis issue in the first if block after the in_array() where there seem to be missing an parenthesis? I've changed it here, but you need to reiterate it, to verify its correctness. In general if you wrap and get alternating && vs || you need to take care and verify correctness regarding ordering and grouping of conditionals.

Also note how I moved the starting brace, {, to the next line to keep it aligned with both the if and the ending brace, }. This also gives a much needed vertical space to break between conditions and the statement to be executed.

I've put the ending parenthesis, ), on a separate line as well, this comes somewhat down to personal taste, so do that if that pleases you. Or pack them together on the previous line. Just be consistent, and when dealing with long lines like in this block, try avoid having more than one condition on any line.

Precompute condition parts

Another good option can be to precompute the parts used in the condition. This has at least the following advantages:

  • Shorten the condition statement in general



  • Make it read more easily, as you clearly indicate the test condition



  • Avoid computing values more than once, i.e. $tense->getValue()



  • Simplify grouping and ordering of multiple conditions



  • Reuse of condition part in following if statements



Doing this you can end up with the following:

// Precompute condition parts
$exceptionIsValoir = $exceptionmodel->getValue() === ExceptionModel::VALOIR;

$moodVal =  $mood->getValue();
$moodIsIndicatif = $moodVal == Mood::Indicatif;
$moodIsConditionnel = $moodVal === Mood::Conditionnel; 
$moodIsSubjonctif =  $moodVal === Mood::Subjonctif;

$tenseVal = $tense->getValue();
$tenseIsPresent = $tenseVal=== Tense::Present;
$tenseIsFutur = $tenseVal === Tense::Futur;

$personVal = $person->getValue();
$personIs_2S = $personVal == Person::SecondPersonSingular;
$personIs_1S_2S_3S = in_array($personVal, [
    Person::FirstPersonSingular,
    Person::SecondPersonSingular,
    Person::ThirdPersonSingular
  ]);
$personIs_1S_2S_3S_3P = in_array($personVal, [
    Person::FirstPersonSingular,
    Person::SecondPersonSingular,
    Person::ThirdPersonSingular,
    Person::ThirdPersonPlural
  ]);

// The actual validations  
if ($exceptionIsValoir
    && (   ($moodIsIndicatif && $tenseIsPresent && $personIs_1S_2S_3S)
        || $tenseIsFutur
        || ($moodIsConditionnel && $tenseIsPresent)
        || ($moodIsImperatif && $tenseIsPresent && $personIs_2S)))
{
    $word_stem = word_stem_length($infinitiveVerb, 4) . 'u';
}

if ($exceptionIsValoir
    && ($moodIsSubjonctif && $tenseIsPresent && $personIs_1S_2S_3S_3P))
{
    $word_stem = word_stem_length($infinitiveVerb, 4) . 'ill';
}


Early returns to reduce nesting levels

The concept of early returns to reduce nesting levels, is that if you have a structure like the following with no else block on the first few levels:

```
function something() {
if first_level_condition {
if second_level_condition {
if third_level_condition and something {
do something useful
} else {
do something else
}
} // Note missing e

Code Snippets

if ($exceptionmodel->getValue() === ExceptionModel::VALOIR
    && (  ($mood->getValue() === Mood::Indicatif 
           && $tense->getValue() === Tense::Present
           && in_array($person->getValue(), 
                     [ Person::FirstPersonSingular,
                       Person::SecondPersonSingular,
                       Person::ThirdPersonSingular
                     ]))
        || $tense->getValue() === Tense::Futur
        || ($mood->getValue() === Mood::Conditionnel
            && $tense->getValue() === Tense::Present) 
        || ($mood->getValue() === Mood::Imperatif
            && $tense->getValue() === Tense::Present
            && $person->getValue() === Person::SecondPersonSingular
           )
       )
   )
{
    $word_stem = word_stem_length($infinitiveVerb, 4) . 'u';
}

if ($exceptionmodel->getValue() === ExceptionModel::VALOIR 
   && ($mood->getValue() === Mood::Subjonctif 
       && $tense->getValue() === Tense::Present
       && in_array($person->getValue(), 
                   [ 
                     Person::FirstPersonSingular,
                     Person::SecondPersonSingular,
                     Person::ThirdPersonSingular,
                     Person::ThirdPersonPlural
                   ])
      )
   ) 
{
    $word_stem = word_stem_length($infinitiveVerb, 4) . 'ill';
}
// Precompute condition parts
$exceptionIsValoir = $exceptionmodel->getValue() === ExceptionModel::VALOIR;

$moodVal =  $mood->getValue();
$moodIsIndicatif = $moodVal == Mood::Indicatif;
$moodIsConditionnel = $moodVal === Mood::Conditionnel; 
$moodIsSubjonctif =  $moodVal === Mood::Subjonctif;

$tenseVal = $tense->getValue();
$tenseIsPresent = $tenseVal=== Tense::Present;
$tenseIsFutur = $tenseVal === Tense::Futur;

$personVal = $person->getValue();
$personIs_2S = $personVal == Person::SecondPersonSingular;
$personIs_1S_2S_3S = in_array($personVal, [
    Person::FirstPersonSingular,
    Person::SecondPersonSingular,
    Person::ThirdPersonSingular
  ]);
$personIs_1S_2S_3S_3P = in_array($personVal, [
    Person::FirstPersonSingular,
    Person::SecondPersonSingular,
    Person::ThirdPersonSingular,
    Person::ThirdPersonPlural
  ]);


// The actual validations  
if ($exceptionIsValoir
    && (   ($moodIsIndicatif && $tenseIsPresent && $personIs_1S_2S_3S)
        || $tenseIsFutur
        || ($moodIsConditionnel && $tenseIsPresent)
        || ($moodIsImperatif && $tenseIsPresent && $personIs_2S)))
{
    $word_stem = word_stem_length($infinitiveVerb, 4) . 'u';
}

if ($exceptionIsValoir
    && ($moodIsSubjonctif && $tenseIsPresent && $personIs_1S_2S_3S_3P))
{
    $word_stem = word_stem_length($infinitiveVerb, 4) . 'ill';
}
function something() {
    if first_level_condition {
        if second_level_condition {
            if third_level_condition and something {
                do something useful
            } else {
                do something else
            }
        }  // Note missing else block on second level
     }  // Note missing else block on first level
}
function something() {
    if not first_level_condition
       or not second_level_condition {
        return
    }

    if third_level_condition and something {
        do something useful
    } else {
        do something else
    } 
}
// Precompute condition parts
... as before ...

// Build condition group
$c = ($moodIsIndicatif && $tenseIsPresent && $personIs_1P_2P_3P)
$c = $c || $tenseIsFutur
$c = $c || ($moodIsConditionnel && $tenseIsPresent)
$c = $c || ($moodIsImperatif && $tenseIsPresent && $personIs_2S)

if ($exceptionIsValoir && $c)
{
   $word_stem = word_stem_length($infinitiveVerb, 4) . 'u';
}
...

Context

StackExchange Code Review Q#111847, answer score: 3

Revisions (0)

No revisions yet.