brownplt / pyret-lang

The Pyret language.
Other
1.07k stars 110 forks source link

change string-index-of to not return -1 #1681

Closed shriram closed 4 months ago

shriram commented 1 year ago

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.

sreshtaaa commented 1 year ago

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.

shriram commented 1 year ago

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

blerner commented 5 months ago

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"?

shriram commented 4 months ago

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

blerner commented 4 months ago

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

shriram commented 4 months ago

Yours!