4
\$\begingroup\$

I hope the title is clear. The objective is as following:

There are 4 clocks in a hotel in London. One is showing the time for London. The others for New York, Tokyo and Hong Kong.

Generate a random time for the clock in London, with that result, determine the hour for New York(+5), Tokyo(-9) and Hong Kong(-8). Then show the results.

This is not homework, this is practice. Is this a good way? Or can i generate a random timespan? All input will be appreciated!

Random generator = new Random();

DateTime london = DateTime.Now;

DateTime timeInLondon = london.AddMinutes(generator.Next());

DateTime timeInNewYork = timeInLondon.AddHours(5);
DateTime timeInTokyo = timeInLondon.AddHours(-9);
DateTime timeInHongKong = timeInLondon.AddHours(-8);

Console.WriteLine("London: " +london.ToShortTimeString() +"\n" +"New York: " +timeInNewYork.ToShortTimeString() + "\n" +"Tokyo: " +timeInTokyo.ToShortTimeString() + "\n" +"Hong Kong: " +timeInHongKong.ToShortTimeString());
\$\endgroup\$
5
\$\begingroup\$
DateTime london = DateTime.Now;

DateTime timeInLondon = london.AddMinutes(generator.Next());

This can be rewritten as

var timeInLondon = DateTime.Now.AddMinutes(generator.Next());

You can use var on the other lines as well.

Removing the london variable makes it less likely you make mistakes like this:

Console.WriteLine("London: " +london.ToShortTimeString()...

Where you actually output DateTime.Now as it was at the start of your program, but not, as you probably wanted, the randomized timeInLondon!

Now about your random time.

Your seconds will obviously not be random. Apart from that, you only add a positive random number, with a maximum of around 4085 years. As such, I guess that is enough to make the resulting time more or less random as far as hours and minutes are concerned. I would opt for adding seconds though. You still would add anything up to 68 years to the current time, still probably ending up with a random-enough distribution over the 24-hour clock, including seconds:

var timeInLondon = DateTime.Now.AddSeconds(generator.Next());

Finally, if you want to nitpick, it would be better to use time zone information instead of hard-coding magic numbers like 5 or -8. Time zones do not at all times have the same number of hours difference between them(*).

You could use TimeZoneInfo.ConvertTime Method (DateTime, TimeZoneInfo, TimeZoneInfo)

Keep in mind that if you opt for that, you might need to rethink randomisation of your time, since the date comes into play to determine the time in other places.


(*)Not all countries start and end DST on the same dates, some countries do not even use DST, and on the southern hemisphere, DST goes "the other way around". This may all be irrelevant for the locations you picked, though.

\$\endgroup\$
4
  • \$\begingroup\$ Hello @Oerkelens, thank you very much! I didn't even realize that mistake until you mentioned it :P. Is it common practice to use var instead of the type of the variable? I don't understand why you would use var instead of DateTime. \$\endgroup\$
    – user146857
    Aug 17 '17 at 22:30
  • \$\begingroup\$ Using var is advocated by Microsoft and included in many coding guideline. It it shorter (usually, in case of int it's not), saves typing and I find that it generally makes life easier :) "Dictionary<string, Dictionary<int, double>> someDictionary = new Dictionary<string, Dictionary<int, double>>();" becomes all the more readable as "var someDictionary = new Dictionary<string, Dictionary<int, double>>(); \$\endgroup\$
    – oerkelens
    Aug 17 '17 at 22:36
  • 1
    \$\begingroup\$ Hmm.. indeed! So i guess that if you use var, that the compiler determines the type of the variable. Just did a quick google search, so you can use var for local variables and when the type of the variable is obvious. \$\endgroup\$
    – user146857
    Aug 17 '17 at 22:53
  • \$\begingroup\$ @IliasJoels - I find it's more readable the way you did it with the helper variable DateTime london = DateTime.Now; then DateTime.Now.AddMinutes(generator.Next()); and in fact I do it my self all the time. Otherwise you don't know what Now means. \$\endgroup\$
    – t3chb0t
    Aug 18 '17 at 4:21
3
\$\begingroup\$

This is a very naive solution. There's more to consider than just the hours offset. What happens when it's daylight savings time in London, but not in New York? You'll be off by an hour. You can manage all of this by using a DateTimeOffset, which includes the hours offset from UTC, and a bunch of logic instead of a plain DateTime, but it's really better to use a library like NodaTime for this.

\$\endgroup\$
4
  • \$\begingroup\$ While i do agree, we can only use what we learned thus far. That's probably why in the objective the teacher didn't ask to consider daylight savings time and why she has already given the time differences between the different time zones. \$\endgroup\$
    – user146857
    Aug 17 '17 at 22:34
  • \$\begingroup\$ Well then, go back and blow your teacher away by asking "What about Daylight savings time?" \$\endgroup\$
    – RubberDuck
    Aug 17 '17 at 23:22
  • 1
    \$\begingroup\$ I thought you said that this was not homework? We're trying to give you the best advice here, regardless of what your teacher says. \$\endgroup\$ Aug 18 '17 at 0:19
  • \$\begingroup\$ It isn't homework. It is just a practice bundle made by the teacher. I truly appreciate the advice and it was all good advice! \$\endgroup\$
    – user146857
    Aug 18 '17 at 1:06
0
\$\begingroup\$

I find you did well. Especially using the helper variables to give DateTime.Now a meaning. As the other answer suggested it's technically possible to use DateTime.Now.AddMinutes but then you don't know what Now is. Using helper variables is always a good thing and you should keep it that way. Don't make it short just because it's possible. It's better to write a little bit more and still know what you did few weeks later.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy