Re: libgit2 - a true git library

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Junio C Hamano <gitster@xxxxxxxxx> wrote:
> "Shawn O. Pearce" <spearce@xxxxxxxxxxx> writes:
> 
> >>   * proper public "stuff" naming (I e.g. realy like types names -- not
> >>     struct or enum tags, that I don't really care -- ending with _t as
> >>     it helps navigating source.
> >
> > Fixed, types now end in _t.
> 
> Ugh.
> 
> You could talk me into it if you promise never typedef structures (or
> pointer to structures) with such symbols, I guess.
 
I should write that one down in CONVENTIONS.

IMHO:

  typedef uint32_t uid_t;       /* sane */
  typedef enum {...} status_t;  /* sane */
  typedef struct foo_t foo_t;   /* sane */

  typedef struct {...} foo_t;   /* borderline insane */

  typedef char* str_t;          /* totally nuts */
  typedef char**** str_pppp_t;  /* totally nuts */

Hiding the fact that scalar types like a uid_t are 32 bits on
this system is reasonable.  Heck, uid_t is already in POSIX,
we shouldn't fight that sort of idea.  It at least improves
documentation somewhat.

Hiding the fact that some scalar type is an enum, so you don't have
to type "enum blah" everywhere is also reasonable.  Its slightly
better than #define some magic constants and passing an int
everywhere.  Its a reasonable balance between reducing keystrokes
and keeping the code semi-self-documenting.

Hiding the fact that an opaque struct (or union) you cannot ever
see the members of is a struct or union is good API design.  You can
later change the major class from struct to union or back, or totally
redefine it, but the caller never needs to know what is going on.

Hiding a pointer is wrong.  Callers should know they are getting a
pointer, or are being asked to supply a pointer-to-a-pointer.  So the
"FILE*" stdio functions are sane, because we don't know what is under
a FILE type but we do know when we are dealing with a pointer to one.


My original proposal didn't stick _t onto the end of everything,
because I didn't think it was really necessary.  I'm fine with it
either way.  It may be better to include the _t suffix, it seems
to be somewhat common in libraries.

-- 
Shawn.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux