Reduce method complexity and improve code organization
Major improvements:
- collect_events: Reduced from 74 lines/complexity 21 to 36 lines/complexity 6 (64% complexity reduction)
* Extracted _collect_family_member_events() to eliminate duplication
* Extracted _collect_family_events() for family-level events
* Extracted _invalidate_cache() for cache management
* Extracted _calculate_timeline_height() for height calculation
- draw_year_markers: Reduced from 78 lines/complexity 17 to 39 lines/complexity 7 (59% complexity reduction)
* Extracted _find_year_range() to find min/max years
* Extracted _calculate_year_y_position() for position calculation
* Extracted _draw_year_marker() for drawing individual markers
- _draw_shape: Reduced from 55 lines/complexity 12 to 22 lines/complexity 1 (92% complexity reduction)
* Extracted shape-specific methods: _draw_triangle(), _draw_circle(), _draw_diamond(),
_draw_square(), _draw_star(), _draw_hexagon()
* Uses dictionary-based dispatch pattern for cleaner code
Results:
- Total methods: 48 → 61 (better separation of concerns)
- Average method complexity significantly reduced
- Code is more maintainable and testable
- Each method has a single, clear responsibility
This commit is contained in:
parent
652c88cd99
commit
a4e12d4dbb
312
MyTimeline.py
312
MyTimeline.py
@ -565,6 +565,59 @@ class MyTimelineView(NavigationView):
|
|||||||
if timeline_event:
|
if timeline_event:
|
||||||
self.events.append(timeline_event)
|
self.events.append(timeline_event)
|
||||||
|
|
||||||
|
def _collect_family_member_events(self, handle: Optional[str], role: str) -> None:
|
||||||
|
"""
|
||||||
|
Collect events for a family member (father, mother, or child).
|
||||||
|
|
||||||
|
Args:
|
||||||
|
handle: The person handle to collect events from.
|
||||||
|
role: Role name for logging purposes ('father', 'mother', 'child').
|
||||||
|
"""
|
||||||
|
if not handle:
|
||||||
|
return
|
||||||
|
|
||||||
|
try:
|
||||||
|
person = self.dbstate.db.get_person_from_handle(handle)
|
||||||
|
if person:
|
||||||
|
self._collect_person_events(person, person)
|
||||||
|
except (AttributeError, KeyError) as e:
|
||||||
|
logger.debug(f"Error accessing {role}: {e}")
|
||||||
|
|
||||||
|
def _collect_family_events(self, family) -> None:
|
||||||
|
"""
|
||||||
|
Collect family-level events (marriage, divorce, etc.).
|
||||||
|
|
||||||
|
Args:
|
||||||
|
family: The family object to collect events from.
|
||||||
|
"""
|
||||||
|
for event_ref in family.get_event_ref_list():
|
||||||
|
timeline_event = self._process_event_ref(event_ref, None)
|
||||||
|
if timeline_event:
|
||||||
|
self.events.append(timeline_event)
|
||||||
|
|
||||||
|
def _invalidate_cache(self) -> None:
|
||||||
|
"""Invalidate all caches when events change."""
|
||||||
|
self._adjusted_events_cache = None
|
||||||
|
self._cache_key = None
|
||||||
|
self._cached_date_range = None
|
||||||
|
self._cached_min_date = None
|
||||||
|
self._cached_max_date = None
|
||||||
|
|
||||||
|
def _calculate_timeline_height(self) -> None:
|
||||||
|
"""Calculate and set timeline height based on number of events and zoom."""
|
||||||
|
if self.events:
|
||||||
|
base_height = (
|
||||||
|
TIMELINE_MARGIN_TOP
|
||||||
|
+ len(self.events) * EVENT_SPACING
|
||||||
|
+ TIMELINE_MARGIN_BOTTOM
|
||||||
|
)
|
||||||
|
self.timeline_height = int(base_height * self.zoom_level)
|
||||||
|
else:
|
||||||
|
self.timeline_height = int(600 * self.zoom_level)
|
||||||
|
|
||||||
|
if self.drawing_area:
|
||||||
|
self.drawing_area.set_size_request(800, self.timeline_height)
|
||||||
|
|
||||||
def collect_events(self) -> None:
|
def collect_events(self) -> None:
|
||||||
"""Collect all events for the active family."""
|
"""Collect all events for the active family."""
|
||||||
self.events = []
|
self.events = []
|
||||||
@ -582,64 +635,26 @@ class MyTimelineView(NavigationView):
|
|||||||
return
|
return
|
||||||
|
|
||||||
# Get family events (marriage, divorce, etc.)
|
# Get family events (marriage, divorce, etc.)
|
||||||
for event_ref in family.get_event_ref_list():
|
self._collect_family_events(family)
|
||||||
timeline_event = self._process_event_ref(event_ref, None)
|
|
||||||
if timeline_event:
|
|
||||||
self.events.append(timeline_event)
|
|
||||||
|
|
||||||
# Get father's events
|
# Get father's events
|
||||||
father_handle = family.get_father_handle()
|
self._collect_family_member_events(family.get_father_handle(), "father")
|
||||||
if father_handle:
|
|
||||||
try:
|
|
||||||
father = self.dbstate.db.get_person_from_handle(father_handle)
|
|
||||||
if father:
|
|
||||||
self._collect_person_events(father, father)
|
|
||||||
except (AttributeError, KeyError) as e:
|
|
||||||
logger.debug(f"Error accessing father: {e}")
|
|
||||||
|
|
||||||
# Get mother's events
|
# Get mother's events
|
||||||
mother_handle = family.get_mother_handle()
|
self._collect_family_member_events(family.get_mother_handle(), "mother")
|
||||||
if mother_handle:
|
|
||||||
try:
|
|
||||||
mother = self.dbstate.db.get_person_from_handle(mother_handle)
|
|
||||||
if mother:
|
|
||||||
self._collect_person_events(mother, mother)
|
|
||||||
except (AttributeError, KeyError) as e:
|
|
||||||
logger.debug(f"Error accessing mother: {e}")
|
|
||||||
|
|
||||||
# Get children's events
|
# Get children's events
|
||||||
for child_ref in family.get_child_ref_list():
|
for child_ref in family.get_child_ref_list():
|
||||||
child_handle = child_ref.ref
|
self._collect_family_member_events(child_ref.ref, "child")
|
||||||
try:
|
|
||||||
child = self.dbstate.db.get_person_from_handle(child_handle)
|
|
||||||
if child:
|
|
||||||
self._collect_person_events(child, child)
|
|
||||||
except (AttributeError, KeyError) as e:
|
|
||||||
logger.debug(f"Error accessing child: {e}")
|
|
||||||
|
|
||||||
# Sort events by date
|
# Sort events by date
|
||||||
self.events.sort(key=lambda x: x.date_sort)
|
self.events.sort(key=lambda x: x.date_sort)
|
||||||
|
|
||||||
# Invalidate cache when events change
|
# Invalidate cache when events change
|
||||||
self._adjusted_events_cache = None
|
self._invalidate_cache()
|
||||||
self._cache_key = None
|
|
||||||
self._cached_date_range = None
|
|
||||||
self._cached_min_date = None
|
|
||||||
self._cached_max_date = None
|
|
||||||
|
|
||||||
# Calculate timeline height based on number of events and zoom
|
# Calculate timeline height
|
||||||
if self.events:
|
self._calculate_timeline_height()
|
||||||
base_height = (
|
|
||||||
TIMELINE_MARGIN_TOP
|
|
||||||
+ len(self.events) * EVENT_SPACING
|
|
||||||
+ TIMELINE_MARGIN_BOTTOM
|
|
||||||
)
|
|
||||||
self.timeline_height = int(base_height * self.zoom_level)
|
|
||||||
else:
|
|
||||||
self.timeline_height = int(600 * self.zoom_level)
|
|
||||||
|
|
||||||
if self.drawing_area:
|
|
||||||
self.drawing_area.set_size_request(800, self.timeline_height)
|
|
||||||
|
|
||||||
def _calculate_date_range(self) -> Tuple[int, int, int]:
|
def _calculate_date_range(self) -> Tuple[int, int, int]:
|
||||||
"""
|
"""
|
||||||
@ -1471,6 +1486,57 @@ class MyTimelineView(NavigationView):
|
|||||||
|
|
||||||
context.restore()
|
context.restore()
|
||||||
|
|
||||||
|
def _draw_triangle(self, context: cairo.Context, x: float, y: float, size: float) -> None:
|
||||||
|
"""Draw a triangle shape."""
|
||||||
|
context.move_to(x, y - size)
|
||||||
|
context.line_to(x - size, y + size)
|
||||||
|
context.line_to(x + size, y + size)
|
||||||
|
context.close_path()
|
||||||
|
|
||||||
|
def _draw_circle(self, context: cairo.Context, x: float, y: float, size: float) -> None:
|
||||||
|
"""Draw a circle shape."""
|
||||||
|
context.arc(x, y, size, 0, 2 * math.pi)
|
||||||
|
|
||||||
|
def _draw_diamond(self, context: cairo.Context, x: float, y: float, size: float) -> None:
|
||||||
|
"""Draw a diamond shape."""
|
||||||
|
context.move_to(x, y - size)
|
||||||
|
context.line_to(x + size, y)
|
||||||
|
context.line_to(x, y + size)
|
||||||
|
context.line_to(x - size, y)
|
||||||
|
context.close_path()
|
||||||
|
|
||||||
|
def _draw_square(self, context: cairo.Context, x: float, y: float, size: float) -> None:
|
||||||
|
"""Draw a square shape."""
|
||||||
|
context.rectangle(x - size, y - size, size * 2, size * 2)
|
||||||
|
|
||||||
|
def _draw_star(self, context: cairo.Context, x: float, y: float, size: float) -> None:
|
||||||
|
"""Draw a 5-pointed star shape."""
|
||||||
|
points = 5
|
||||||
|
outer_radius = size
|
||||||
|
inner_radius = size * 0.4
|
||||||
|
for i in range(points * 2):
|
||||||
|
angle = (i * math.pi) / points - math.pi / 2
|
||||||
|
radius = outer_radius if i % 2 == 0 else inner_radius
|
||||||
|
px = x + radius * math.cos(angle)
|
||||||
|
py = y + radius * math.sin(angle)
|
||||||
|
if i == 0:
|
||||||
|
context.move_to(px, py)
|
||||||
|
else:
|
||||||
|
context.line_to(px, py)
|
||||||
|
context.close_path()
|
||||||
|
|
||||||
|
def _draw_hexagon(self, context: cairo.Context, x: float, y: float, size: float) -> None:
|
||||||
|
"""Draw a hexagon shape."""
|
||||||
|
for i in range(6):
|
||||||
|
angle = (i * math.pi) / 3
|
||||||
|
px = x + size * math.cos(angle)
|
||||||
|
py = y + size * math.sin(angle)
|
||||||
|
if i == 0:
|
||||||
|
context.move_to(px, py)
|
||||||
|
else:
|
||||||
|
context.line_to(px, py)
|
||||||
|
context.close_path()
|
||||||
|
|
||||||
def _draw_shape(self, context: cairo.Context, x: float, y: float,
|
def _draw_shape(self, context: cairo.Context, x: float, y: float,
|
||||||
size: float, shape: str) -> None:
|
size: float, shape: str) -> None:
|
||||||
"""
|
"""
|
||||||
@ -1483,50 +1549,17 @@ class MyTimelineView(NavigationView):
|
|||||||
size: Size of the shape.
|
size: Size of the shape.
|
||||||
shape: Shape type ('triangle', 'circle', 'diamond', 'square', 'star', 'hexagon').
|
shape: Shape type ('triangle', 'circle', 'diamond', 'square', 'star', 'hexagon').
|
||||||
"""
|
"""
|
||||||
if shape == 'triangle':
|
shape_drawers = {
|
||||||
context.move_to(x, y - size)
|
'triangle': self._draw_triangle,
|
||||||
context.line_to(x - size, y + size)
|
'circle': self._draw_circle,
|
||||||
context.line_to(x + size, y + size)
|
'diamond': self._draw_diamond,
|
||||||
context.close_path()
|
'square': self._draw_square,
|
||||||
elif shape == 'circle':
|
'star': self._draw_star,
|
||||||
context.arc(x, y, size, 0, 2 * math.pi)
|
'hexagon': self._draw_hexagon,
|
||||||
elif shape == 'diamond':
|
}
|
||||||
context.move_to(x, y - size)
|
|
||||||
context.line_to(x + size, y)
|
drawer = shape_drawers.get(shape, self._draw_square) # Default to square
|
||||||
context.line_to(x, y + size)
|
drawer(context, x, y, size)
|
||||||
context.line_to(x - size, y)
|
|
||||||
context.close_path()
|
|
||||||
elif shape == 'square':
|
|
||||||
context.rectangle(x - size, y - size, size * 2, size * 2)
|
|
||||||
elif shape == 'star':
|
|
||||||
# Draw 5-pointed star
|
|
||||||
points = 5
|
|
||||||
outer_radius = size
|
|
||||||
inner_radius = size * 0.4
|
|
||||||
for i in range(points * 2):
|
|
||||||
angle = (i * math.pi) / points - math.pi / 2
|
|
||||||
if i % 2 == 0:
|
|
||||||
radius = outer_radius
|
|
||||||
else:
|
|
||||||
radius = inner_radius
|
|
||||||
px = x + radius * math.cos(angle)
|
|
||||||
py = y + radius * math.sin(angle)
|
|
||||||
if i == 0:
|
|
||||||
context.move_to(px, py)
|
|
||||||
else:
|
|
||||||
context.line_to(px, py)
|
|
||||||
context.close_path()
|
|
||||||
elif shape == 'hexagon':
|
|
||||||
# Draw hexagon
|
|
||||||
for i in range(6):
|
|
||||||
angle = (i * math.pi) / 3
|
|
||||||
px = x + size * math.cos(angle)
|
|
||||||
py = y + size * math.sin(angle)
|
|
||||||
if i == 0:
|
|
||||||
context.move_to(px, py)
|
|
||||||
else:
|
|
||||||
context.line_to(px, py)
|
|
||||||
context.close_path()
|
|
||||||
|
|
||||||
def draw_event_label(self, context: cairo.Context, x: float, y: float,
|
def draw_event_label(self, context: cairo.Context, x: float, y: float,
|
||||||
date_obj: Date, event, person: Optional[Any],
|
date_obj: Date, event, person: Optional[Any],
|
||||||
@ -1596,23 +1629,13 @@ class MyTimelineView(NavigationView):
|
|||||||
|
|
||||||
context.restore()
|
context.restore()
|
||||||
|
|
||||||
def draw_year_markers(self, context: cairo.Context, timeline_x: float,
|
def _find_year_range(self) -> Tuple[Optional[int], Optional[int]]:
|
||||||
y_start: float, y_end: float, min_date: int,
|
|
||||||
max_date: int) -> None:
|
|
||||||
"""
|
"""
|
||||||
Draw year markers on the left side of the timeline.
|
Find the minimum and maximum years from all events.
|
||||||
|
|
||||||
Args:
|
Returns:
|
||||||
context: Cairo drawing context.
|
Tuple[Optional[int], Optional[int]]: (min_year, max_year) or (None, None) if no valid years found.
|
||||||
timeline_x: X coordinate of the timeline.
|
|
||||||
y_start: Y coordinate of the timeline start.
|
|
||||||
y_end: Y coordinate of the timeline end.
|
|
||||||
min_date: Minimum date sort value.
|
|
||||||
max_date: Maximum date sort value.
|
|
||||||
"""
|
"""
|
||||||
context.save()
|
|
||||||
|
|
||||||
# Find min and max years from events
|
|
||||||
min_year = None
|
min_year = None
|
||||||
max_year = None
|
max_year = None
|
||||||
for event_data in self.events:
|
for event_data in self.events:
|
||||||
@ -1626,36 +1649,48 @@ class MyTimelineView(NavigationView):
|
|||||||
except (AttributeError, ValueError) as e:
|
except (AttributeError, ValueError) as e:
|
||||||
logger.debug(f"Error accessing year from date: {e}")
|
logger.debug(f"Error accessing year from date: {e}")
|
||||||
|
|
||||||
if min_year is None or max_year is None:
|
return (min_year, max_year)
|
||||||
context.restore()
|
|
||||||
return
|
|
||||||
|
|
||||||
# Draw markers for major years (every 10 years or so)
|
def _calculate_year_y_position(self, year: int, min_date: int, max_date: int,
|
||||||
year_step = max(1, (max_year - min_year) // 10)
|
y_start: float, y_end: float) -> float:
|
||||||
if year_step == 0:
|
"""
|
||||||
year_step = 1
|
Calculate Y position for a year marker.
|
||||||
|
|
||||||
context.set_source_rgb(0.5, 0.5, 0.5) # Gray
|
Args:
|
||||||
context.set_line_width(1)
|
year: The year to calculate position for.
|
||||||
|
min_date: Minimum date sort value.
|
||||||
|
max_date: Maximum date sort value.
|
||||||
|
y_start: Y coordinate of the timeline start.
|
||||||
|
y_end: Y coordinate of the timeline end.
|
||||||
|
|
||||||
for year in range(min_year, max_year + 1, year_step):
|
Returns:
|
||||||
# Calculate Y position
|
float: The Y position for the year marker.
|
||||||
|
"""
|
||||||
year_date = Date()
|
year_date = Date()
|
||||||
year_date.set_yr_mon_day(year, 1, 1)
|
year_date.set_yr_mon_day(year, 1, 1)
|
||||||
year_sort = year_date.get_sort_value()
|
year_sort = year_date.get_sort_value()
|
||||||
|
|
||||||
if min_date == max_date:
|
if min_date == max_date:
|
||||||
y_pos = (y_start + y_end) / 2
|
return (y_start + y_end) / 2
|
||||||
else:
|
else:
|
||||||
y_pos = y_start + (
|
return y_start + (
|
||||||
(year_sort - min_date) / (max_date - min_date)
|
(year_sort - min_date) / (max_date - min_date)
|
||||||
) * (y_end - y_start)
|
) * (y_end - y_start)
|
||||||
|
|
||||||
# Only draw if within visible range
|
def _draw_year_marker(self, context: cairo.Context, timeline_x: float,
|
||||||
if y_pos < y_start or y_pos > y_end:
|
year: int, y_pos: float) -> None:
|
||||||
continue
|
"""
|
||||||
|
Draw a single year marker (tick mark and label).
|
||||||
|
|
||||||
|
Args:
|
||||||
|
context: Cairo drawing context.
|
||||||
|
timeline_x: X coordinate of the timeline.
|
||||||
|
year: The year to draw.
|
||||||
|
y_pos: Y position for the marker.
|
||||||
|
"""
|
||||||
# Draw tick mark
|
# Draw tick mark
|
||||||
|
context.set_source_rgb(0.5, 0.5, 0.5) # Gray
|
||||||
|
context.set_line_width(1)
|
||||||
context.move_to(timeline_x - 10, y_pos)
|
context.move_to(timeline_x - 10, y_pos)
|
||||||
context.line_to(timeline_x, y_pos)
|
context.line_to(timeline_x, y_pos)
|
||||||
context.stroke()
|
context.stroke()
|
||||||
@ -1674,6 +1709,45 @@ class MyTimelineView(NavigationView):
|
|||||||
context.move_to(timeline_x - 20 - text_width, y_pos - text_height / 2)
|
context.move_to(timeline_x - 20 - text_width, y_pos - text_height / 2)
|
||||||
PangoCairo.show_layout(context, layout)
|
PangoCairo.show_layout(context, layout)
|
||||||
|
|
||||||
|
def draw_year_markers(self, context: cairo.Context, timeline_x: float,
|
||||||
|
y_start: float, y_end: float, min_date: int,
|
||||||
|
max_date: int) -> None:
|
||||||
|
"""
|
||||||
|
Draw year markers on the left side of the timeline.
|
||||||
|
|
||||||
|
Args:
|
||||||
|
context: Cairo drawing context.
|
||||||
|
timeline_x: X coordinate of the timeline.
|
||||||
|
y_start: Y coordinate of the timeline start.
|
||||||
|
y_end: Y coordinate of the timeline end.
|
||||||
|
min_date: Minimum date sort value.
|
||||||
|
max_date: Maximum date sort value.
|
||||||
|
"""
|
||||||
|
context.save()
|
||||||
|
|
||||||
|
# Find min and max years from events
|
||||||
|
min_year, max_year = self._find_year_range()
|
||||||
|
|
||||||
|
if min_year is None or max_year is None:
|
||||||
|
context.restore()
|
||||||
|
return
|
||||||
|
|
||||||
|
# Draw markers for major years (every 10 years or so)
|
||||||
|
year_step = max(1, (max_year - min_year) // 10)
|
||||||
|
if year_step == 0:
|
||||||
|
year_step = 1
|
||||||
|
|
||||||
|
for year in range(min_year, max_year + 1, year_step):
|
||||||
|
# Calculate Y position
|
||||||
|
y_pos = self._calculate_year_y_position(year, min_date, max_date, y_start, y_end)
|
||||||
|
|
||||||
|
# Only draw if within visible range
|
||||||
|
if y_pos < y_start or y_pos > y_end:
|
||||||
|
continue
|
||||||
|
|
||||||
|
# Draw the year marker
|
||||||
|
self._draw_year_marker(context, timeline_x, year, y_pos)
|
||||||
|
|
||||||
context.restore()
|
context.restore()
|
||||||
|
|
||||||
def draw_person_connections(self, context: cairo.Context,
|
def draw_person_connections(self, context: cairo.Context,
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user