principlecsharpMinor
Parsing and formatting a time two ways (perfectionistic approach)
Viewed 0 times
formattingperfectionisticwaystimetwoparsingandapproach
Problem
I'm still not satisfied by the readability of the code. The wanted behavior which would be self descriptive would sound like: "I've two fields; one is for the date and one for the hour. This two fields should be in a UserControl of which I have two instances, one is for the event time and one is for the event end time."
My personal opinion is all the extra code is something which is specific to C# syntax, and not to my purpose. Handling null, exceptions, conversions between dates and strings makes me to write that extra code and a developer could get confused and wonder on what was the purpose of that solution. The only thing which is at a functional level in this code is an assignment.
The
Which solution would you use?
First formulation
Second formulation
My personal opinion is all the extra code is something which is specific to C# syntax, and not to my purpose. Handling null, exceptions, conversions between dates and strings makes me to write that extra code and a developer could get confused and wonder on what was the purpose of that solution. The only thing which is at a functional level in this code is an assignment.
The
String.Format is an example of what I mean... it's a really mean solution. Now that every approach tries to separate Model, View and Controllers, I'm mixing the solution I found to to fill a gap of conversion and the code wrote to make the behavior I needed.Which solution would you use?
First formulation
public override void OnEventTimeChanged()
{
DateTime eventDateWithTime;
TimeSpan eventTime;
if (TimeSpan.TryParse(EventTime, out eventTime))
{
eventDateWithTime = (EventDate.Date + eventTime);
CultureInfo cultureInfo = Thread.CurrentThread.CurrentUICulture;
EndTime = String.Format(cultureInfo, "{0:HH:mm}", eventDateWithTime);
EndDate = eventDateWithTime;
}
}Second formulation
public override void OnEventTimeChanged()
{
CultureInfo cultureInfo = Thread.CurrentThread.CurrentUICulture;
TimeSpan eventTime;
if (!TimeSpan.TryParse(EventTime, out eventTime)) return;
DateTime eventDateWithTime = (EventDate.Date + eventTime);
string eventTimeString = String.Format(cultureInfo, "{0:HH:mm}", eventDateWithTime);
EndTime = eventTimeString;
EndDate = eventDateWithTime;
}Solution
In the 2nd version, the
Should I return from a function early or use an if statement?
Which way you choose, is a matter of taste. There's nothing wrong with either one.
If you do go with the 2nd one, it can be improved a little bit:
That is:
if acts as a guard: it prevents invalid states to get through it. Many people prefer it this way, for example in this related post: Should I return from a function early or use an if statement?
Which way you choose, is a matter of taste. There's nothing wrong with either one.
If you do go with the 2nd one, it can be improved a little bit:
public override void OnEventTimeChanged()
{
TimeSpan eventTime;
if (!TimeSpan.TryParse(EventTime, out eventTime))
{
return;
}
DateTime eventDateWithTime = EventDate.Date + eventTime;
CultureInfo cultureInfo = Thread.CurrentThread.CurrentUICulture;
string eventTimeString = String.Format(cultureInfo, "{0:HH:mm}", eventDateWithTime);
EndTime = eventTimeString;
EndDate = eventDateWithTime;
}That is:
cultureInfois not needed in theif, so declare and initialize it later. Even more precisely, declare and initialize it right before you really need it.
- It's recommended to use braces with
ifstatements always, even if only one line will go inside the block. Remember this bug: https://www.imperialviolet.org/2014/02/22/applebug.html
Code Snippets
public override void OnEventTimeChanged()
{
TimeSpan eventTime;
if (!TimeSpan.TryParse(EventTime, out eventTime))
{
return;
}
DateTime eventDateWithTime = EventDate.Date + eventTime;
CultureInfo cultureInfo = Thread.CurrentThread.CurrentUICulture;
string eventTimeString = String.Format(cultureInfo, "{0:HH:mm}", eventDateWithTime);
EndTime = eventTimeString;
EndDate = eventDateWithTime;
}Context
StackExchange Code Review Q#63030, answer score: 4
Revisions (0)
No revisions yet.