Tine subsurface updates

[Linus Torvalds]

On Thu, May 3, 2012 at 2:45 AM, Jef Driesen <jefdriesen at telenet.be> wrote:
> On 2012-05-02 19:35, Linus Torvalds wrote:
>
> I'm already working on some of those changes, along with the other items on
> the roadmap [1,2] (namespace prefix, cleanups, etc). This will be a huge
> break in backwards compatibility (e.g. the namespace prefix alone will
> affect every single function).

So don't worry about *that* break. I'll be dancing in the streets when
you break the compile because you've fixed the interfaces and they'll
actually be something we can rely on.

It's the small, annoying, stupid and *unnecessary* breaking that
drives me completely bonkers.

The small stupid ones that expose some internal libdivecomputer stuff
that shouldn't have been exposed in the first place, and are totally
unnecessary, and are *not* good programming. Not for you, not for the
user.

And the latest Frog addition is a prime example.

You used to have a function called hw_ostc_parser_create() that
created the parser for the OSTC. Fine. The naming even explained what
it did. But it is *insane* to change that function to take an extra
argument that is just an integer that says "it's the Frog, not an
OSTC".

The reason it's insane is that it
 (a) breaks existing users of the interface for no good reason
 (b) isn't even the clean way of doing things
 (c) now the naming makes no sense.

and none of these problems would exist if you had just created a new
"hw_frog_parser_create()" function, and left the old one alone. It
would have been a *cleaner* interface. It would have resulted in more
obvious code. It would actually document things better than just
passing a random new integer to a now mis-named function.

Sure,  you want to share 99% of the parser setup code, but that's
irrelevant. That's an internal implementation issue, and could
trivially and simply have been done by having an internal static
function inside the library that nobody else would ever see:

  enum hw_computers { HW_OSTC, HW_FROG };

  static parser_status_t hw_parser_create (parser_t **out, int model);

(and note: none of the above would be exposed to anybody else: it's
not in a header file, it's in one of your implementation C files) and
then you'd just have done

  parser_status_t hw_ostc_parser_create (parser_t **out)
  {
      return hw_parser_create(out, HW_OSTC);
  }

  parser_status_t hw_frog_parser_create (parser_t **out)
  {
      return hw_parser_create(out, HW_FROG);
  }

and now you'd have a *cleaner* interface to the outside world that
wouldn't even have broken anything? Win-win.

It's the *ugly* and pointless breakage that I get so upset about. It
makes things harder for everybody for no actual reason. And it is
indicative of the same old "I don't think of libdivecomputer as a
library, I think of it as a random collection of internal routines"
problem.

Interfaces are important. Even during development. They are doubly
important when it's a library that other projects use.

                     Linus