Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fixed a bug when used the function "in_array" #25

Closed

Conversation

jkhhjkhiukhjhukjhkh
Copy link

@jkhhjkhiukhjhukjhkh jkhhjkhiukhjhukjhkh commented Jul 6, 2018

I used your code to convert a PHP array to a .toml file, but when I used TomlBuilder class to build my .toml file, the program always threw the DumpException.
When my array included the zero element, your expression return true, made program stop.

if (in_array($key, $this->keyListArryOfTables)) {
    throw new DumpException(
        sprintf('The table %s has already been defined as previous array of tables', $key)
    );
}

I debugged it and found this bug.document of in_array

I added the third param true in your code, I guessed it would always suit for your scene.
In that case, I could build my program easily.

Additionally, there were some mistakes in the code. For example, the variable $this->keyListArryOfTables was a key=>value array, so I thought array_key_exists would be more correct than in_array.

if (in_array($key, $this->keyListArryOfTables, true)) {
        throw new DumpException(
        sprintf('The table %s has already been defined as previous array of tables', $key)
    );
}
if (false == isset($this->keyListArryOfTables[$key])) {
        $this->keyListArryOfTables[$key] = 0;
        $this->addKeyToKeyList($key);
    } else {
        $counter = $this->keyListArryOfTables[$key] + 1;
        $this->keyListArryOfTables[$key] = $counter + 1;
}

@jkhhjkhiukhjhukjhkh jkhhjkhiukhjhukjhkh changed the title fixed a bug when use the function "in_array" fixed a bug when used the function "in_array" Jul 6, 2018
@yosymfony
Copy link
Owner

Could you describe your use-case with an Toml example?

@jkhhjkhiukhjhukjhkh
Copy link
Author

jkhhjkhiukhjhukjhkh commented Jul 9, 2018

Please see my new PR.
In my branch builder, if you checkout the TomlBuiler.php to your vision 7c60cdf, you will find the program throw the exception, You can easy debug it.

if (in_array($key, $this->keyListArryOfTables)) {
    var_dump($key);
    var_dump($this->keyListArryOfTables);
    var_dump(in_array($key, $this->keyListArryOfTables));
    var_dump(in_array($key, $this->keyListArryOfTables, true));
    var_dump(array_key_exists($key, $this->keyListArryOfTables));
    throw new DumpException(
        sprintf('The table %s has already been defined as previous array of tables', $key)
    );
}

The output:

string(5) "a2.b1"
array(1) {
  'a2' =>
  int(0)
}
bool(true)
bool(false)
bool(false)
Fatal error: Uncaught Yosymfony\Toml\Exception\DumpException: The table a2.b1 has already been defined as previous array of tables in TomlBuilder.php on line 294

yosymfony added a commit that referenced this pull request Aug 1, 2018
…he PR #25 has been solved.

The test file `TomlBuilderTest` has been refactored for readability
@yosymfony
Copy link
Owner

I just resolved your issue. The problem was I was looking for a key in the array values. I just committed that change. In a few days, I'll do a release.

@yosymfony yosymfony closed this Aug 8, 2018
@yosymfony
Copy link
Owner

yosymfony commented Aug 8, 2018

Thank you for this PR. It'll be fixed in v1.0.4

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants