-
Notifications
You must be signed in to change notification settings - Fork 4
Add a new article about naming convention. #9
Conversation
I will assign @vonglasow for the review of this PR (if he agrees). |
<p> | ||
This is not something we will use often, but it is important to have a | ||
strict naming here. Based on this naming, the user will be able to choose | ||
if the resulting data must be cached or not (for instance, all to |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
all to conversions
=> all <code>to</code> conversions
You can complete for example with hoaproject/Central#54 (comment) but it looks good for me. |
ff25811
to
ccac8df
Compare
</p> | ||
<p> | ||
So far, we use this formalism: <code>getFoo()</code> to name a method | ||
that returns the value foo. This can be a direct attribute, or a |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add around foo also
</li> | ||
</ul> | ||
<p> | ||
Conversions prefixed as and into typically decrease abstraction, either |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add <code>
around as
and into
they are keyword needed to be highlighted
</ul> | ||
<p> | ||
Conversions prefixed as and into typically decrease abstraction, either | ||
exposing a view into the underlying representation (as) or deconstructing |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same here around <code>
for as and to few lines below around brackets
<p> | ||
Conversions prefixed as and into typically decrease abstraction, either | ||
exposing a view into the underlying representation (as) or deconstructing | ||
data into its underlying representation (into). Conversions prefixed to, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Conversions prefixed to -> Conversions prefixed to
eda08d9
to
e3039f2
Compare
Hello @vonglasow, Any news about this article ? Do you think it miss something ? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Excellent work, congrats!
Maybe we should say that we took inspiration from the Rust language?
and simplification</a>. | ||
</p> | ||
<p> | ||
So far, we use this formalism: <code>getFoo()</code> to name a method |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove parenthesis please.
</p> | ||
<h2>Property access</h2> | ||
<p> | ||
Initially we decided to remove the <code>get</code> prefix from getters |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
“getter methods”
<p> | ||
Initially we decided to remove the <code>get</code> prefix from getters | ||
methods because it's not mandatory to understand the code. Also in many | ||
languages this prefix have been drop/is not present so we can ask if it's |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
“has been dropped”
<p> | ||
After receiving some <a href="https://github.com/hoaproject/Central/issues/54#issuecomment-281268963">community | ||
feedbacks</a> it seems that this prefix is too present in the PHP world. | ||
Removing it will make code more complex to read and understand for PHP |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe not “more complex” but “less consistent”, thoughts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure here... The code will be less consistent without the prefix but it's more complex to read...
Maybe we can use :
"Remove it will make the code harder to read and understand ..." ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or just :
“Removing it will make the code harder to understand for most PHP developers.“
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
</p> | ||
<h2>Conversions</h2> | ||
<p> | ||
Conversions can be expensive so the method prefix must be clear enough to |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
“A conversation have a computing cost. Some of them can be expensive, so the method…”
<h2>Conversions</h2> | ||
<p> | ||
Conversions can be expensive so the method prefix must be clear enough to | ||
know operation cost directly inside the code. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
“directly by reading the code”
know operation cost directly inside the code. | ||
</p> | ||
<p> | ||
We decided to introduce 3 method prefixes: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
“We have decided”
e3039f2
to
a877ff4
Compare
We can add a line about Rust, I've found that link : https://aturon.github.io/style/naming.html Can't find a note about naming convention in the Rust documentation, is there something like that ? |
Let's use this link 👍! |
7f3da38
to
71247ad
Compare
Ok I've added the link in article introduction :
|
71247ad
to
ffddf52
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Waiting the final feedback from @vonglasow.
reading the code. | ||
</p> | ||
<p> | ||
We have decided to introduce 3 method prefixes: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Except this typo everything looks ok for me.
We have decided to introduce 3 method
s
prefixes:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just fix the last typo and everything is ok.
This article explain what was discussed in the RFC : hoaproject/Central#54
ffddf52
to
b26e34b
Compare
The typo is now fixed 😄, thank you for the review ! |
Hello @Hywan,
I've just started writing the article about naming convention. The content is still WIP and is actually nothing more than a transcription of issue body.
Is it the right way to create a new article ?