facebook-csharp-sdk / simple-json

JSON library for .NET 2.0+/SL4+/WP7/WindowsStore with optional support for dynamic and DataContract
MIT License
382 stars 143 forks source link

Class named the same as its Namespace #57

Closed ghost closed 10 years ago

ghost commented 10 years ago

Hello,

Thank you for a really great project. One minor issue that I have is that in this project, there is a static class named SimpleJson which resides in a namespace which is also called SimpleJson.

Please consider changing the name of that class or the namespace as this is considered a major design faux-pas. Eric Lippert - one of the principle developers on the C# compiler team, has written a 4 part series, solely to address this topic.

Esam-Bustaty commented 10 years ago

I have to say that this is an important point, now it's useless to add using SimpleJson; in the top of the file, and it's ugly to write SimpleJson.SimpleJson.SerializeObject().

I think also that the class name is redundant, SimpleJson is the name of the library, so giving this name to the namespace is a good idea, but the class SimpleJson should be named something that reflects what it actually does, it's responsible for serialization and deserialization, so it can be named Serialization or JsonSerialization, it's also acts as the main class for interaction, so it can be JsonCore or just Core, Serialization itself is considered mapping of objects to strings and vice-versa, so it can be called JsonMapper.

I know that the names I suggested ain't great, but let's be open to suggestions and see if someone come up with a descent name, and please let me know if renaming the class is refused for any reason so we can seek other solutions.

prabirshrestha commented 10 years ago

closing as won't fix.

If you install using nuget, the namespace is not SimpleJson and is same as your project namespace. If you are not using nuget, you need to manually change it.

https://github.com/facebook-csharp-sdk/simple-json/blob/master/jakefile.js#L264-L276

Here are some examples:

https://github.com/facebook-csharp-sdk/facebook-csharp-sdk/blob/4035d1ff5379ee50ed8d4aaf252b8b583fd531fa/Source/Facebook/SimpleJson.cs#L68

https://github.com/octokit/octokit.net/blob/b67b128947ca0e489bf5933b52d67ccd7fcfcaae/Octokit/SimpleJson.cs#L74

https://github.com/restsharp/RestSharp/blob/196c2511099b93e8900f0456b69e326bf995dfe9/RestSharp/SimpleJson.cs#L71