Designing Classes CONFUSES Me

[Vel Williams]

Hello everyone!

I'm in my first year of coding, and I try to read everything, but I retain nothing when it comes to creating code which has been a very annoying problem.

class Time {
    private long hour;
    private long minute;
    private long second;
    private long milliseconds;

    public Time() {
    }

    public Time(long milliseconds) {
        this.milliseconds = System.currentTimeMillis();
    }

    public Time(long hour, long minute, long second) {
        this.hour = hour;
        this.minute = minute;
        this.second = second;
    }

    public long getHour() {
        return hour;
    }
    public long getMinute() {
        return minute;
    }
    public long getSecond() {
        return second;
    }

    public static long setTime(long elapseTime) {
        long totalMilliseconds = System.currentTimeMillis();
        long totalSeconds = totalMilliseconds / 1000;
        long second = totalSeconds % 60;
        long totalMinutes = totalSeconds / 60;
        long minute = totalMinutes % 60;
        long totalHours = totalMinutes / 60;
        long hour = totalHours % 24;

        return elapseTime;
    }
}

Here are the items the class is supposed to contain and where I think I created them:
The data fields hour, minute, and second that represent a time.
Lines 16 - 20. I added milliseconds so I could convert the System.currentTimeMillis() to hours, seconds and minutes later
A no-arg constructor that creates a Time object for the current time. (The values of the data fields will represent the current time)
Lines 22-23
A constructor that constructs a Time object with a specified elapsed time since midnight, January 1, 1970, in milliseconds. (The values of the data fields will represent this time)
Lines 25-27
A constructor that constructs a Time object with the specified hour, minute, and second.
Lines 29-33
Three getter methods for the data fields hour, minute, and second, respectively.
Lines 35-43
A method named setTime(long elapseTime) that sets a new time for the object using the elapsed time. For example, if the elapsed time is 555550000 milliseconds, the hour is 10, the minute is 19, and the second is 10
Lines 45-55

If all of these are correct, then I believe my next step is to debug my setTime method so it will actually return the correct elapseTime and allow the code to execute correctly. Any clarification, tips or tricks are greatly appreciated! This is usually what takes me hours to figure out.

[Ron McLeod]

Welcome to CodeRanch!

Please provide the code as text rather than an image.

Thanks.

[Vel Williams]

Fixed Ron, thank you! Just learning how cool this forum is.

[Carey Brown]

Your first constructor does nothing, which is not correct.
Your second constructor does not use the argument passed in.
The setTime() method calculates the time parts but never sets them. It should not be  'static'.

[Carey Brown]

Hours, minutes, and seconds should probably be ints.
Millis is probably used in the internals of some methods but should not be a member variable.
A toString() method may not be required but is useful for debugging.

[Carey Brown]

You can use one constructor to help build another.

    public Time() {
    	this( System.currentTimeMillis() );
    }

[Paul Clapham]

Why is your instance variable milliseconds a plural noun whereas the other three are singular nouns? That makes it even more difficult for me to decide whether your Time class is supposed to represent an instant in time or an interval of time.

Although the blue text in your original post is already unhelpful because it seems to be both, or neither. A Java class should have a description of what its instances represent, and it should preferably include comments which explain that. That blue text is from somewhere else, then? Did you quote it exactly or have you interpreted the requirements in this way?

[Norm Radder]

Paul Clapham wrote: A Java class should have a description of what its instances represent, and it should preferably include comments which explain that.

That is the first time I have seen anyone on this forum say anything positive about commenting a program.

[Tim Holloway]

Norm Radder wrote:

Paul Clapham wrote: A Java class should have a description of what its instances represent, and it should preferably include comments which explain that.

That is the first time I have seen anyone on this forum say anything positive about commenting a program.

Some of us believe that JavaDoc isn't just for Oracle-supplied code.

Also, some of us like to do what Donald Knuth labelled "literate programming". Where you write the comments first. then implement the code accordingly.

Yes, I'll confess that I was largely influenced by the pre-OOP programming languages where it was harder to tell what was going on just by looking at a chunk of code. Especially a large chunk of spaghetti. In assembly language. But I still find the practices  I developed useful.

If you read the docs on Javadocs, you'll see that not only do they describe the different annotations that result in the various documentation elements, but also set guidelines on how to write things. For example, the first sentence of a class or method comment should be a short description, one shouldn't have to be a classically-educated polyglot to be able to comprehend, and so forth.

Note also that it is common for IDEs to encourage JavaDocs by having a high-level menu that facilitate running the JavaDoc processor on a project.

My own rules include "tell what the class/method is FOR and NOT how it does things", as Java code is sufficiently self-descriptive for that. Also, from Calculus, what the "domain" and "range" of a resource is. Meaning, what's legal input, what's expected output AND what will happen if illegal input is given.

If you have ever noticed, when I post code here on the Ranch, unless it's a simple snippet, it will have JavaDoc comments on it.

[Campbell Ritchie]

Carey Brown wrote:. . . Millis is probably used in the internals of some methods but should not be a member variable. . . .

Unfortunately, it says to record the milliseconds since 1st January 1970. That means the class has two mutually incompatible purposes, one being to record hours minutes seconds and the other to record milliseconds alone. I see Paul has already said that. We cannot blame OP for that, but whoever set the exercise. In order to record milliseconds, you would need a milliseconds field
Another thing: I would remove that static method. It looks good, but it cannot do anything to your obkects, and I think it should be moved into a different class. It doesn't constitute any part of Time, but it does seem to constitute a way to test a Time object, which is different.

[Mike Simmons]

Campbell Ritchie wrote:

Carey Brown wrote:. . . Millis is probably used in the internals of some methods but should not be a member variable. . . .

Unfortunately, it says to record the milliseconds since 1st January 1970. That means the class has two mutually incompatible purposes, one being to record hours minutes seconds and the other to record milliseconds alone. I see Paul has already said that. We cannot blame OP for that, but whoever set the exercise. In order to record milliseconds, you would need a milliseconds field

Paul was saying something different, I believe, about how it's unclear whether the class represents a specific time or an interval.   The fact that it can be constructed from milliseconds since midnight GMT 1970-01-01 suggest to me it's a specific time.

Note that the instructions repeat the list of fields several times, omitting milliseconds.  Milliseconds are only mentioned in one constructor, and one setTime() method.  This suggest to me that you can create a time from milliseconds, but you don't need to be able to report them later.  That means you don't need to store milliseconds as a field.  There is an open question as to how to handle milliseconds that are not a multiple of 1000.  They could be truncated or rounded.  Or, seconds could be represented as a double.  These are design decisions, if they're not specified in the requirements.

Campbell Ritchie wrote:Another thing: I would remove that static method. It looks good, but it cannot do anything to your obkects, and I think it should be moved into a different class. It doesn't constitute any part of Time, but it does seem to constitute a way to test a Time object, which is different.

I think this misses the point.  The method, as written, doesn't do anything really, just manipulate local variables which are then forgotten.  Extracting it to another class will continue to not do anything useful.  Carey's comments are more useful: make the method non-static.  And the method should then use its non-static nature to allow it to set fields, not local variables.  Then, it would be doing something useful.

[Tim Holloway]

A high-level note:

This class is designed to hold a time of no definite reference. That is, it does not take into account whether this is the current time of day, UTC time of day, or some other time odf day (which can be a real issue for webapps with users in multiple timezones). Although in certain usages, it would be UTC-relative.

Also, fatally, the milliseconds returned grom System.getMilliseconds isn't simply milliseconds, it's the number of total milliseconds since the epoch time: midnight, January 1, 1970 UTC.

So the class properties hours, minutes and seconds are apples, but milliseconds is an orange. To be totally consistent, the system.getMilliseconds() results need to be extracted from the system time milliseconds.

[Campbell Ritchie]

Tim Holloway wrote:. . . This class is designed to hold a time of no definite reference. . . .

What's wrong with that? It would make it too complicated to include time zones, and what we have here is a very simplified version of LocalTime. Only, as you will have seen from the Java™ Tutorials link, LocalTime is immutable and doesn't have any setXXX() methods.

[Tim Holloway]

Campbell Ritchie wrote:

Tim Holloway wrote:. . . This class is designed to hold a time of no definite reference. . . .

What's wrong with that? It would make it too complicated to include time zones, and what we have here is a very simplified version of LocalTime. Only, as you will have seen from the Java™ Tutorials link, LocalTime is immutable and doesn't have any setXXX() methods.

I didn't say it needed to. Just pointed out that it has specific characteristics which should be kept in mind when using it.

And that it wasn't consistent, since part and only part of it was not only timezone-specific (UTC), but carried more information than expected.

[Campbell Ritchie]

Tim Holloway wrote:. . . part . . . carried more information than expected.

Yes, that is what Paul meant about apples and oranges.

[Tim Holloway]

Campbell Ritchie wrote:

Tim Holloway wrote:. . . part . . . carried more information than expected.

Yes, that is what Paul meant about apples and oranges.

Actually, that waas me. Paul's comment was on inconsistent pluralisation.

[Campbell Ritchie]

Tim Holloway wrote:. . . Actually, that waas me. . . .

Sorry for my mistake. No wonder MS told me off earlier.

[Mike Simmons]