5
\$\begingroup\$

I need to change some array values in a 2-dimensional array for some cases. Sometimes the all values from an child array and sometimes only some values. In this cases I should only change the variable $stamm which $stamm2.

This code is the statement of an if condition, where all relevant verbs for this changings is mentioned.

$verb could be one over 25.000 french verbs. For this example you could use 'acheter'.

  $array = array( 
  array('e', 'es', 'e', 'ons', 'ez', 'ent'), array('ais', 'ais', 'ait', 'ions', 'iez', 'aient'), array('ai', 'as', 'a', 'âmes', 'âtes', 'èrent'), array('erai', 'eras', 'era', 'erons', 'erez', 'eront'), array('e', 'es', 'e', 'ions', 'iez', 'ent'), array('asse', 'asses', 'ât', 'assions', 'assiez', 'assent'), array('erais', 'erais', 'erait', 'erions', 'eriez', 'eraient'), array('e', 'ons', 'ez'), array('XXX'), array('ant')); 

My short old code:

$stamm = substr("$verb", 0, - 2);    
$stamm2 = substr_replace($stamm, '<u>è</u>', -2, 1);

$array[0][0] = $array[0][2] = $array[4][0] = $array[4][2] = $array[7][0] = $stamm2.'e';
$array[0][1] = $array[4][1] = $stamm2.'es';
$array[0][5] = $array[4][5] = $stamm2.'ent';

My new improved code:

$stamm = substr("$verb", 0, - 2);    
$stamm2 = substr_replace($stamm, '<u>è</u>', -2, 1);

$zeiten = array(0, 3, 4, 6, 7);        
foreach($zeiten as $zeit) { 
    foreach($array[$zeit] as $person => $value) {               
        if ((($zeit == 0  || $zeit == 4) && !in_array($person,array(3,4))) || ($zeit == 3 || $zeit == 6) || ($zeit == 7 && $person==0)) 
        $array[$zeit][$person] = str_replace($stamm, $stamm2, $value);      
        }   
    } 

Is this the the new code the best solution? Is it possible to improve the if condition?

\$\endgroup\$
6
  • 1
    \$\begingroup\$ Please provide a more context on the problem you are trying to solve and what values are in $array, $stamm and $stamm2. \$\endgroup\$ Commented Jun 10, 2015 at 8:53
  • \$\begingroup\$ @Nobody I have edited my question. \$\endgroup\$
    – Grischa
    Commented Jun 10, 2015 at 9:06
  • 1
    \$\begingroup\$ It is still missing the variable $array. To make it more clear: Preferably your code should be runnable as is. As far as I can see you should provide example values for $array and $verb and would be fine. \$\endgroup\$ Commented Jun 10, 2015 at 9:18
  • \$\begingroup\$ @Nobody Their is also an array_walk function to combine $array and $stamm. The code is workung for both versions. \$\endgroup\$
    – Grischa
    Commented Jun 10, 2015 at 9:25
  • 1
    \$\begingroup\$ You are still missing an example value for $verb and the overall description of what you are trying to do. My assumption is: You are trying to build a function that gives all possible inflections of a given verb according to french language. Is that right? \$\endgroup\$ Commented Jun 10, 2015 at 9:31

1 Answer 1

7
\$\begingroup\$

Let me first review this from an readability point of view, then answer your question and an proposed solution.

Naming

Your variable names could be more descriptive.

  • $array -> (maybe) $conjugations (german: $konjugationen)
  • $stamm -> $word_stem (german: $wort_stamm)
  • $zeiten -> $tenses (german: $zeitformen)?

Structure

The most intriguing part for me is $array's structure. As I understand it, it is a 2D table of conjugations of the $verb where rows have the same tense and columns the same person. This should be somehow reflected in the data structure because it is not obvious to the viewer. Maybe you should even implement a class for that.

Comparison

Maybe it has to do with my lack of domain knowledge (I don't speak french) but I find both solutions unreadable. This is enforced by the naming problems.

The second one looks slightly more general with regard to reuse but the if portion is not very flexible.

Improvement

As I said, I have no idea about french but I suppose this holds true:

Each verb can be conjugated for person and tense. There is a set of rules that defines how this happens for each person and tense depending on the verb's form and a set of exemptions from those rules.

The solution to the problem consists of:

  1. Classifying the verb as to which rule/exemption to apply
  2. applying the rule

To me this sounds like 1. could be solved by a factory that creates strategy instances for 2.

$conjugator = get_conjugator($verb, $tense, $person);
$conjugated_verb = $conjugator->conjugate($verb);

Maybe the exceptions/rules are even partial per tense or person. This would mean you would have to make this distinction as well and produce a separate conjugator for each tense, person, verb triple.

Now the code you have posted is pretty much the part that goes into the implementation of a conjugator. And seeing that there could be many of them it would be nice to make implementing them as simple as possible.

Their interface would probably consist of a function that detects whether they are applicable (the if condition in your case) and a function that does the actual flexion of the input (the if's then code block).

\$\endgroup\$
7
  • 1
    \$\begingroup\$ I would even go as far to suggest that OP implement a Trie data structure. It is perfect for this. \$\endgroup\$ Commented Jun 10, 2015 at 12:13
  • 1
    \$\begingroup\$ @EvanBechtol: I am not sure if it is a pure string matching problem. It probably is more complex. \$\endgroup\$ Commented Jun 10, 2015 at 12:20
  • \$\begingroup\$ This would however be a really nice usecase for a Trie. Good review! \$\endgroup\$
    – Pinoniq
    Commented Jun 17, 2015 at 20:24
  • \$\begingroup\$ @Nobody I have a long time nothing to hear from you. Maybe you have now a little bit time to finish the conjugation script together. \$\endgroup\$
    – Grischa
    Commented Mar 12, 2016 at 1:07
  • 1
    \$\begingroup\$ @Grischa: I am still swamped with work but I hope to have more time available soon. I will contact you when I am available. \$\endgroup\$ Commented Mar 12, 2016 at 12:09

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.