search

genielabs / zwave-lib-dotnet

Z-Wave Home Automation library for .NET / Mono
Apache License 2.0
63 stars 37 forks source link

Implemented Ultraviolet sensor value #26

Closed Bounz closed 7 years ago

Bounz commented 7 years ago

Implemented Ultraviolet sensor value. Added ability to invoke arbitrary z-wave command in test application

Bounz commented 7 years ago

Because it's very useful - you can easily distinguish between instance fields and parameters passed to the method, especially if your methods are quite big. This style was used at both my places of work and is used by the .Net Core team, too (https://github.com/dotnet/corefx/blob/master/Documentation/coding-guidelines/coding-style.md). And yes, I use R# and like this tool very much.

genemars commented 7 years ago

Ok I'm asking because I was adopting too this convention time go, but then I decided to drop it because modern IDEs should already tell you if the field is global or local by using different colors. Anyway... it's ok =) I also noticed you made same kind of changes in HG code. So let's say we adopt this convention now on.

Bounz commented 7 years ago

Are you using R#? We can include .DotSettings file into the repository, so it'll be easier to synchronize code style. Also, what do you think about using C#6 language features?

genemars commented 7 years ago

I am using Linux so R# is not available, but I think that Rider can do it anyway: https://www.jetbrains.com/rider/features/ So ok for .DotSettings file. I didn't look into CS6 yet, but it's ok for me as long as it doesn't break compatibility with mono (let's say 3.2 or 4.0 and up).

Bounz commented 7 years ago

Looks like C#6 was implemented in Mono 4.0, so I'll use the new syntax in future work. http://www.mono-project.com/docs/about-mono/releases/4.0.0/ I'm also using Rider when coding on Mac, but it's not so smooth as Visual Studio meanwhile. Looking forward to a stable Release of Rider and hope it will become more stable and handy.

genemars commented 7 years ago

Hi @Bounz I've been thinking more about this prefixing convention and even if .Net Core team is adopting this, I don't feel like it is a wise decision. This is not needed anymore and will just create confusion and aesthetic issues in the code that was already refactored in the past. So this convention is not valid here. Whenever you have time/feel like to, please refactor both ZWaveLlib and in HomeGenie 1.2-wip branch by dropping all prefixes. Thanks :smiley_cat:

Bounz commented 7 years ago

Ok, your repo - your rules.

Bounz commented 6 years ago

One more thing to say about naming conventions. I was looking through the code of MIG library and found this bit of code: image At the first glance, it looks that lastAddedNode is just a local variable or method argument. But no, it's a private field in the class that may be modified somewhere else in the code, but it looks similar to returnValue local variable. The only help here is that Rider colors them differently, but in other editors, it looks completely confusing.

genemars commented 6 years ago

returnValue is white.... lastAddedNode is magenta. Isnt't it? Also MonoDevelop employs different colors and Visual Studio too. What other editors are you talking about?

Bounz commented 6 years ago

Maybe we have different color schemes, but here is what I see in Visual Studio 2017: image

And this is how it looks in VS Code: image

Anyway, never mind, I was just thinking out loud.