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

fix sample code of Array#<< #1646

Closed
wants to merge 1 commit into from
Closed

fix sample code of Array#<< #1646

wants to merge 1 commit into from

Conversation

selmertsx
Copy link

@selmertsx selmertsx commented Jun 12, 2017

nice to meet you!
I felt something wrong with the sample code of the <<
in the array.c, so I sent you a PR.

Currently, the description of << is as follows.

Append---Pushes the given object on to the end of this array. This
expression returns the array itself, so several appends
may be chained together.

However, we can get this result without updating the receiver.
For example, this result can be obtained also when writing code as follows.

class Array
  def <<(other)
    self + [other]
  end
end

So, I will suggest another sample code

a = [1, 2]
a << "c" << "d" << [ 3, 4 ]
a #=> [1, 2, "c", "d", [3, 4]]

Please consider this proposal.

@k0kubun
Copy link
Member

k0kubun commented Jun 12, 2017

Of course you can return the same result without updating receiver. But "this expression returns the array itself, so several appends may be chained together" doesn't deny "we can get this result without updating the receiver".

The previous code isn't broken and the changed code calls the same method. So it didn't make sense to me. Could you rephrase why you "felt something wrong with the sample"? Please provide the source code in which the sample is broken. With Array#<< definition you provided, [ 1, 2 ] << "c" << "d" << [ 3, 4 ] works.

@selmertsx
Copy link
Author

Thank you for commenting to my PR 😁
And I'm sorry that my English makes you misunderstood 😅

With the current sample code, I think that it can not fully
describe "This expression returns the array itself".

The current sample code describes "chained togeter"
by [1, 2] << "c" << .... but it doesn't describe
the [1, 2] array is changed.

The proposed sample code also describes [1, 2] is changed.
I think that it describes "This expression returns the array itself".

How about it?

@k0kubun
Copy link
Member

k0kubun commented Jun 12, 2017

It made sense. You want to make it clear that a is modified.

Then, since your code doesn't explain << "expression returns the array itself", how about this? (This version is similar to Array#push's one)

     a = [ 1, 2 ]
     a << "c" << "d" << [ 3, 4 ]
             #=>  [ 1, 2, "c", "d", [ 3, 4 ] ]
     a
             #=>  [ 1, 2, "c", "d", [ 3, 4 ] ]

There was a possibility of misunderstanding
in the previous sample code.
@selmertsx
Copy link
Author

your code doesn't explain << "expression returns the array itself", how about this? (This version is similar to Array#push's one)

Surely!
I fixed sample code as you taught me.
What about you?

@hsbt hsbt closed this in a100bf3 Jun 12, 2017
@selmertsx selmertsx deleted the fix-sample-code-in-array branch June 12, 2017 11:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
2 participants