git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Shawn O. Pearce" <spearce@spearce.org>
To: Junio C Hamano <gitster@pobox.com>
Cc: Pierre Habouzit <madcoder@debian.org>,
	git@vger.kernel.org, Scott Chacon <schacon@gmail.com>
Subject: Re: libgit2 - a true git library
Date: Fri, 31 Oct 2008 14:58:57 -0700	[thread overview]
Message-ID: <20081031215857.GZ14786@spearce.org> (raw)
In-Reply-To: <7vfxmc8r8g.fsf@gitster.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> wrote:
> "Shawn O. Pearce" <spearce@spearce.org> 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.

  reply	other threads:[~2008-10-31 22:00 UTC|newest]

Thread overview: 83+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-10-31 17:07 libgit2 - a true git library Shawn O. Pearce
2008-10-31 17:28 ` Pieter de Bie
2008-10-31 17:29   ` Pieter de Bie
2008-10-31 17:47 ` Pierre Habouzit
2008-10-31 18:41   ` Shawn O. Pearce
2008-10-31 18:54     ` Pierre Habouzit
2008-10-31 19:57       ` Shawn O. Pearce
2008-10-31 20:12         ` Pierre Habouzit
2008-10-31 20:05     ` Junio C Hamano
2008-10-31 21:58       ` Shawn O. Pearce [this message]
2008-11-01 17:30     ` Pierre Habouzit
2008-11-01 18:44       ` Andreas Ericsson
2008-11-01 18:48         ` Pierre Habouzit
2008-11-01 20:29         ` Shawn O. Pearce
2008-11-01 21:58           ` Andreas Ericsson
2008-11-02  1:50             ` Shawn O. Pearce
2008-11-03 10:17               ` Andreas Ericsson
2008-11-02  1:56       ` Shawn O. Pearce
2008-11-02  9:25         ` Pierre Habouzit
2008-10-31 20:24   ` Nicolas Pitre
2008-10-31 20:29     ` david
2008-10-31 20:56       ` Nicolas Pitre
2008-10-31 21:43         ` Shawn O. Pearce
2008-10-31 21:50           ` Shawn O. Pearce
2008-10-31 21:51           ` Pierre Habouzit
2008-10-31 21:31     ` Pierre Habouzit
2008-10-31 22:10       ` Nicolas Pitre
2008-11-01 10:52         ` Andreas Ericsson
2008-10-31 23:24       ` Pieter de Bie
2008-10-31 23:28         ` Shawn O. Pearce
2008-10-31 23:49           ` Junio C Hamano
2008-11-01  0:02             ` Pierre Habouzit
2008-11-01  0:19               ` Shawn O. Pearce
2008-11-01  1:02                 ` Pierre Habouzit
2008-11-01  0:13             ` Shawn O. Pearce
2008-11-01  1:15               ` Nicolas Pitre
2008-11-01  1:19                 ` Shawn O. Pearce
2008-11-01  1:45                   ` Nicolas Pitre
2008-11-01  1:52                     ` Shawn O. Pearce
2008-11-01  2:26                       ` Johannes Schindelin
2008-11-01 11:01                         ` Pierre Habouzit
2008-11-01 13:50                           ` Nicolas Pitre
2008-11-01 17:01                             ` Pierre Habouzit
2008-11-01 20:26                             ` Johannes Schindelin
2008-10-31 23:14     ` Junio C Hamano
2008-10-31 23:33       ` Pierre Habouzit
2008-10-31 23:41       ` Shawn O. Pearce
2008-10-31 23:56         ` Jakub Narebski
2008-11-01  0:41         ` david
2008-11-01  1:00           ` Shawn O. Pearce
2008-11-01  1:04             ` david
2008-11-01  1:08               ` Pierre Habouzit
2008-11-01  1:33                 ` Nicolas Pitre
2008-11-01  1:38                   ` Pierre Habouzit
2008-11-01  1:49                     ` Nicolas Pitre
2008-11-01  1:43                   ` Shawn O. Pearce
2008-11-01  1:53                     ` Nicolas Pitre
2008-11-01 22:57                       ` Shawn O. Pearce
2008-11-02  0:26                         ` Scott Chacon
2008-11-02  1:07                           ` Scott Chacon
2008-11-02  1:36                             ` Shawn O. Pearce
2008-11-02  5:09                             ` David Brown
2008-11-03 16:20                               ` Shawn O. Pearce
2008-11-01  1:06           ` Pierre Habouzit
2008-11-01  1:36             ` david
2008-10-31 20:24   ` Brian Gernhardt
2008-10-31 21:59   ` Andreas Ericsson
2008-10-31 22:01     ` Shawn O. Pearce
2008-10-31 22:51       ` Junio C Hamano
2008-11-01 11:17       ` Andreas Ericsson
2008-10-31 23:22     ` Johannes Schindelin
2008-10-31 23:18 ` Bruno Santos
2008-10-31 23:25   ` Shawn O. Pearce
2008-11-01 19:18 ` Andreas Ericsson
2008-11-01 20:42   ` Shawn O. Pearce
2008-11-02  2:30     ` Johannes Schindelin
2008-11-02  9:19     ` Pierre Habouzit
2008-11-03 13:08     ` Andreas Ericsson
2008-11-08 13:26 ` Steve Frécinaux
2008-11-08 14:35   ` Andreas Ericsson
2008-11-08 17:27     ` Pierre Habouzit
2008-11-09 10:17       ` Andreas Ericsson
2008-11-09 21:02         ` Shawn O. Pearce

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20081031215857.GZ14786@spearce.org \
    --to=spearce@spearce.org \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=madcoder@debian.org \
    --cc=schacon@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).