prismicio-community / csharp-kit

Development kit for the C# language
11 stars 24 forks source link

Rewrite API to idiomatic C# #2

Open tucaz opened 9 years ago

tucaz commented 9 years ago

Hi,

I just create a new pull request with some improvements and fixes regarding culture handler and while doing it I noticed that the C# used in this library is not very idiomatic.

Would you guys be willing to accept pull requests moving the code to a more idiomatic C#?

Thanks

erwan commented 9 years ago

Hi,

It's important for us to have idiomatic code in each langage. That said there are times where we balance that with consistency with other kits. In the case of the C# kit, it's not the langage I'm the most proficient in so there are certainly areas that can be improved in terms of code style.

To prevent you from taking time to make big pull requests that we may not be able to integrate for API consistency reasons, could you first let me know what are the points that would need to be changed to be more idiomatic C#?

tucaz commented 9 years ago

I'm sorry for the long time it took for me to reply.

A few changes that I would do are:

erwan commented 9 years ago

Hi,

If you want to do pull requests that's great. Just make sure to make separate PR for each issue to keep them small and quick to review. Otherwise I'll probably do it eventually.

ghost commented 8 years ago

Whatever happened to this effort? I agree, the code could be made more idiomatic without losing any functionality and maintaining readability.

erwan commented 8 years ago

As I said it's something I'll probably do eventually, but it's not very high priority on my task list.

ghost commented 8 years ago

It sounds like tucaz is willing to do the work (as per the first message). Would accepting contributions from the community make it more feasible?

erwan commented 8 years ago

Hi @PeteK68 ,

Yes, I will accept contributions for points I noted above as soon as they don't break compatibility with the current version (deprecating existing methods if necessary).

Ideally it would be in separate pull requests so they are easier to review and can be integrated more quickly.

Grepsy commented 6 years ago

We're evaluating this product and the .NET API not being idiomatic is a bit of a turn-down. Especially since it hasn't been fixed for over two years.

AlexFursenkoOd commented 6 years ago

Could you modify method Query(params IPredicate[] predicates) in Form.cs to use iterator of type IPredicate instead of Predicate. Until you do so there is no way to create my own predicate. There is no sense to inherit from IPredicate because method Query uses iterator of type Predicate, so if I pass my predicate of type IPredicate method will try to convert it to instance of type Predicate which will through an Exception.

Or perhaps mark method Query of type Predicate as virtual. This way we can inherit from type Predicate. It will allow us to override existing implementation of Predicate class Anyway, for now there is no real way to add not supported in this library predicates (like "has")