New class names in last releases #826
Replies: 11 comments
-
They aren't CSV related. I'm open to suggestions on a good prefix if you have one, but I don't want to use Csv since those classes can be used for reading other file types too. Excel and fixed width files are 2 examples. Also, you can use this if you want to change the name. using CsvConfiguration = CsvHelper.Configuration.Configuration; |
Beta Was this translation helpful? Give feedback.
-
I just accepted new names and changed my code accordingly, but I have to agree with @IvanYur4enk0. Probably Josh wanted to remove inflationary use of "Csv" prefix in class names and I think adding some prefix to every class isn't a good pattern in general. But from practical point of view it made life easier and I personally liked it in CsvHelper. One reason "pro prefix": CsvHelper is no .NET base library or something similar and therefore e.g. "Configuration" is no name this library should take (yes, I'm aware of namespaces, too.) @JoshClose: One reason against your comment above: Your library and main namespace is called "CsvHelper". So you would have to rename it to "Helper" or "CsvExcelFixedWidthHelper" to make this argument consistent... ;-) It's totally fine to stick with "Csv*" for namespace and classes. I'm not sure but I guess csv files are main reason to use your great library and even when not: it's the established name of this lib. If you compare with Newtonsoft.Json: objects there are prefixed with "Json*" everywhere and I think this is quite useful. It allows to use any generic word like "Object" without the need to look for some workaround name: JsonObject, JsonArray, ... My last point: fiddling with "using" to alias types is not very practical. IMHO it's a mess. I use another (great) library [Language-Ext] (https://github.com/louthy/language-ext) and there are some naming conflicts with System.Collections.Generic. I worked with different solutions there but none of them is satisfying. |
Beta Was this translation helpful? Give feedback.
-
All very good points. Where do I draw the line though? I really don't want Someone suggested at one point of nesting all the classes inside a |
Beta Was this translation helpful? Give feedback.
-
Where is difference between namespace and nested classes from users point of view? You can have Csv.Configuration easily by changing your namespace logic -- without nested classes. (I personally would think of a flat hierarchy here as an improvement.) Main problem remains: in real life most developers will use I really see the point why you changed this. From theory point of view this is looking "as it should be". But: would you mind to use same class name twice within your library (in different sub name spaces)? Anyway: it's your library, thanks for making this open source! |
Beta Was this translation helpful? Give feedback.
-
I think with the nested classes doing I would not want to use the same class name in multiple places, more than likely. I'd like to make changes here, but I don't want to keep doing it. |
Beta Was this translation helpful? Give feedback.
-
With the nested class idea, you can use partial classes as well. So you do
Public static partial class CSV
I have used this technique to version objects in the past. It works and
reads quite well.
…On Nov 6, 2017 4:45 PM, "Josh Close" ***@***.***> wrote:
I think with the nested classes doing using CsvHelper.Configuration you'd
use Csv.Configuration. You wouldn't be able to do using
CsvHelper.Configuration.Csv. You would however be able to do using
Configuration = CsvHelper.Configuration.Csv.Configuration is you wanted
to remove the Csv. part. It's something someone suggested, but I don't
know if I've seen it actually used anywhere. It might also make things
needless complicated in the source code... but I'd have to try it out to
see how it works.
I would not want to use the same class name in multiple places, more than
likely.
I'd like to make changes here, but I don't want to keep doing it.
CsvReader and CsvWriter really aren't CSV related at all. They just turn
a string[] from the parser into a class object. It would be nice to find
another name.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<https://github.com/JoshClose/CsvHelper/issues/826#issuecomment-342297774>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AD_oharocIZtJs_aI7_MwqRtZsS3qcurks5sz33hgaJpZM4QNgHB>
.
|
Beta Was this translation helpful? Give feedback.
-
makes items in static class available without namespace / class name. This is used e.g. in Language-Ext https://github.com/louthy/language-ext It makes static fields and methods directly available, too. |
Beta Was this translation helpful? Give feedback.
-
I'm still open to changing this. I don't want to do it more than once again. The future idea is there will be a I also don't really want to do any other file formats. Fixed width is so similar that it makes sense, but not really anything else. If I just stick with these 2, it's probably OK to prefix everything with I think I've added too many features already and the complexity has gone up. I think the context was a good way to solve some issues, but it also created some other issues; one major one being a performance slow down. |
Beta Was this translation helpful? Give feedback.
-
From my understanding I am fully agree and vote to add Csv prefix for each public class in that wonderful library. It will be convenient for other developers and I think all others expect and will use only CSV parsing functionality from that library according it naming and information on primary page. |
Beta Was this translation helpful? Give feedback.
-
Now I have class names like |
Beta Was this translation helpful? Give feedback.
-
Yeah, that sucks, I know. You probably wouldn't be able to F2 because it's not a valid object anymore. |
Beta Was this translation helpful? Give feedback.
-
Hello,
Could you please clarify, why good context class names like CsvConfiguration, CsvClassMap and others was replaced by common names like Configuration and ClassMap?
Because seems that it is very strange renaming and it is antipattern to use such common class names for that purposes in .NET no one other framework using such names even very common Microsoft libraries. If just suppose that each configuration (transaction, EntityFramework, ADO.NET, some other 3d party libraries and e.t.c) will have an equal common name like Configuration(I understand that it will have different namespaces) - it will be uncomfortable for everyone because solution will have dozens of Configuration classes and it does not match any popular\good naming conventions for class names...
Such renamings cause concerns for others when 3rd library in some release start live using some strange naming conventions that are not part of the common .NET conventions and what expect in future in that way.
Best Regards
Beta Was this translation helpful? Give feedback.
All reactions