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

Should refactor DateTime.Humanize #70

Closed
MehdiK opened this issue Jan 31, 2014 · 3 comments
Closed

Should refactor DateTime.Humanize #70

MehdiK opened this issue Jan 31, 2014 · 3 comments
Labels

Comments

@MehdiK
Copy link
Member

MehdiK commented Jan 31, 2014

DateTime.Humanize has gotten a bit complex over time. It needs to be refactored.

@wahidshalaly
Copy link
Contributor

I'm thinking to do the following

  1. No need for two methods to show either xxxAgo() or xxxFromNow(). One method with a flag (isFuture) is enough & method should use the proper format from resources.
  2. No need for two methods to show either Singlexxx() or Multiplexxx(int count). Again, one method with input (with default = 1) is enough
  3. Not sure about this yet, but I'm thinking we should also refactor the DefaultFormatter
  4. Split it into two formatters: DateTimeFormatter & TimeSpanFormatter
  5. Shorten names by removing prefixes DateHumaniz_xxx & TimeSpanHumanize_xxx

@MehdiK
Copy link
Member Author

MehdiK commented Mar 14, 2014

Great ideas. This issue is however only about refactoring the implementation of DateTime.Humanize extension: the method is around 70 LoC & IMO a method over 20 lines is already too long!!

You're talking about formatters which is a different issue. So I created #102 and copied your thoughts over. I think refactoring formatters could make this method a bit cleaner but I still think we need to have much fewer if conditions and lines of code in this method.

@MehdiK
Copy link
Member Author

MehdiK commented Mar 23, 2014

Some love has been given to this in #106 but it still could do with more.

@MehdiK MehdiK closed this as completed Apr 3, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants