Code style improvements

RelationalAI / rai-sdk-csharp

The RelationalAI Software Development Kit (SDK) for C#

Apache License 2.0

0 stars 1 forks source link

Code style improvements #20

Closed Zlata-Zueva closed 2 years ago

Zlata-Zueva commented 2 years ago

Adjusts StyleCop configuration to better match C# coding conventions and follows code style accordingly.
Adds enums for EngineState, DatabaseState, TransactionAsyncState, TransactionMode with useful methods and updates the model to return them instead of strings.
Created solution folders structure.
Simplifies code using such language capabilities as null propagation, string interpolation, LINQ, etc.
Fixes possible NullReference warnings.

larf311 commented 2 years ago

@bradlo What do you think about the enums? I argued for having them a long time ago but others were against them. The idea is that if we introduce a new engine size, we wouldn't have to release a new SDK (and users have to upgrade) to take advantage of it. Perhaps engine size is different than states though.

larf311 commented 2 years ago

I really appreciate the PR, but in the future, could you submit each of the bullets as separate PRs, especially StyleCop changes? PRs with 116 file changes are effectively impossible to review.

stanislav-bedunkevich commented 2 years ago

against them

@larf311 @bradlo on the other hand, having enums for that feels more natural, as we can document each item, possibly with some more details if needed (like what's the cpu/mem power, cost etc).

In addition, if we're talking about backward compatibility, we would need to be able to support XS-XL sizes in the backend anyway if we decide to rename them etc, right? If talking about additions, it should be fine to update some of the major versions of the SDK we support (and which are in use) with a patch adding more engines. The users will have to update their code anyway?

stanislav-bedunkevich commented 2 years ago

I really appreciate the PR, but in the future, could you submit each of the bullets as separate PRs, especially StyleCop changes? PRs with 116 file changes are effectively impossible to review.

It makes perfect sense, apologies for having this this big, as the amount of stylecop changes got rather huge.

bradlo commented 2 years ago

enums are language dependent, in most modern languages, enums are the right thing to do. however, i added them to the python SDK based on feedback, and regret it because it just makes the code more awkward given the need to explicitly convert in too many cases. i think the C# deltas in the PR look great.