celix-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Alexander Broekhuis <a.broekh...@gmail.com>
Subject Re: event admin headers
Date Mon, 12 Aug 2013 09:25:56 GMT
Hi Erik,

Great work! If I have some time left I will take a more detailed look into
everything you did/are doing.

For now some remarks regarding the headers, first is the formatting. On [1]
and [2] some more details are available. It doesn't use the _pt convention
yet, but the other parts should be clear.
Especially function naming is important. Simply have a function equals in
event.h is a great recipe for failure ;). If any other included part also
has an equals function the build will fail. So it is better to use
"event_equals". This might also applies to the constants..

Second remark/improvement: You now have included the service headers (ie
the structs with function pointers), for implementation details I always
try to make a private header as well, eg "event_admin_private.h" in which
all functions are declared, instead of only functions pointers. For a user
not relevant, but for a maintainer/implementor they are.

Also for implementation details it might be worthwhile to check out the
Felix implementation. For several parts of Celix I took the Felix
implementation as a reference. They already have a working and proven
solution for a long time, so why reinvent stuff when we can learn and take
from a good reference.

[1]: http://incubator.apache.org/celix/documentation/design.html
[2] http://incubator.apache.org/celix/documentation/mapping.html

2013/8/12 Erik Jansman <Erik@jansman.eu>

> Hello,
> I have attached the header files created for the event admin. I have left
> out the eventProperties and TopicPermissions at this point. The
> topicPermission is a security aspect which I wanted to leave out of the
> google summer of code project in order to be able to have more change to
> get the project done in time. I have left the eventProperties out of the
> implementation because I'm unsure of how it is supposed to work and if it
> is needed.
> Event_admin, event_handler and event_constants are all as far as I can
> tell confirming to the specification. In event.h there is a method equals
> which according to the spec will do: "Compares this Event object to another
> object.
> An event is considered to be equal to another event if the topic is equal
> and the properties are equal.
> The properties are compared using the java.util.Map.equals() rules which
> includes identity compar-
> ison for array values." I was thinking of first implementing by comparing
> two pointers and not check on all properties.
> What does everyone think of the header files?

Met vriendelijke groet,

Alexander Broekhuis

  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message