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

Documentation for contains does not explain every parameter #2303

Closed
JJ opened this issue Sep 9, 2018 · 14 comments
Closed

Documentation for contains does not explain every parameter #2303

JJ opened this issue Sep 9, 2018 · 14 comments
Labels
docs Documentation issue (primary issue type)

Comments

@JJ
Copy link
Contributor

JJ commented Sep 9, 2018

The problem

From this thread in the perl6-users mailing list referring to this page

Suggestions

Read the thread and see if some of the solutions proposed fits, or else address that problem.

@JJ JJ added the docs Documentation issue (primary issue type) label Sep 9, 2018
@ToddAndMargo
Copy link

Suggested wording:

A second parameter can be added after the string to specify at which position in the string -- starting at zero -- to start looking for a match. Delimiter is a comma.

For example, "e" is the fifth character and occupies position four in the string:

say "abcdefghij".contains("e", 4); # position 4 is "e"
True

say "abcdefghij".contains("e", 5); # position 5 is "f"
False

@JJ
Copy link
Contributor Author

JJ commented Sep 10, 2018

@ToddAndMargo the documentation says

searches for $needle starting from $start.

It also shows the different forms of the multi, and examples which show that behavior:

say "Hello, World".contains(',', 3);       # OUTPUT: «True␤» 

Am I missing something here or what you suggest is already in the documentation?

@ToddAndMargo
Copy link

ToddAndMargo commented Sep 10, 2018 via email

@JJ
Copy link
Contributor Author

JJ commented Sep 10, 2018

Still don't understand what you really want here. C<$needle> and C<$start> are the positional variable names used in the function definition. Do you want the documentation to say first and second parameter instead? That is,

searches for the first argument in the string starting from the number of characters indicated by the second argument

?
What I just did is just substitute the name of the variables (which is in the documentation) by its position.

JJ added a commit that referenced this issue Sep 10, 2018
And clarifies what happens if the second argument is not present. Addresses #2303. Not closing it until everyone is happy.
@ToddAndMargo
Copy link

ToddAndMargo commented Sep 10, 2018 via email

@JJ
Copy link
Contributor Author

JJ commented Sep 10, 2018

Again, you are going to pardon me, but $needle is in the definition of the function:

multi method contains(Cool:D: Str(Cool) $needle, Cool $start? --> Bool:D)

which is in the documentation. That's how every method is defined in the documentation. It's further clarified by the examples, which are more or less the same than the examples that you suggest. Again, maybe using "first argument" and "second argument" might be clearer for a beginner, but those two arguments receive a name right there and I can't really see how that's an improvement.

JJ added a commit that referenced this issue Sep 10, 2018
Which is actually what does the heavy lifting. Refs #2303 and grateful for @ToddAndMargo for drawing our attention to this issue.
@JJ JJ closed this as completed in 10ff0d6 Sep 10, 2018
@CurtTilmes
Copy link
Contributor

CurtTilmes commented Sep 10, 2018 via email

@ToddAndMargo
Copy link

ToddAndMargo commented Sep 11, 2018 via email

@finanalyst
Copy link
Collaborator

finanalyst commented Sep 11, 2018 via email

@ToddAndMargo
Copy link

ToddAndMargo commented Sep 11, 2018 via email

@CurtTilmes
Copy link
Contributor

  1. The signature provides a concise, correct description of the calling sequence that is simply
    critical in a reference document, even though it may make the document look more complicated.

Even though it appears simpler, this:

 STR contains SUBSTR,POSITION
 STR contains SUBSTR

is an incomplete description of that calling sequence, perhaps suitable for a tutorial, but not sufficient for a reference document.

  1. Search for the existence of a SUBSTR in STR starting at or after POSITION.

Referring to this thing we search for as $substr instead of $needle could be a good idea. I'm not opposed to that at all. (I kind of like $needle for historic reasons, but can see how $substr might be clearer.)

  1. If POSITION is omitted, starts searching from the beginning of the string (0).

Except for the redundant (0), I like this too.

I'm also not opposed to using $position instead of $pos, although I prefer the abbreviation.

  1. If the SUBSTR is located, "contains" returns "True", otherwise it returns "False".

Putting the quotes around the values would be incorrect, otherwise I like this.
"True" indicates a a string, while True is a special Boolean value.

If you made it If $substr is located, returns True, otherwise returns False. I could go with this.

  1. Your example seems a little verbose, I prefer the existing examples.

  2. The existing document is missing the return type -- It should definitely get a --> Bool:D stuck at the end of the signature.

  3. The existing document seems to have inconsistent indications for coercion, sometime separate signatures for Cool/Str, but then sometimes together Cool/Int. There should be a standard, and all signatures should follow it.

  4. In the existing document, this:

Coerces the invocant (represented in the signature by Str:D: and first argument (which we are calling $needle) to Str (if it's not already, that is, the first and third multi),

I hate...

Are we really going to add such language to every single routine that takes a Str? (I do like the 'Note' with the link to traps.)

Are we really going to add "(which we are calling $blah)" to every single placeholder we refer to?

This is simply extraneous verbiage that were we to decide was helpful must be added to almost every single description...

@CurtTilmes CurtTilmes reopened this Sep 12, 2018
@ToddAndMargo
Copy link

Hi Curt,

Thank you for reopening!

I do not like $needle unless there is corresponding $haystack somewhere near by. When I first saw needle, I thought needle was yet another reference that I did not understand. This is why in the first example I posted, I used both $Needle and $Haystack. When written this way, it makes a lot of sense. And it is fun to write too.

I put quotes around True and False as I wanted to differentiate them from the spoken (English) True and the programming True. But I do see you point. How about putting them in italics?

I like to spell things out like $Position, instead of $pos as I want to make it as clear as possible. When explaining concepts, I think it is best to write them out. The programmer can shorten then to $p later if he wants.

As for the "redundant (0)", I did this on purpose. I don't always know when things start counting from one or from zero. Perl 5's match "m/" is a real offender here as they start from one. (Talk about having to rewrite a bunch of stuff when converting from P5 to P6!). Now strings in other language's or other language's functions may or may not start at zero and if you are familiar with others (I am several), it is easy to goof them up, even if you know that Perl 6 starts at zero. I am king of the goof ups.

The thing I would like to see the most in the documentation is the functions written plainly for new and regular users and not require the speciality knowledge of the developers to figure out. The developers have their own specifications sheets. Leave the docs to the rest of us riff-raff!

-T

@ToddAndMargo
Copy link

ToddAndMargo commented Sep 12, 2018

Speaking of not explaining every parameter. Where in the following does it state that there is a single return parameter and that it is of type boolean ("Bool")?

And what the heck is Int(Cool:D)? Is it an Int or a Cool and is constrained D or not?

multi method contains(Str:D: Cool:D $needle)
multi method contains(Str:D: Str:D $needle)
multi method contains(Str:D: Cool:D $needle, Int(Cool:D) $pos)
multi method contains(Str:D: Str:D $needle, Int:D $pos)

@finanalyst
Copy link
Collaborator

finanalyst commented Sep 12, 2018 via email

@JJ JJ closed this as completed in df4f027 Sep 12, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Documentation issue (primary issue type)
Projects
None yet
Development

No branches or pull requests

4 participants