awaescher / OllamaSharp

The easiest way to use the Ollama API in .NET
https://www.nuget.org/packages/OllamaSharp
MIT License
524 stars 76 forks source link

[Question] Why is the Chat messages list hidden behind readonly #82

Closed Plootie closed 1 month ago

Plootie commented 1 month ago

I'm curious about the design choice to have a chat's message list private and only exposed via the IReadOnlyCollection Messages property. For someone wishing to, for example, limit the chat messages to n would need to do something similar to:

List<Message> modifyableMessages = new List<Message>(ollamaChat.Messages);
if(modifyableMessages.Length > 50)
{
    modifyableMessages.RemoveRange(0, modifyableMessages.Length - 50)
}
ollamaChat.SetMessages(modifyableMessages);

which can be a little clunky. Are there any particular reasons why we couldn't have this field exposed directly?

awaescher commented 1 month ago

Generally, I prefer keeping lists to the owning class exclusively and - if required - exposing them as IEnumerable or alike. This is to prevent other classes from manipulating its state.

I admit in this case I guess it would be better to expose the list as such. It's a library that helps to chat after all. If someone wants to change the messages ... why not?

Would you like to submit a pull request? I guess we could remove the SetMessages(), too.

Plootie commented 1 month ago

Understandable. Sure! I'll submit one when I'm back from holiday on Monday ^^

awaescher commented 1 month ago

Awesome, looking forward! I am back on Monday too 😄