pyret-lang icon indicating copy to clipboard operation
pyret-lang copied to clipboard

change string-index-of to not return -1

Open shriram opened this issue 2 years ago • 2 comments

Currently, string-index-of returns -1 in the error case, which violates DCIC and good programming practices.

@blerner thinks it might be because it was useful for Bootstrap, but @schanzer confirms that Bootstrap never uses it.

@jpolitz is concerned about breaking backward compatibility.

Can we:

  1. Add a new function with a different name that raises an error.
  2. Update the docs of string-index-of to say it's been deprecated and is only there for legacy reasons.
  3. Add examples for the new function that have multi-character strings? All the examples currently have only one character.

Suggested name: substring-at.

shriram avatar Jan 01 '23 16:01 shriram

Instead of having the new function raise an error, can we return an option type? There are situations (concretely, in a cs111 homework) in which it's useful to search for the first substring that matches a pattern if it exists, and if it doesn't, do something else in the function body, rather than erroring out.

sreshtaaa avatar Jan 19 '23 20:01 sreshtaaa

I guess we need both? substring-at-opt and substring-at-exn?

shriram avatar Jan 19 '23 23:01 shriram

I don't like those names; to me, substring-at should take a string and a number and return the substring at that index (which then begs the question of the length of the desired substring...) I'd pick string-find-index instead. I'm not sure to distinguish the two variants, though: The only parallel design we have is dict.get (Option) vs dict.get-value (error-throwing), and name-to-color (Option) vs color-named (error-throwing). So maybe we have string-find-index (option) and string-get-index (error-throwing)?, with the idea that "finding" might not find, but "get" demands a result "or else"?

blerner avatar May 02 '24 16:05 blerner

I like this naming convention. Does it sense to apply it more widely?

shriram avatar May 18 '24 21:05 shriram

"This" is ambiguous -- do you mean your proposed naming convention (-at-opt and -at-exn) or mine (-find-index and -get-index)?

blerner avatar May 18 '24 21:05 blerner

Yours!

shriram avatar May 18 '24 22:05 shriram