search

csharpvitamins / CSharpVitamins.ShortGuid

A convenience wrapper for dealing with base64 encoded Guids
MIT License
103 stars 20 forks source link

Update ShortGuid.cs #2

Closed vanillajonathan closed 4 years ago

vanillajonathan commented 4 years ago

Explicitly declare property visibility. Proper capitalization of Base64. Proper punctuation.

vanillajonathan commented 4 years ago

@davetransom ping!

davetransom commented 4 years ago

Sorry @vanillajonathan, I forgot about this one when I didn't quite know how I should reply straight away.

The documentation corrections are worth accepting (and thank you), however, reorganising the struct is not something I'd merge as I've written it the way I want it.

I'll happily merge if you revert the stylistic changes.

vanillajonathan commented 4 years ago

But members of a class should be have theit member visibility explicitly declared, and if not explicitly declared as private, they are implicitly internal, and in this case I believe they should be private. Also private and internal members should be placed above public members, and non-static members should be placed above static members according to the Microsoft Framework Design Guidelines.

davetransom commented 4 years ago

Are you confusing default access modifiers for class (internal) vs members (private)?

My previous comment still stands.

vanillajonathan commented 4 years ago

Yes. I am confusing default access modifiers.

I updated the patch.