While reviewing the tree-building exercise, I noticed that the current test suite implicitly enforces where validation must occur — specifically inside the BuildTree() function rather than within the Record class itself.
For example, this test
fails if validation is performed during record instantiation, even though that approach can make sense from an object-oriented design perspective.
The same pattern appears in other tests (e.g. test_cycle_directly(), test_cycle_indirectly(), test_higher_id_parent_of_lower_id()).
To make the tests more flexible — and to support both procedural and OOP-focused implementations — they could be extended as follows:
def test_root_node_has_parent(self):
try:
records = [
Record(0, 1),
Record(1, 0)
]
except ValueError as err:
self.assertEqual(
str(err),
"Node parent_id should be smaller than it's record_id."
)
return
with self.assertRaises(ValueError) as err:
BuildTree(records)
self.assertEqual(type(err.exception), ValueError)
self.assertEqual(err.exception.args[0], "Node parent_id should be smaller than it's record_id.")
This keeps all current valid solutions passing while also allowing for solutions where validation logic is encapsulated within the Record class, aligning better with OOP principles and enabling richer design discussions during mentoring.
I’m not too familiar with Python…isn’t this less verbose and does the same:
def test_root_node_has_parent(self):
with self.assertRaises(ValueError) as err:
records = [
Record(0, 1),
Record(1, 0)
]
BuildTree(records)
self.assertEqual(type(err.exception), ValueError)
self.assertEqual(err.exception.args[0], "Node parent_id should be smaller than it's record_id.")
You’re absolutely right—your suggested version is cleaner and more concise while achieving the same result. This would make the test more readable and maintainable without sacrificing clarity.