Stuck at Clock exercise

Hello,

I’m having trouble passing all test cases for the Clock exercise: Clock in Go on Exercism

49 tests passed / 19 tests failed

Test 42
TestAddMinutesStringless/ add across midnight

clock_test.go:52: New(23, 59).Add(2) = 00:01, want 00:01

My (rookie) code:

package clock

import "fmt"

type Clock struct {
    minutes int
}

func New(h, m int) Clock {
    sum := (h * 60) + m
    
	return Clock{sum}
}

func (c Clock) Add(m int) Clock {
    c.minutes += m
    
	return c
}

func (c Clock) Subtract(m int) Clock {
    c.minutes -= m
    
	return c
}

func (c Clock) String() string {
    hours := c.minutes / 60
    minutes := (c.minutes - (hours * 60))

    if minutes < 0 {
        minutes = 60 - (minutes * -1)
        hours--
    }

    hours = hours % 24
    if hours < 0 {
        hours = 24 - (hours * -1)
    }

	return fmt.Sprintf("%02d:%02d", hours, minutes)
}

Thanks.

This is because the tests don’t check == via the String() method, but it’s looking at the internal object (checking for deep equality).

For example, in the case of New(23, 59).Add(2) (the first failing test case), it’s checking that New(23, 59).Add(2) == New(0, 1).

This isn’t the case, because New(23, 59).Add(2).minutes == 1441, while New(0, 1).minutes == 1.

So you probably need to rethink your approach a bit - instead of doing all the reducing in the String() method, maybe use New() in Add and Subtract and do the reducing work inside your New() function :slightly_smiling_face:

1 Like

Welcome both to the forum!

This is correct. Despite the test results making it seem that the “actual vs got” are equal, it’s looking at the internal state of the object and those are different. Feel free to inspect the code of TestAddMinutesStringless to see this happening.

Your tests are currently failing with:

--- FAIL: TestAddMinutesStringless (0.00s)
    --- FAIL: TestAddMinutesStringless/add_across_midnight (0.00s)
        clock_test.go:54: New(23, 59).Add(2) = 00:01, want 00:01
    --- FAIL: TestAddMinutesStringless/add_more_than_one_day_(1500_min_=_25_hrs) (0.00s)
        clock_test.go:54: New(5, 32).Add(1500) = 06:32, want 06:32
    --- FAIL: TestAddMinutesStringless/add_more_than_two_days (0.00s)
        clock_test.go:54: New(1, 1).Add(3500) = 11:21, want 11:21

But opening a debugger, or inserting some prints in the tests can actually show what’s going on. Specifically, inserting this into the test:

fmt.Printf("actual: %#v (String(): %s), expected: %#v (String(): %s)\n", actual, actual.String(), expected, expected.String())

Will print:

actual: clock.Clock{minutes:1441} (String(): 00:01), expected: clock.Clock{minutes:1} (String(): 00:01)
actual: clock.Clock{minutes:1832} (String(): 06:32), expected: clock.Clock{minutes:392} (String(): 06:32)
actual: clock.Clock{minutes:3561} (String(): 11:21), expected: clock.Clock{minutes:681} (String(): 11:21)

This means that while your String function is able to correctly take two different internal states and see them as equivalent strings, this test tests the internal state of the clock and not for the string representation (hence the “stringless” in the test name).

Maybe the test output can be improved a bit to showcase better what is going on. Feel free to open a PR in exercism/go suggesting changes :) The PR will be auto-closed but I can re-open it for you, just link this discussion in the PR.


@Nikola24 While it’s absolutely fine to ask for help here in the forum, consider also requesting mentoring, for situations like these. Using the online editor you can’t request mentoring if your tests are currently failing. However, if you submit solutions with the Exercism CLI you can request mentoring even with failing tests. I just mention this because while I’m a mentor on the Go track, I’m not sure all Go mentors see the forum regularly.

1 Like

@Steffan153 @andrerfcsantos Thank you both!

I was suspecting it was the type comparison issue, but test was showing equal string output and got me confused.

I opened an issue: Improve test output for the Clock exercise · Issue #2621 · exercism/go · GitHub
Still don’t feel comfortable enough to submit a PR :)

I may be old fashioned, I do like to leave topics like these on the forum for new struggling learners. But I might try the mentoring feature in the future :+1:

No worries.

Feel free to keep posting here on the forum more issues you find in the track, we highly appreciate it!

I created and merged a PR fixing this clock: improve test output for "stringless" tests by andrerfcsantos · Pull Request #2622 · exercism/go · GitHub

If you update the exercise you should now see the new output. For reference, this is what the solution you posted now outputs.

New test output
$ go test
--- FAIL: TestAddMinutesStringless (0.00s)
    --- FAIL: TestAddMinutesStringless/add_across_midnight (0.00s)
        clock_test.go:52: New(23, 59).Add(2)
                 Got: "00:01" (clock.Clock{minutes:1441})
                Want: "00:01" (clock.Clock{minutes:1})
    --- FAIL: TestAddMinutesStringless/add_more_than_one_day_(1500_min_=_25_hrs) (0.00s)
        clock_test.go:52: New(5, 32).Add(1500)
                 Got: "06:32" (clock.Clock{minutes:1832})
                Want: "06:32" (clock.Clock{minutes:392})
    --- FAIL: TestAddMinutesStringless/add_more_than_two_days (0.00s)
        clock_test.go:52: New(1, 1).Add(3500)
                 Got: "11:21" (clock.Clock{minutes:3561})
                Want: "11:21" (clock.Clock{minutes:681})
--- FAIL: TestSubtractMinutesStringless (0.00s)
    --- FAIL: TestSubtractMinutesStringless/subtract_across_midnight (0.00s)
        clock_test.go:71: New(0, 3).Subtract(4)
                 Got: "23:59" (clock.Clock{minutes:-1})
                Want: "23:59" (clock.Clock{minutes:1439})
    --- FAIL: TestSubtractMinutesStringless/subtract_more_than_two_hours (0.00s)
        clock_test.go:71: New(0, 0).Subtract(160)
                 Got: "21:20" (clock.Clock{minutes:-160})
                Want: "21:20" (clock.Clock{minutes:1280})
    --- FAIL: TestSubtractMinutesStringless/subtract_more_than_one_day_(1500_min_=_25_hrs) (0.00s)
        clock_test.go:71: New(5, 32).Subtract(1500)
                 Got: "04:32" (clock.Clock{minutes:-1168})
                Want: "04:32" (clock.Clock{minutes:272})
    --- FAIL: TestSubtractMinutesStringless/subtract_more_than_two_days (0.00s)
        clock_test.go:71: New(2, 20).Subtract(3000)
                 Got: "00:20" (clock.Clock{minutes:-2860})
                Want: "00:20" (clock.Clock{minutes:20})
--- FAIL: TestCompareClocks (0.00s)
    --- FAIL: TestCompareClocks/clocks_with_hour_overflow (0.00s)
        clock_test.go:85: Clock1 == Clock2 is false, want true
                Clock1: "10:37" (clock.Clock{minutes:637})
                Clock2: "10:37" (clock.Clock{minutes:2077})
    --- FAIL: TestCompareClocks/clocks_with_hour_overflow_by_several_days (0.00s)
        clock_test.go:85: Clock1 == Clock2 is false, want true
                Clock1: "03:11" (clock.Clock{minutes:191})
                Clock2: "03:11" (clock.Clock{minutes:5951})
    --- FAIL: TestCompareClocks/clocks_with_negative_hour (0.00s)
        clock_test.go:85: Clock1 == Clock2 is false, want true
                Clock1: "22:40" (clock.Clock{minutes:1360})
                Clock2: "22:40" (clock.Clock{minutes:-80})
    --- FAIL: TestCompareClocks/clocks_with_negative_hour_that_wraps (0.00s)
        clock_test.go:85: Clock1 == Clock2 is false, want true
                Clock1: "17:03" (clock.Clock{minutes:1023})
                Clock2: "17:03" (clock.Clock{minutes:-1857})
    --- FAIL: TestCompareClocks/clocks_with_negative_hour_that_wraps_multiple_times (0.00s)
        clock_test.go:85: Clock1 == Clock2 is false, want true
                Clock1: "13:49" (clock.Clock{minutes:829})
                Clock2: "13:49" (clock.Clock{minutes:-4931})
    --- FAIL: TestCompareClocks/clocks_with_minute_overflow (0.00s)
        clock_test.go:85: Clock1 == Clock2 is false, want true
                Clock1: "00:01" (clock.Clock{minutes:1})
                Clock2: "00:01" (clock.Clock{minutes:1441})
    --- FAIL: TestCompareClocks/clocks_with_minute_overflow_by_several_days (0.00s)
        clock_test.go:85: Clock1 == Clock2 is false, want true
                Clock1: "02:02" (clock.Clock{minutes:122})
                Clock2: "02:02" (clock.Clock{minutes:4442})
    --- FAIL: TestCompareClocks/clocks_with_negative_minute_that_wraps (0.00s)
        clock_test.go:85: Clock1 == Clock2 is false, want true
                Clock1: "04:10" (clock.Clock{minutes:250})
                Clock2: "04:10" (clock.Clock{minutes:-1190})
    --- FAIL: TestCompareClocks/clocks_with_negative_minute_that_wraps_multiple_times (0.00s)
        clock_test.go:85: Clock1 == Clock2 is false, want true
                Clock1: "06:15" (clock.Clock{minutes:375})
                Clock2: "06:15" (clock.Clock{minutes:-3945})
    --- FAIL: TestCompareClocks/clocks_with_negative_hours_and_minutes (0.00s)
        clock_test.go:85: Clock1 == Clock2 is false, want true
                Clock1: "07:32" (clock.Clock{minutes:452})
                Clock2: "07:32" (clock.Clock{minutes:-988})
    --- FAIL: TestCompareClocks/clocks_with_negative_hours_and_minutes_that_wrap (0.00s)
        clock_test.go:85: Clock1 == Clock2 is false, want true
                Clock1: "18:07" (clock.Clock{minutes:1087})
                Clock2: "18:07" (clock.Clock{minutes:-14753})
    --- FAIL: TestCompareClocks/full_clock_and_zeroed_clock (0.00s)
        clock_test.go:85: Clock1 == Clock2 is false, want true
                Clock1: "00:00" (clock.Clock{minutes:1440})
                Clock2: "00:00" (clock.Clock{minutes:0})
FAIL
exit status 1
FAIL    clock   0.439s

It’s a good point that posting things here on the forum gives visibility of the issue to other people.