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

New user experience - method naming #162

Closed
andig opened this issue Dec 28, 2022 · 9 comments
Closed

New user experience - method naming #162

andig opened this issue Dec 28, 2022 · 9 comments
Labels
Question Indicates that an issue, pull request, or discussion needs more information

Comments

@andig
Copy link

andig commented Dec 28, 2022

Thank you very much for this library! I came here from https://github.com/jinzhu/now and found many functions I was missing.

I'd like to share what made the start difficult for me- naming. The naming convention could imho be more consistent and go-ish. Please allow me to make three breaking suggestions which would imho increase consistency and usability:

  • carbon.Time2Carbon stutters- carbon.FromTime seems to be the natural name
  • same reverse- c.ToTime instead of the odd c.Carbon2Time (why repeat the Carbon when you already have a carbon instance or us 2 when other functions use To?)
  • all carbon.CreateXxx could become carbon.Xxx wich is a little less verbose
@andig andig added the Question Indicates that an issue, pull request, or discussion needs more information label Dec 28, 2022
@gouguoyin
Copy link
Member

Thank you for your feedback. Here is the reply to your feedback.

@gouguoyin
Copy link
Member

gouguoyin commented Dec 29, 2022

For the naming of carbon.CreateFromXxx, I have referred to the naming rules of other languages or third party libraries such as
carbon

$year = 2000; $month = 4; $day = 19;
$hour = 20; $minute = 30; $second = 15; $tz = 'Europe/Madrid';
echo Carbon::createFromDate($year, $month, $day, $tz)."\n";
echo Carbon::createMidnightDate($year, $month, $day, $tz)."\n";
echo Carbon::createFromTime($hour, $minute, $second, $tz)."\n";
echo Carbon::createFromTimeString("$hour:$minute:$second", $tz)."\n";
echo Carbon::create($year, $month, $day, $hour, $minute, $second, $tz)."\n";

php

$date=date_create_from_format("j-M-Y","25-Sep-2016");

c#

COMVariant theDay; 
theDay = COMVariant::createFromDate(today()); 
info(theDay.toString());

In addition, in terms of English grammar, carbon.CreateFromXxx is more legal and semantic than carbon.FromXxx. If carbon.CreateFromXxx is renamed as carbon.FromXxx, carbon.ParseByXxx is renamed as carbon.ByXxx ?

@gouguoyin
Copy link
Member

gouguoyin commented Dec 29, 2022

Carbon.Carbon2Time is not named carbon.FromTime to avoid ambiguity with carbon.CreateFromTime

Carbon.Time2Carbon is not named carbon.ToTime to avoid ambiguity with carbon.ToTimeString and carbon.Time

In addition,Carbon.Carbon2Time or Carbon.Time2Carbon is the conversion between two different structures of carbon and golang time. I don't think it is appropriate to use Carbon.FromTime or Carbon.ToTime

@andig
Copy link
Author

andig commented Dec 30, 2022

I did not mean to imply that the naming is wrong or not consistent with the carbon ancestry. What I was trying to say is that it feels alien for me as a somewhat experienced Go developer. Your milage may vary of course.

Carbon.Carbon2Time is not named carbon.FromTime to avoid ambiguity with carbon.CreateFromTime

All Create methods assemble a time. FromTime would be a straight conversion between both types.

Carbon.Time2Carbon is not named carbon.ToTime to avoid ambiguity with carbon.ToTimeString and carbon.Time

I meant to suggest carbon.FromTime. The idiomatic way in Go to convert to string seems to be String(). With different formats probably TimeString() etc. I've not found examples of prefixing those with To.

I don't think it is appropriate to use Carbon.FromTime or Carbon.ToTime

It would take away the "stuttering" which is a comment that regularly appears in Go repo's issue discussion.

All that said, no need to change anything. I was just sharing my experience, happy to have this issue closed if you want.

@gouguoyin
Copy link
Member

gouguoyin commented Dec 30, 2022

Thanks for sharing your experience, the naming of method is really a headache, maybe GPT-AI can solve this problem in the future.

@gouguoyin
Copy link
Member

gouguoyin commented Dec 30, 2022

If possible, help me review whether the German translation resources de.json are correct.

@andig
Copy link
Author

andig commented Dec 30, 2022

LGTM. Depending on context now is jetzt. gerade eben is the moment that has just passed while jetzt is now. Both may be valid:

  • it has happened just now gerade eben
  • it is happening right now jetzt

@gouguoyin
Copy link
Member

gouguoyin commented Jan 4, 2023

Thank you very much, can you create a pr for de.json?

@gouguoyin gouguoyin reopened this Jan 7, 2023
@gouguoyin
Copy link
Member

gouguoyin commented Jan 7, 2023

v2.2.3 released and added FromStdTime alias method of Time2Carbon method and ToStdTime alias method of Carbon2Time method, Time2Carbon and Carbon2Time will be removed in the future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Question Indicates that an issue, pull request, or discussion needs more information
Development

No branches or pull requests

2 participants