P-p-H-d / mlib

Library of generic and type safe containers in pure C language (C99 or C11) for a wide collection of container (comparable to the C++ STL).
BSD 2-Clause "Simplified" License
869 stars 76 forks source link

Naming Critique #82

Closed vladipus closed 2 years ago

vladipus commented 3 years ago

As I'm refactoring the library in my separate branch (also thinking about renaming it to avoid the confusion), I'm thinking more and more about renaming the internal macros, ending with I. Prefixing with _ is a lot better than suffixing with I, because those macros actually clutter the global completion namespace. So when you try to auto-complete something like TUPLE, a lot of internal "garbage" macros appear in the list, which is not really useful.

Another thing is that certain macros of the library are actually missing the M_ prefix, which in turn clutter the global namespace, which should be system- or user-defined.

I hope you don't mind some critique from the actual user of your tech ;)

vladipus commented 3 years ago

The new _ct suffix for type aliases is also confusing. Does that mean "constant type"? Well, that is even more vague, since the constant may actually mean const. You could also prefix the internally used type alias with an underscore, IMO.

vladipus commented 3 years ago

I think this is also a typo (m-shared.h): image

P-p-H-d commented 3 years ago

I'm thinking more and more about renaming the internal macros, ending with I. Prefixing with _ is a lot better than suffixing with I, because those macros actually clutter the global completion namespace. So when you try to auto-complete something like TUPLE, a lot of internal "garbage" macros appear in the list, which is not really useful.

Prefixing with _ is illegal as per the C standard. It is a reserved prefix:

All identifiers that begin with an underscore and either an uppercase letter or another underscore are always reserved for any use (§7.1.3) All identifiers that begin with an underscore are always reserved for use as identifiers with file scope in both the ordinary and tag name spaces (§7.1.3)

Future version of the norm lists more exactly the reserved pattern in §J.6.1:

_[a-zA-Z_][a-zA-Z0-9_]*

Prefixing with something else than may be better indeed for auto-completion (even if TUPLE_ should only propose valid macros).

certain macros of the library are actually missing the M_ prefix, which in turn clutter the global namespace, which should be system- or user-defined.

Which one for example?

Does that mean "constant type"? No. The suffix _ct indicates a confidential type (not exported)

I think this is also a typo (m-shared.h):

Yes, indeed. There is two '_' in the suffix. However, this confidential type is not used in the oplist, so there is no impact. So, I'll remove theses useless types instead. Thanks.

vladipus commented 3 years ago

One more consideration from me is about C-string methods, string_get_str for example. This is opinionated really, but I would personally call those methods with something like cstr instead of str, as str is really confusing - your own string type is also starting with str.

vladipus commented 3 years ago

Prefixing with _ is illegal as per the C standard. It is a reserved prefix:

Are you talking about a single underscore or double underscore? I think the former is fine, as I'm using it currently without any warnings while using the strictest level of warning generation.

vladipus commented 3 years ago

this is even more confusing with general-purpose methods like _in_str where no C-string is present....

P-p-H-d commented 3 years ago

Are you talking about a single underscore or double underscore? I think the former is fine, as I'm using it currently without any warnings while using the strictest level of warning generation.

Yes. More exactly, as per the standard (see the extract of the standard in my previous comment):

underscore + digit is not reserved however.

this is even more confusing with general-purpose methods like _in_str where no C-string is present....

Yes, naming is not perfect. :) In particular string_get_str which is not related to cstr at all.

vladipus commented 3 years ago

I suggest you ditch the _str methods for FILE and just rename them to _stream or _file. that's what I'm going to to in my branch, at least. =)

P-p-H-d commented 3 years ago

So when you try to auto-complete something like TUPLE, a lot of internal "garbage" macros appear in the list, which is not really useful.

I try changing the prefix of the internal macros (so that it start with something). But the auto completion I used (VS code) is "too smart" and still presents then even if they start with another prefix. So the renaming was useless (and not committed).

I suggest you ditch the _str methods for FILE and just rename them to _stream or _file. that's what I'm going to to in my branch, at least. =)

I plan (long term) to obsolete then as they can be implemented through IN_SERIAL / OUT_SERIAL.

vladipus commented 3 years ago

I'd suggest you could use something simpler like read\write serialize\deserialize. those are also pretty standard these days.

P-p-H-d commented 3 years ago

For internal macro naming, I think I'll use the following rule:

with F transform a little bit the container name (preventing the automatic completion to works). Like this for list:

For generated internal functions, I think I will change the naming as per:

vladipus commented 3 years ago

Well, it would be better to auto-complete starting at M_. The list should include "public" entities only then, with no internal "garbage" terms.

M_L1ST_DEF_P4 looks rather scary, man)) Better use some hexadecimal representation of the name's symbols (joking). Why not choose some kind of uncommon prefix. I'm currently using a small i letter for my prefixes and it does just fine.

M ... public stuff for the end user. iM ... lib's development area

Isn't the m_ prefix historically relates to members?

P-p-H-d commented 3 years ago

Yes, it is a little bit strange. Unfortunately, the analyzer of the auto-complete function of some IDE (example Visual Code) don't just take the prefix to complete but tries to find a match even if the prefix doesn't match. So even if I start a macro with M_i_LISTsomething, when I write LIST, I will still have M_i_LIST_something as a potential match. The only thing I found which works is to rewrite completely the prefix LIST into something else (here L1ST). I don't know how to inform the auto-complete function that a symbol shall not be auto-completed, or to inform it of a kind of "priority" in the symbols.

The m_ prefix is indeed used by a lot of C++ code to identify the members of a class.

vladipus commented 3 years ago

My personal opinion is that changing the prefix is "good enough" as changing the name to l33dspeak is "too much".

skotopes commented 2 years ago

@P-p-H-d

I'd say thank you for this wonderful library. There is one thing that were bothering me for last year:

I found that me and my team mates are constantly confusing string_clean/string_clear, no matter how many times we bring this topic we do it again and again.

Can you change method names to classic _alloc/_free?

P-p-H-d commented 2 years ago

Can you create another issue on this subject so that we can properly discuss it? This issue is about the missing prefix on the exported names (the internal names have already been done).

skotopes commented 2 years ago

Can you create another issue on this subject so that we can properly discuss it? This issue is about the missing prefix on the exported names (the internal names have already been done).

Sure.

vladipus commented 2 years ago

You can also check out my fork, cause it has some quite nifty functionality to rename the methods: https://github.com/vladipus/mlib I had similar issues to yours originally.

skotopes commented 2 years ago

@vladipus we'd like to harmonize all those nuances and contribute back, forking is our last resort

P-p-H-d commented 2 years ago

All the public API have been put in the M_ namespace. All the internal API have been put in their respective namespace. A wrapper to the old names is provided (which is enabled by default and can be disabled by defining M_USE_SMALL_NAME to 0 before including any header file).

Planning:

The renaming of the _str suffix has been put in its own ticket (#99)