-
Notifications
You must be signed in to change notification settings - Fork 18k
time: add weekday helper functions #25469
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
Comments
The behavior I infer from the "Possible Solution" is simple modular arithmetic and it's directly supported by the language with no need for conversions: https://play.golang.org/p/v6XOucCpgUW. Also, I don't see enough value in having the two special cases, |
cznic i see enought logic in your snippet of code... i leave up to you the decision to implement it as an function or not in the future releases of golang. It was only my humble suggestion and i think my words were very clear. i don't see why not, neither also you explain to me why not. |
func prevWeekday(d time.Weekday) time.Weekday { return (d - 1) % 7 } and please, you want to every person (in every project) write these two functions over and over again? |
is not my way to feel how "we must avoid to rebuild the whell", i insist to reflect on this cznic. |
also reconsider the fact that we rely on abstractions in order build bigger system... so, nextWeekday and prevWeekday are part of those abstractions... after all, to implement a function like that in golang i have study before the golang convention for weekdays values (implying to be aware of any convention changes) and that take me time... so i only want to spare some time to others so they (as i) can spend it in more important things that are "above" this problem's domain. Just try to stay focus on our natural task: build abstrations, the right ones; and recall is not about complexity. |
FTR today := time.Now()
tomorrow := today.Add(time.Duration(24) * time.Hour) // adds a day duration
tomorrowWeekDay := tomorrow.WeekDay() is not a correct solution of the problem. |
i believe it works fine (and it does in prod) https://play.golang.org/p/lxbPySc8amm... i encourage you to explain why this is not a correct solution. |
The duration between weekday changes is not fixed at 24 hours in the general case. |
See also #19700. |
As @cznic notes, you can already iterate over weekday constants using the usual mathematical operators, and the expression For the specific problem of iterating over sequential days, this proposal doesn't seem to carry its weight: it would add API surface for trivial mathematical operations. For less-trivial use-cases, #19700 proposes a more comprehensive API. I'm closing this issue as a duplicate of that one. |
bcmills if i understand you right, you stated that it shall not be included because it is very simple logic. If that the case, let me point out clearly that we have diferent point of views here... i don't even put in trial the complexity of the algorithm, neither i care about how it is implemented. Please try to read "Context" section and please answer having in mind those points. I would appreciate very much that feedback, as a college or coworker point of view. Thanks you both (also cznic) for the reading and the feedback. |
New API always carries a cost. If we add a |
thanks for the reply ianlancetaylor. Allow me to highlight a previous question
I mean, i know that is very light work to do, but as far i stated this could be a problem to more than one client of the library, so, the question i would make myself as client of the library is ¿why delegate the weigth to each client instead of putting it in a single place (like the "time" library) and once for all?. |
Encapsulation of complex expression and/or logic often makes sense. Encapsulating trivial expressions (two operators, single non-constant operand) not so much. There is probably a lot if similar one-liners out there. Never will they all end in the stdlib, so someone must decide where to draw the line. |
So we all recognize that the complexity exists and is fact (regardless the level of complexity). I see that neither any of my motivations (explained in the Motivations and Context Sections) is not enough to promote this "issue/request" to a "nice to have feature", and is OK. I do have interest of knowing what do you think of each of one of them, without any rush to answer, of course. I do not know with how many work you have to deal with day by day, but for a start i will assume that is a lot. So I don't want to be annoying with this, and, if i'm about to be it, please excuse me. Thanks |
I think I would summarize your motivations and contents as saying that these new methods would be useful. I agree. Nobody would propose ideas that aren't useful. From my perspective, every change has both a benefit and a cost. We make changes where the benefit is larger than the cost. You've discussed the benefit. The various replies to you above discuss the cost. In other words, we agree about the benefits you've mentioned. Where we disagree is whether the benefits are worth the cost. |
And let me add that I believe that you guys are the ones that know the tradeoff of this, i mean, the cost and the benefits. Thanks ian, for your feedback, it was very useful to me in order to understand better what is really the matter here, and therefore the thing that, perhaps, may be put on the table for discussion someday. |
#Perhaps in future editions it would be nice to have a convenient function to get the following (or previous) week day of a given one.
Current Behaviour
As a client of time.Util library, for a given day if i rqquire to calculate the following one
Motivation
the
func (t Time) Weekday() Weekday
does non-linear calculations (i do take a look at the code and see the logic behind and it is not trivial, and the order of the function could match a binary search "O(log n)" as thefunc (l *Location) lookup(sec int64) (name string, offset int, isDST bool, start, end int64)
may be called during the process (look forl.lookup(sec)
in time.Time source file) that could be simplified in the context of iterating through weekdays in a circular fashion (either forward or backward).Possible Solution
The above was my implementation that works on my project.
As the language specs forbids: i can't use WeekDay as type receiver, thus, extending and binding the functionality to the concrete type; so i resort to write my own utility func.
Additional Test Code
Context
My motivation arises for three reasons:
time.WeekDay
type in order to retrieve the relative availability information for the correspondent day.I would love to receive some feedback about this including other professional's point of views.
Thanks and hope you find useful something of what i have just share.
The text was updated successfully, but these errors were encountered: