calimero-project / calimero-core

Core library for KNX network access and management
Other
129 stars 65 forks source link

Refactor Tunnel Type with enum #24

Closed ouaibsky closed 8 years ago

ouaibsky commented 8 years ago

Hi

I'm working a little bit with Calimero. My personnal project is to provide a type of REST interface on top of "classical" IP connection

This is a simple PT that transform some int constant into Enum. It's easiest to used in code and avoid some recurrent check.

Regards Christophe

bmalinowsky commented 8 years ago

Thx, it's on feat/tunneling-enum as I will use the opportunity to remove some upper-casing

ouaibsky commented 8 years ago

ok. I had a look.

Java enum are usually in upper case but you're the boss :)

http://docs.oracle.com/javase/tutorial/java/javaOO/enum.html Because they are constants, the names of an enum type's fields are in uppercase letters.

Why did you convert byte to int ? because in the code where you need it it looks like you need byte

bmalinowsky commented 8 years ago

The elements/fields of an enum are upper-case, the convention never included the enum type. Anyway, the Sun guidelines are ancient and were written in '96?, probably still for Oak, with constants aligned to the predominant use of macros in C. I mostly write in C/C++, and there we also gradually moved away from the upper-casing (except for #defines maybe); C# never had it for enums.

Prefering a) classes that are (effectively) immuatable, b) final variables or c) class variables (static), none of those are distinguished in their naming. The emphasis of static final variables in that mix that shout constant (what UPPER_CASE basically does), does not serve much benefit. It's in the declaration, and the compiler enforces it. YMMV, and I might be totally off.

Why did you convert byte to int ? because in the code where you need it it looks like you need byte

The method getCode should return the correct code in any case, which would not be true for busmonitor layer if the return type of the signature is byte:

int code = TunnelingLayer.BusMonitorLayer.getCode();

would return -128. This is wrong, it should be 0x80. And almost everybody forgets the & 0xff widening to int. On the other hand, missing narrowing is always indicated by the compiler.

ouaibsky commented 8 years ago

Thx for the explanation even if I'm more or less up to date (I implemented various network protocol for market exchange) I meant at the end you want to write a byte on network, signed or unsigned it's 8 bits. Purpose of enum is to abstract this byte, So when you need it on your code (where it's not network stuff) you use the enum when you want send byte on network you call getCode or maybe to be renamed as getCodeAsSignedByte but at this stage your are supposed to know how Byte / Int are managed.

C.