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

Simplify getter naming, introduce data conversion naming #54

Open
Hywan opened this Issue Feb 14, 2017 · 10 comments

Comments

@Hywan
Copy link
Member

Hywan commented Feb 14, 2017

Hello fellow @hoaproject/hoackers and users!

In this RFC, I would like to simplify and standardize the API to get data.

Introduction

So far, we use this formalism: getFoo() to name a method that returns the value foo. This can be a direct attribute, or a computation. To get this data within another form, i.e. to convert this data into another type, we don't have any formalism yet. For instance, if foo is an array, and we would like to get it as a string, we will probably name a method like getFooAsString() but this is not deterministic.

Shorten getter methods

First proposal is to remove the get prefix for all getters. So getFoo() will be renamed like foo(). Real example:

$production->collection()->title();

instead of:

$production->getCollection()->getTitle();

The meaning is strictly the same, but I think the former is easier and shorter to read.

However, the setFoo() formalism for setters will remain. I would like to see set_foo() but it does not respect PSR, so let's not start a war 😉.

Conversions

I would like to introduce 3 method prefixes:

Prefix Cost Consumes convertee
as Free No
to Expensive No
into Variable Probably

Example:

  • Let's $x be a float. asInteger() will be (almost) free.
  • Let's $x be an array. toString() will be expensive because we have to iterate over the array, to allocate a string, and to convert every pairs in the array as a string (like a serializer).
  • Let's $x be an object. intoArray() will not be that expensive, it might reference all attributes into an array, so that's just one allocation.

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, on the other hand, typically stay at the same level of abstraction but do some work to change one representation into another.

Outro

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 conversions are likely to be cached because they might be expensive).

Thoughts?

@Hywan Hywan added this to the Roadmap for 2017 milestone Feb 14, 2017

@Hywan Hywan self-assigned this Feb 15, 2017

@Hywan Hywan added the enhancement label Feb 15, 2017

@Hywan Hywan changed the title No more `get*`, introduce `as*`, `to*` and `into*` Simplify getter namings, introduce data conversion namings Feb 21, 2017

@Hywan Hywan changed the title Simplify getter namings, introduce data conversion namings Simplify getter naming, introduce data conversion naming Feb 21, 2017

@Swader

This comment has been minimized.

Copy link

Swader commented Feb 21, 2017

First proposal is to remove the get prefix for all getters.

Okay, but why exactly? What's wrong with the current approach? I think it's more consistent, predictable, and fits in well with the setter naming.

Let's $x be an array. toString() will be expensive because we have to iterate over the array, to allocate a string, and to convert every pairs in the array as a string (like a serializer).

How does this work with multi dimensional arrays? Would this conversion be predictable / clear / standardized? The method name does not accurately convey the nature of the conversion - I wouldn't be sure what to expect in the final string.

@Hywan

This comment has been minimized.

Copy link
Member

Hywan commented Feb 21, 2017

@Swader Why removing the get prefix. Why not. In many languages I tried recently, this prefix has been drop/is not present, and I didn't feel any blockers. So from my point of the view, it becomes useless. And this is more “data-centric”/“data-oriented”. A class is a structure, we ask data on it, no need for a get prefix. We might have “conflicts” with action methods, like map, which is not a getter. But it works in collaboration with RFC #56. For instance:

$product->collection()->isSome()

is strictly equivalent to:

$production->hasCollection()

The former is more readable from my point of view. The API is unified more importantly.

To answer:

What's wrong with the current approach?

What's wrong: It's heavy, and not necessary. The get prefix is not mandatory to understand the code.

Now, about the toString(): This is an example. The to prefix indicates an expensive conversions, not the form of the results. It warns the user that (i) this is a conversion, (ii) it is costly, (iii) then it might cache the results (i.e. avoid calling it in a loop for instance if possible), (iv) the result is a string. The suffix is not necessarily a type name, toXmlRpcMessage is still valid. The important part is to. So in your case, toJson will provide more information than toString, even if we don't know what JSON representations we are using (strings, objects?). Maybe toJsonString is better. Note, having a real generic serializer in this case is even better 😉.

The goal is to define a naming convention to indicate the cost of conversions, and the abstraction changes. I think this is useful.

@shulard

This comment has been minimized.

Copy link
Contributor

shulard commented Feb 21, 2017

Helping developers knowing the cost of an operation by convention naming is a very nice idea 👍.

Also the collaboration with RFC #56 and getter naming simplification make the code simpler to read for me.

But what about existing code ? Once implemented this will introduce a BC break for most of the getter methods... Do you think the code must be compatible for some time ?

@Hywan

This comment has been minimized.

Copy link
Member

Hywan commented Feb 21, 2017

Nop. Since we are thinking to drop PHP 5.5, we will introduce a big BC, like everytime we drop a PHP version. The goal is then to take this opportunity to rewrite the API.

@theofidry

This comment has been minimized.

Copy link

theofidry commented Feb 21, 2017

I can understand where this is coming from, but IMO it's too alien to most PHP developers and the BC break introduced by it is not worth it. It only hurts the adoption of Hoa projects.

@Hywan

This comment has been minimized.

Copy link
Member

Hywan commented Feb 21, 2017

@theofidry So just like @Swader you suggest to keep the get prefix, but there is no issue to introduce the conversion prefixes?

@Swader

This comment has been minimized.

Copy link

Swader commented Feb 21, 2017

Your example about map() conflicting is something I didn't even think of - that's another reason not to drop it, definitely. But yes, the prefixes for conversions sound good.

@Hywan

This comment has been minimized.

Copy link
Member

Hywan commented Feb 21, 2017

2 persons external from the Hoa project with a good influence in the PHP world telling me to not drop the get prefix is enough for me —so far— to change my mind.

Thank you for your feedback!

@Hywan Hywan added the ready label Mar 10, 2017

@shulard

This comment has been minimized.

Copy link
Contributor

shulard commented May 19, 2017

Since we choose to don't remove the get prefix, do you think there is something important to do here ? I looked at some Hoa libraries and haven't found methods with invalid convention.
Maybe writing a guide about the naming convention on https://hoa-project.net/ can be nice (or just an article ?). if you agree, I can work on that content 😄.

@Hywan

This comment has been minimized.

Copy link
Member

Hywan commented May 24, 2017

👍 go!

shulard added a commit to shulard/Blog that referenced this issue Jun 20, 2017

Add a new article about naming convention.
This article explain what was discussed in the RFC : hoaproject/Central#54

shulard added a commit to shulard/Blog that referenced this issue Jun 20, 2017

Add a new article about naming convention.
This article explain what was discussed in the RFC : hoaproject/Central#54

shulard added a commit to shulard/Blog that referenced this issue Jun 20, 2017

Add a new article about naming convention.
This article explain what was discussed in the RFC : hoaproject/Central#54

shulard added a commit to shulard/Blog that referenced this issue Jun 30, 2017

Add a new article about naming convention.
This article explain what was discussed in the RFC : hoaproject/Central#54

shulard added a commit to shulard/Blog that referenced this issue Jun 30, 2017

Add a new article about naming convention.
This article explain what was discussed in the RFC : hoaproject/Central#54

shulard added a commit to shulard/Blog that referenced this issue Jun 30, 2017

Add a new article about naming convention.
This article explain what was discussed in the RFC : hoaproject/Central#54

shulard added a commit to shulard/Blog that referenced this issue Jun 30, 2017

Add a new article about naming convention.
This article explain what was discussed in the RFC : hoaproject/Central#54

@Hywan Hywan added in progress and removed ready labels Jul 4, 2017

shulard added a commit to shulard/Blog that referenced this issue Jul 5, 2017

Add a new article about naming convention.
This article explain what was discussed in the RFC : hoaproject/Central#54

@Hywan Hywan referenced this issue Aug 17, 2017

Open

steps for next version - bc break #74

0 of 5 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment