Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implemented Ultraviolet sensor value #26

Merged
merged 2 commits into from
Jun 24, 2017
Merged

Conversation

Bounz
Copy link
Collaborator

@Bounz Bounz commented Jun 24, 2017

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

Copy link
Member

@genemars genemars left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just wondering... are you using #Resharper or something? Why the underscore in front of private statics? I never understood that.

@Bounz
Copy link
Collaborator Author

Bounz commented Jun 24, 2017

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.

@Bounz Bounz merged commit 33753b3 into genielabs:master Jun 24, 2017
@genemars
Copy link
Member

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
Copy link
Collaborator Author

Bounz commented Jun 24, 2017

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
Copy link
Member

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
Copy link
Collaborator Author

Bounz commented Jun 24, 2017

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
Copy link
Member

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 😺

@Bounz
Copy link
Collaborator Author

Bounz commented Jul 18, 2017

Ok, your repo - your rules.

@Bounz
Copy link
Collaborator Author

Bounz commented Sep 27, 2017

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
Copy link
Member

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
Copy link
Collaborator Author

Bounz commented Sep 28, 2017

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants