Marenthyu / LiveSplit.StreamingPredictionsComponent

A Component to automate Channel Point Predictions on Twitch
BSD 3-Clause "New" or "Revised" License
12 stars 0 forks source link

StringBuilder or standardized JSON output for payloads? #1

Open kenman345 opened 3 years ago

kenman345 commented 3 years ago

Hey,

Love the project, havent used it myself yet but was looking over the code and found a few instances of building payloads by hand and with + signs.

https://github.com/Marenthyu/LiveSplit.StreamingPredictionsComponent/blob/7a4a056194252e20a93f8d7db24b393ae796d079/UI/Components/StreamingPredictionsComponent.cs#L180-L186

I was thinking by builder the payloads this way, you are probably using more memory since strings are immutable, than intended. Even if for a short time. I was thinking that a StringBuilder might be a good temporary solution, but I also figured another solution might work better long term.

The proposed solution, of which I dont know every precise detail but enough to know what we will need to learn, is to make a set of classes that inherit from each other with the construction/inheritance using the settings to get started. If we then add the unique values of a request payload, then we can store the value to an object, then cast them to the payload type and use a standard JSON library to serialize the object itself. This would allow you to set up a different class that pertains to a given endpoint in the process and centralize the update of them. if the request type is used in multiple places, it will be easier to find the connections and require less effort to update all the locations with a new string to be built since we only need to add the missing data if it was not already stored in the object.

Marenthyu commented 3 years ago

You are absolutely correct that the way it is done currently, it's not the most efficient!

In general, i don't think this usecase warramts going that far out of our way, though, since it is intended for streamers that have PCs that are decent enough to hable the few additional Bytes in RAM this causes - switching it to a String.Format or StringBuilder is probably still reasonable. Feel free to open a PR!

Marenthyu commented 3 years ago

Ahaha, i just got myself during testing: Since I'm not JSON Escaping things properly, entering a " in the title or options, stuff breaks.

That'll need to be fixed!