cequence-io / openai-scala-client

Scala client for OpenAI API
MIT License
191 stars 20 forks source link

Method terminate in GuiceContainer has side effects but is declared parameterless #39

Closed phelps-sg closed 1 year ago

phelps-sg commented 1 year ago

The method io.cequence.openaiscala.service.GuiceContainer.terminate has return type Unit but is declared without parentheses. The Scala style guide states that methods without parentheses should only be used when the method has no side effects. Here the terminate method calls terminate() on the ActorSystem and thus it has side effect.

In general, methods which have Unit return type should always declared with parentheses (since if they do not have side effects they are not doing anything at all).

See https://alvinalexander.com/source-code/scala-style-use-parentheses-methods-side-effects/:

"However, this syntax (leaving off parentheses) should only be used when the method in question has no side-effects (purely-functional). In other words, it would be acceptable to omit parentheses when calling queue.size, but not when calling println() (or openGarageDoor()). This convention mirrors the method declaration convention given above.”

“Religiously observing this convention will dramatically improve code readability and will make it much easier to understand at a glance the most basic operation of any given method. Resist the urge to omit parentheses simply to save two characters!”

I am using JetBrains IntelliJ as an IDE, and it reports this is a warning in the editor:

Warning:(21, 17) Method with Unit result type is parameterless

See first example here: https://blog.jetbrains.com/scala/2011/03/10/signature-matters/

Also note that in source of ActorSystem.terminate() it is declared with parantheses.

See also #22.