patterncsharpMinor
Constructor for a Time object
Viewed 0 times
objectconstructorfortime
Problem
I am trying to write a constructor method that takes
The program works, but it feels very simple and amateurish. Is there anything I could do to make it more robust?
dd, hh, mm and ss as parameters and change it as a normal type of Time struct (e.g you enter 25 hours as the hh parameter, it changes it to 1 day and 1 hour).struct Time
{
public Time(int dd, int hh, int mm, int ss)
{
seconds= ss % 60;
minutes = mm % 60;
hours = hh % 24;
days = dd % 7;
if (ss > 60)
{
minutes++;
}
if (mm > 60)
{
hours++;
}
if (hh > 24)
{
days++;
}
}
private int days, hours, minutes, seconds;
public void Write()
{
Console.WriteLine("It has been " + days + " days, " + hours + " hours, " + minutes + " minutes, " + seconds + " seconds.");
}
}
class Program
{
static void Main(string[] args)
{
Time t = new Time(5, 28, 65, 55);
t.Write();
Console.ReadKey();
}
}The program works, but it feels very simple and amateurish. Is there anything I could do to make it more robust?
Solution
I can see several issues in your code:
-
Days
I guess the row with code
-
Boundaries
If you call
-
Incorrect additions
Try to call your constructor with
The correct logic should be something like this:
But, as Jamal mentioned, the constructor is for setting data members and other logic should be splitted into other method(s). That's why my example is not perfect from the design point of view. I also used your variable names in my example. I can imagine better names for them.
-
Days
I guess the row with code
days = dd % 7; is some historic rest of code when converting to weeks (?).-
Boundaries
If you call
Time(0, 0, 0, 60), the condition if (ss > 60) is not true and the addition minutes++; is not executed. You should use if (ss >= 60). The same error is repeated in other if statements.-
Incorrect additions
Try to call your constructor with
Time(0, 0, 0, 86400). If my understanding is correct, you should get 1 day, but you get only 1 minute.The correct logic should be something like this:
public Time(int dd, int hh, int mm, int ss)
{
seconds = ss % 60;
minutes = mm % 60;
hours = hh % 24;
days = dd;
if (ss >= 60)
{
minutes = mm + ss / 60;
}
if (minutes >= 60)
{
hours = hh + minutes / 60;
minutes %= 60;
}
if (hours >= 24)
{
days = dd + hours / 24;
hours %= 24;
}
}But, as Jamal mentioned, the constructor is for setting data members and other logic should be splitted into other method(s). That's why my example is not perfect from the design point of view. I also used your variable names in my example. I can imagine better names for them.
Code Snippets
public Time(int dd, int hh, int mm, int ss)
{
seconds = ss % 60;
minutes = mm % 60;
hours = hh % 24;
days = dd;
if (ss >= 60)
{
minutes = mm + ss / 60;
}
if (minutes >= 60)
{
hours = hh + minutes / 60;
minutes %= 60;
}
if (hours >= 24)
{
days = dd + hours / 24;
hours %= 24;
}
}Context
StackExchange Code Review Q#146884, answer score: 4
Revisions (0)
No revisions yet.