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

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

Closed
shriram opened this issue Jan 1, 2023 · 6 comments
Closed

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

shriram opened this issue Jan 1, 2023 · 6 comments

Comments

@shriram
Copy link
Member

shriram commented Jan 1, 2023

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
Copy link

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
Copy link
Member Author

shriram commented Jan 19, 2023

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

@blerner
Copy link
Member

blerner commented May 2, 2024

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
Copy link
Member Author

shriram commented May 18, 2024

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

@blerner
Copy link
Member

blerner commented May 18, 2024

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

@shriram
Copy link
Member Author

shriram commented May 18, 2024

Yours!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants