Skip to content

Fixed and simplified array_replace description #1470

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

Closed
wants to merge 1 commit into from

Conversation

cartok
Copy link

@cartok cartok commented Mar 18, 2022

Relates to: php/doc-de#76

<function>array_replace</function> is not recursive
</para>
<para>
<function>array_replace</function> creates a new array by joining all given arrays in given order.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't that be the description of array_merge()? It is certainly not what array_replace() is doing; see e.g. https://3v4l.org/XfXpa.

It seems to me that the current description is a bit verbose, but basically correct; it is only misleading, that the description suggests that the first array would be mutated. Maybe similar wording as for str_replace() might be used.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea ok, "joining" is propably not the best fitting word here.

What about this?

creates a new array by joining all given arrays in given order, replacing values of already existing keys.

In the end the arrays are joined but it goes by keys, not indices. They not are just concatenated like in array_merge

<function>array_replace</function> is not recursive
</para>
<para>
<function>array_replace</function> creates a new array by joining all given arrays in given order, replacing values of already existing keys.
Copy link
Contributor

@Crell Crell Jun 27, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
<function>array_replace</function> creates a new array by joining all given arrays in given order, replacing values of already existing keys.
<function>array_replace</function> creates a new array and assigns an item into it for each key in each of the provided arrays. If a key appears in multiple input arrays, the latter array's value is used.

How about this?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For me its implicit that the latter arrays value is used from the former short description. Another thing is that it does not only assigns keys to the new array

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A little redundancy here is fine, especially as it's in a different section. Good point on the key/value question. I've updated the suggestion comment above.

Copy link
Member

@cmb69 cmb69 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would be rendered as:

Screenshot 2022-12-26 145520

This doesn't look like any improvement to me.

@cartok
Copy link
Author

cartok commented Dec 27, 2022

This would be rendered as:

Screenshot 2022-12-26 145520 Screenshot 2022-12-26 145520

This doesn't look like any improvement to me.

Why not, formatting / display or content?

@cmb69
Copy link
Member

cmb69 commented Dec 27, 2022

Why not, formatting / display or content?

Almost everything. If you describe something, it doesn't make much sense to start telling what it is not (let alone that a sentence should end with a period, and there rarely is the need to have single sentence paragraphs). Furthermore, "joining" might easily mislead readers to associate that with https://www.php.net/join (yeah, that function is still a thing).
In my opinon, @Crell's suggestion as well as my suggestion are better than what we have here.

@cartok
Copy link
Author

cartok commented Dec 27, 2022

Why not, formatting / display or content?

Almost everything. If you describe something, it doesn't make much sense to start telling what it is not (let alone that a sentence should end with a period, and there rarely is the need to have single sentence paragraphs). Furthermore, "joining" might easily mislead readers to associate that with https://www.php.net/join (yeah, that function is still a thing). In my opinon, @Crell's suggestion as well as my suggestion are better than what we have here.

If you think so. I rather want to read short facts and not complicated stories on such a simple thing. Feel free to close it.

jimwins added a commit to jimwins/doc-en that referenced this pull request Sep 29, 2024
jimwins added a commit that referenced this pull request Oct 1, 2024
@jimwins jimwins closed this Oct 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants