[gnome-print] Filtering code
- From: Chema Celorio <chema ximian com>
- To: sgifford suspectclass com
- Cc: gnome-print ximian com
- Subject: [gnome-print] Filtering code
- Date: 18 Jun 2002 02:14:31 -0500
Hello Scott,
Your work for the filtering stuff is great, excellent work.
I started integrating your filter work into gnome-print and did some
modifications to it to GNOMEfy it. As such the code worked fine and
there isn't anything wrong about not beeing GNOMEfied from a code
perspective it works fine, but having a consistent way of writing code
makes GNOME a lot easier to understand. I got a bit carried away but
decided tonight to give you what I currently have so you can continue
working on it.
This are some of the GNOMEisms I changed.
* #ifdef __GNOME_PRINT_FOO_H__ v.s. #ifdef FOO_H in headers
* Structures are NamedLikeThis rather_than_small_letters
* use gchar rather than char (this is changes on some GNOME
modules, but i like to use the gvariants more)
* Use of constant arguments whenever possible, this clears up
a lot who owns that piece of memory
* Indentation.
* The headers are enclosed inside G_BEING_DECLS and G_END_DECLS, this
is not a problem right for headers that are not installed but if we
had C++ code inside gnome-print (Think about OMNI link) this might
cause problems, it is a general good idea to just wrap the .h files
around them.
* We define variable names when prototyping functions. The struct
inside gnome-print-filter.h should have variable names like :
"(*foo) (gint a, gchar b)"
It helps a lot when trying to understand what the code is doing to
have a meaningful name to attach to the parameters
* functions that create a new structure/object for you are named _new,
init functions usually operate on a structure already created. So
the _new function might call _init on the object to load it with
default values.
* Use gboolean values where applicable, although booleans are really
just ints it helps a lot because whoever is reading the source code
thinks about it as a boolean and you can assume that you will only
have a TRUE or FALSE value in there
* We have a standard way of documenting functions, this allows us to
create documentation from source code comments. For example, the gtk
API documentation as seen here:
http://developer.gnome.org/doc/API/2.0/gtk/gtk-gtktreemodel.html
is created by functions being documented with a standard template,
we have a emacs macro to add automatically the template for
functions. If you use emacs you can add a macro
for it, search for "gnome-doc-insert" in:
http://www.gnome.org/~chema/.emacs
* Use g_return_val_if_fail to check arguments passed to functions,
the main reason is that this macro will emit a useful warning if
this happens, if you simply return a -1 you might never discover
the offending code that is calling the function incorrectly. This
macros also print the file and line number that the assertion
failed. For example, in the setsrt
function we now check if the Filter has a setstr function, if some
code is calling setstr with a filter that does not have a setstr
function, having a warning on the console is going to allow is us
to see this as soon as we make the mistake, also finding the bug is
going to be very easy because you know something is wrong and where
it is happening.
* structure member naming. The names we give to the members of the
structures is very important, we have to be as clear as we can, so
->set_string is prefered to ->setstr.
* We usualy don't add const to structures when calling a method. In
filter->filter you had the first argument as const struct
gnome_print_filter_type. The reason for it is that the const means
the memory pointed by the pointer will not change. For example,
if you call a fitler method with a Hex encode filter,
gnome_print_filter_hexenc_filter is modifying the filter
(d->outbuffsize, d->linepos, d->outbuf are assigned inside)
* GnomePrintFitlerType should really be GnomePrintFilterClass, there
wasn't a clear line on what they instance of a filter was and
its class.
Here is my current patch for it, You can download it and continue
working on this.
I didn't finished polishing the rle filter, but added some comments in
the code. I didn't tested the filter code for hex & rle, it was in
"compiles" state ;-). I don't think that i screwed anything up but I
can't bet my life on it. You'll find the patch unfinished and with some
leftover code too which we need to clean before getting comiting it.
On the short term this is what I'd like to do:
- Only get 3 filters working: hex, rle, & flate. flate is the most
important one because the objects and classes might need tweaking to get
this filter to work. Forget about the other filters for now lets get the
structs and API nailed first.
- We need to make the filters work both ways, encode and decode to make
the test suite reliable. Plus we already had code for this in the 1.4
branch.
- Forget about the _set_pointer and _set_string functions for now,
hardcode those values. We will use GPANodes later for it. For now, only
make it so that one filter works and it is passed as an id by the rest
of gnome-print
- In the testsuite code, I'd like to decode files and decode them rather
than having known stuff. This makes our testing code scale, we don't
need to add more files when we port more filters.
- Move as much of the functionality to gnome-print-filter.c as possible
for the filters to use (already did some of this)
- We might want to use a macro to reduce the boilerplate code. See:
gnumeric/src/sheet-object-widget.c , not a high priority right now
thought.
Why did you used size_t for needed bytes instead of gint? I changed it
to gint but have the feeling it should really be size_t.
Excellent work ! Thanks a lot,
Chema
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]