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

STYLE: Distinguish ivars from local vars #8

Merged
merged 6 commits into from
Feb 5, 2017

Conversation

hjmjohnson
Copy link
Contributor

@hjmjohnson hjmjohnson commented Feb 4, 2017

A common method for improving code readability
is to conform to a standard naming convention
for class & struct ivars. The two most prevalent
mechanisms for accomplishing this are:

  1. ITK/VTK and many other projects
    ivars beging with "m_" for object ivars, and "s_"
    for static class ivars.

  2. Google Style
    ivars are suffixed with "_"

This practice reduces burden of maintenance for
community developed software by inherently providing
contextual and intent information directly in the
variable name.

@hjmjohnson
Copy link
Contributor Author

@fbbdev Fabio,

I think this project has great potential. I am making recommendations early in the project regarding some style and conventions that will have long term benefits of cultivating a vibrant user and developer base.

Please consider this recommendation. If you are satisfied with these changes, I can work to implement them over the next few days.

Regards,
Hans

@fbbdev
Copy link
Owner

fbbdev commented Feb 4, 2017

Hans,

I'm fine with these changes as long as struct members and public class members retain normal names (as recommended by the Google Style Guide). So in structs (like Point, Rect and others) we should still change method argument names in order to avoid shadowing.

If you're fine with the above requirement, go ahead with the implementation; I will avoid changing class members until I get your pull request, in order to minimize conflicts.

Thank you for your interest in this project :)

@hjmjohnson
Copy link
Contributor Author

@fbbdev Fabulous. I think your proposal is perfectly balanced between pragmatic and conformant.

@hjmjohnson
Copy link
Contributor Author

@fbbdev
I want to bring your attention to:
https://google.github.io/styleguide/cppguide.html#Structs_vs._Classes

This is a fairly common approach to managing structs and classes.

I would recommend converting the "Color" struct to a class.

Do you have a counter argument for your distinction between classes and structs?

Hans

@hjmjohnson
Copy link
Contributor Author

@fbbdev

This now has private class ivars with suffix of "_".

A common method for improving code readability is to conform to a
standard naming convention for class & struct ivars.  The two most
prevalent mechanisms for accomplishing this are:

1) ITK/VTK and many other projects
  ivars beging with "m_" for object ivars, and "s_"
  for static class ivars.

2) Google Style
  ivars are suffixed with "_"

This practice reduces burden of maintenance for community developed
software by inherently providing contextual and intent information
directly in the variable name.

Using suffixed ivars for indicating private class members is adopted.

struct members and public class members retain normal names (i.e. not
suffixed) (as recommended by the Google Style Guide). So in structs
(like Point, Rect and others) we should still change method argument
names in order to avoid shadowing.
@fbbdev
Copy link
Owner

fbbdev commented Feb 4, 2017

@hjmjohnson I definitely agree with google's distinction. Yet I interpret it more liberally:

Methods should not provide behavior

Color is plain data and its methods do not provide behavior; they are operators, or, if you like, getters for computed properties. They could be implemented as external functions, but I made them into methods to make code more terse and allow quick pipeline expressions like e.g. some_color.premultiplied().green(0.4f).hue() in place of plot::hue(plot::green(plot::premultiplied(some_color), 0.4f)). Also, I generally lean towards a functional programming style where data is never mutated in place when not absolutely necessary.

A similar case is block_t in braille.hpp. Again block_t is just a data block and its methods except maybe clear and set implement unary or binary operations on said data. clear and set are tolerated there because they provide fine-grained access to pixel data.

On the other hand image_t is a container exposing memory allocation behavior, so I made it into a class.

I understand the phrase 'provide behavior' as if meaning 'have side effects' or 'implement control logic'.

@fbbdev fbbdev changed the title WIP: STYLE: Distinguish ivars from local vars STYLE: Distinguish ivars from local vars Feb 5, 2017
@fbbdev fbbdev merged commit 234701e into fbbdev:master Feb 5, 2017
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.

None yet

2 participants