patternphpMinor
Verb conjugator for French
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
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
Excerpt 2
```
if ($exceptionmodel->getValue() === Exceptio
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
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:
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
Also note how I moved the starting brace,
I've put the ending parenthesis,
Precompute condition parts
Another good option can be to precompute the parts used in the condition. This has at least the following advantages:
Doing this you can end up with the following:
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
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 linesso 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
ifstatements
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.