I managed to find a little time to work on the Glom bounty today, i managed to get "save position of tables" working. Not very hard to do, i really must compliment the Glom guys (and also the Gtkmm and Bakery guys) for writing very nice and standards compliant code. My only problem is with the:
1 2 3 4 5 6 7 8 9 10 | void Stuf::lookAtThisMethod () { ... } // vs. void Stuff::look_at_this_method () { ... } |
I prefer the first form over the second one, especially the curly braces positioning, but i also think the Java way of naming things is terrific.
But hey, this is small stuff as long as the code itself is easy readable - and Glom is certainly that, although IntelliSense (that doesn't suck) for Emacs wouldn't hurt.
I think i am now on the road to claiming this bounty, however, there is still a few snags in the "get relationships" code, for instance: take a look at this music collection example relationship overview, there is an arrow going from Publishers::Publisher ID to Albums::Album ID - that doesn't seem right.
Guess i'll have to dig a little deeper into Glom's relationship modelling code.
I now have the first three christmas lunches under my belt, and have reached the always dangerous point where i start to get a taste for "snaps" (over 45% proof alchohol) - this usually means that i will soon "over indulge" on that part.
Well - it's only christmas once a year - i'm actually starting to get a bit of a christmas vibe going, usually it will fade away in a couple of days and i can go back to longing for the summer.
Comments (14)
My eyes!
It's the gtkmm convention, which is mostly based on Stroustroup's book. I'm glad you like the Glom source. I think the Box_Data_* classes could do with a lot more documentation about how the virtual methods interact.
Your space before the bracket is heresy. Furthermore, you (and the others) will be punished in the next life for that curly braces positioning. It's against nature. I think there's something in the human rights convention about this. Or maybe not.
Well i had to take doxygen into use to really get an overview of how the classes was related.
After years of Python use i have grown really fond of using a lot of whitespace in my code, i rarely have two [HTML_REMOVED]tokens[HTML_REMOVED] next to each-other without a space in between.
Regarding the human rights convention, i believe Metallica has written a song called [HTML_REMOVED]Free speech for the dumb[HTML_REMOVED].
I think void this_method() makes the most sense in C++ since the STL is written with that naming convention. If anything, mixing naming conventions is ugly.
A good strategy for curly braces positioning is to differentiate conditional code from structural code:
void Foo::some_method() { if (something) { doThis(); } else { doThat(); } }
That makes it easier to browse code, because the [HTML_REMOVED]image[HTML_REMOVED] of the different types of structures is clear. (Remember, the brain interpret code (and text) as images).
I don't quite agree on the my_method() way of doing things in C++ just because STL does so. STL is templates, and when i write templates i also use that syntax.
(Sorry for the way my blog formatted your code sample it came through the mail system alright - you can use three some wiki-like syntax to get proper formatting, three { in a row gives you code [HTML_REMOVED]on[HTML_REMOVED] and three } in a row turns [HTML_REMOVED]off[HTML_REMOVED] code).
Your proposal actually looks like what emacs gives you if you just use default c-style and try to write [HTML_REMOVED]Glom-style[HTML_REMOVED] code. (we use that style at work).
[HTML_REMOVED]I can see that you also have a bit of Java-style in you, with all the doThis() and doThat() methods :-)
I'm actually starting to like this style, but then again, it's hard to teach a new dog old tricks (or whatever ;-)
But i don't think i will ever move away from camelCasing (or javaCasing) - at work we use PascalCasing - a lot of people seems to be adopting that as they start using C#, and i don't care for that one bit. Especially when people also start using them for local variables.
Something that i have started adopting, and am very fond of, is using the m_ for members style, i also try to use _variable for method parameters. This saves you from accidentally using a parameter when you actually wanted to use a local variable.
Oh, and while we are on code-style, i really miss a common code-style for the entire Python library, they have all sorts of casing systems in there. But i guess that will have to wait for Python 4000.
I don't see why a templated class should have different casing conventions than a non-templated class, I mean, in client code, most of the time you use a templated object in the same way as a non-templated object.
Ah crap, the example should of course have been void Foo::some_method() { if (somethingExtendedToShowMyCasingOfVariables) { do_this(); } else { do_that(); } }
Although lately I've begun using this-[HTML_REMOVED]do_that(); to empathize that it is a method and not a non-classed normal function like cos().
I've worked fair amounts with C#, Java, C and C++, and I always try to adopt to the coding conventions (that explains my messed up example :P).
Personally I am experimenting with moving away from any m_ or _ prefixing, since I think that only bloats the code. If one is writing a method and don't know where a particular variable came from (local, class, parameter), then the method is too big anyway.
By the way, I'm planning to do a Glom 1.4.0 release in about a month and a half. It would be great to get this feature into 1.4
That should probably be possible, january looks like i have a bit more spare time than what december offered.
Ping?
Pong!
Sorry about the lack of progress, real life keeps getting in the way. To have time to really wrap my head around the relationship modelling in Glom i think i need to dedicate a complete day (so my head won't be filled with work-related stuff). Hopefully the coming weekend will provide that.
When do you plan on the 1.4.0 release ? You said a month and a half on december the 22. so i guess it is just around the corner.
If i get the relationship code working properly this weekend we should a least have a basic working dialog for 1.4.0.
I am continually merging with upstream (Glom CVS) so integration shouldn't prove much of a problem.
Really, the relationships aren't complicated. It looks like you have everything working. If there's a bug that needs fixing, let's apply it and then fix the bug.
Yes, I'd like to do it soon, roughly on the GNOME 2.18 schedule.
Hi Murray
I have created a patch for the current Glom CVS HEAD, it should apply cleanly.
http://halfdans.net/files/glom/glom_relationships.patch
Could you set me up with commit access to the Glom CVS, to make fixing the last snags easier?
This contains some unrelated changes, such as - int response = Glom::Utils::dialog_run_with_help(m_pDialogConnectionFailed, [HTML_REMOVED]dialog_error_connection[HTML_REMOVED]); + int response = m_pDialogConnectionFailed-[HTML_REMOVED]run();
Could you make a revised patch with just your changes, please? And could you put it in a bugzilla bug.
It's a bit early for SVN write access and it would take a long time to get GNOME SVN write access. It shouldn't be necessary for this one change.
Guess i should be a bit more careful with my Bazaar merges, this should work fine (i had to do some hand-editing of the patch to remove my [HTML_REMOVED]use default automake version[HTML_REMOVED] stuff):
http://halfdans.net/files/glom/glom_relationships.patch
It applies cleanly on my version of upstream.
I am checking this into svn trunk, so you can make changes against that.
However, I am disabling the build because a) goocanvas is maybe not ABI-stable yet, so we should not use it by default, and b):
Unfortunately it needs to be updated for the latest goocanvas API, which has changed some of the class names:
dialog_relationships_overview.h:42: error: [HTML_REMOVED]#8216;GooCanvasItemView[HTML_REMOVED]#8217; has not been declared dialog_relationships_overview.h:42: error: [HTML_REMOVED]#8216;GooCanvasItemView[HTML_REMOVED]#8217; has not been declared dialog_relationships_overview.h:44: error: [HTML_REMOVED]#8216;GooCanvasItemView[HTML_REMOVED]#8217; has not been declared dialog_relationships_overview.h:44: error: [HTML_REMOVED]#8216;GooCanvasItemView[HTML_REMOVED]#8217; has not been declared dialog_relationships_overview.h:46: error: [HTML_REMOVED]#8216;GooCanvasItemView[HTML_REMOVED]#8217; has not been declared dialog_relationships_overview.h:46: error: [HTML_REMOVED]#8216;GooCanvasItemView[HTML_REMOVED]#8217; has not been declared dialog_relationships_overview.h:47: error: [HTML_REMOVED]#8216;GooCanvasView[HTML_REMOVED]#8217; has not been declared dialog_relationships_overview.h:47: error: [HTML_REMOVED]#8216;GooCanvasItemView[HTML_REMOVED]#8217; has not been declared dialog_relationships_overview.h:60: error: ISO C++ forbids declaration of [HTML_REMOVED]#8216;GooCanvasModelSimple[HTML_REMOVED]#8217; with no type dialog_relationships_overview.h:60: error: expected [HTML_REMOVED]#8216;;[HTML_REMOVED]#8217; before [HTML_REMOVED]#8216;*[HTML_REMOVED]#8217; token dialog_relationships_overview.cc:30: error: [HTML_REMOVED]#8216;GooCanvasItemView[HTML_REMOVED]#8217; was not declared in this scope
But please please open a bugzilla bug about this. That's much easier to work with.
Post comment
If you wish, you can use markdown syntax in the comment field.