hoaproject / Central

Hoa is a modular, extensible, and structured set of PHP libraries.
https://hoa-project.net/
Other
104 stars 8 forks source link

Simplify getter naming, introduce data conversion naming #54

Open Hywan opened 7 years ago

Hywan commented 7 years ago

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 :wink:.

Conversions

I would like to introduce 3 method prefixes:

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

Example:

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?

Swader commented 7 years ago

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 commented 7 years ago

@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 :wink:.

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

shulard commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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

Swader commented 7 years ago

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 commented 7 years ago

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!

shulard commented 7 years ago

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 commented 7 years ago

:+1: go!