Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

my palindrome solution in go #46

Merged
merged 1 commit into from Apr 21, 2021
Merged

Conversation

drkennetz
Copy link
Collaborator

can be run in this directory by running go test. I've only handled the types of int and string in my type conversions (but I had a fun time writing a type converter). I also got my hands a little dirty with writing test tables!

Copy link
Collaborator

@birenderjit birenderjit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this will work for say AA11BB.
or we are not considering mixed letters and numbers.

return false
}
// 256 chars in ascii charset
ords := make([]int, 256)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fun idea to pre-make the map

if ords[i]%2 != 0 {
oddCount++
}
if oddCount > 1 {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice to interrupt as early as possible 💯

Comment on lines +9 to +26
tables := []struct {
x interface{}
y bool
}{
{"carrace", true},
{"", false},
{"annnab", false},
{41231234, true},
}

for _, table := range tables {
result := CanMakePalindrome(table.x)
if result != table.y {
t.Error("FAIL: CanMakePalindrome of: ", result, "was incorrect, got: ", result, "expected: ", table.y)
} else {
fmt.Println("SUCCESS: CanMakePalindrome of: ", result, "was correct, got: ", result, "expected: ", table.y)
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

really cool

@drkennetz
Copy link
Collaborator Author

I don't think this will work for say AA11BB.
or we are not considering mixed letters and numbers.

If they are converted to string then ranging over them should return each character’s ascii value. Numbers have an ascii encoding also in the 256 charset so it should work. I am incrementing the index in the int slice of 0s that corresponds to the ascii encoding. I’ll add it to the test table tomorrow to make sure, but I don’t see why it wouldn’t work.

@parhaml parhaml merged commit fd054d4 into main Apr 21, 2021
@parhaml parhaml deleted the drkennetz-potential-palindrome branch April 21, 2021 12:39
@drkennetz
Copy link
Collaborator Author

@birenderjit just to follow up, I added that test case to the table and it passed.

@birenderjit
Copy link
Collaborator

birenderjit commented Apr 21, 2021 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants