Skip to content

OpenAIService.close method declared with parentheses #23

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

Merged
merged 1 commit into from
Jun 12, 2023

Conversation

phelps-sg
Copy link
Contributor

fixes #22

@peterbanda
Copy link
Member

OK, to be consistent with client.close() it makes sense. Although on the caller's side having no parentheses is perfectly fine.

@peterbanda peterbanda merged commit 8e1db14 into cequence-io:master Jun 12, 2023
@phelps-sg
Copy link
Contributor Author

phelps-sg commented Jun 15, 2023

This doesn't affect the functionality. However, by convention in Scala declaring a method with no parentheses implies that it has no side effects, ie that it is a pure function. This is misleading in the case of close, because it has side effects on the underlying web socket. In general, we only use parenless methods to retrieve values in such a way that we do not mutate state, as per the example from the Scala guide quoted in #22:

// doesn't change state, call as birthdate
def birthdate = firstName

// updates our internal state, call as age()
def age() = {
  _age = updateAge(birthdate)
  _age
}

Again, the functionality is not affected but it can cause confusion to people reading the code, and confusion can lead to bugs ;-).

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

Successfully merging this pull request may close these issues.

OpenAIService.close method missing parentheses
2 participants