From 72bce841f41f91cfe56f6c0e69284730fc8d85cd Mon Sep 17 00:00:00 2001 From: Daniel Viegas Date: Sun, 30 Nov 2025 01:17:10 +0100 Subject: [PATCH] Refactor: Extract constants and optimize code quality - Use _normalize_event_type() helper consistently in draw_event_marker() - Extract label drawing constants (text color, vertical offset, hover colors) - Extract marker border width and black color constants - Extract year marker label offset constant - Create _get_events_for_person() helper to reduce duplication - Replace len() < 1 with 'not' for better Pythonic code - Optimize y_positions min/max using sorted list indices - Extract connection line minimum distance constant These changes improve code maintainability, reduce duplication, and enhance readability by replacing magic numbers with named constants. --- MyTimeline.py | 74 ++++++++++++++++++++++++++++++++++----------------- 1 file changed, 49 insertions(+), 25 deletions(-) diff --git a/MyTimeline.py b/MyTimeline.py index ced5d1b..37ab40b 100644 --- a/MyTimeline.py +++ b/MyTimeline.py @@ -150,6 +150,7 @@ CONNECTION_LINE_WIDTH = 3.5 CONNECTION_VERTICAL_LINE_X = 5 # Left of year markers CONNECTION_LINE_SPACING = 25 # Pixels between vertical lines for different persons TIMELINE_LEFT_SPACING = 20 # Spacing between rightmost connection line and timeline +CONNECTION_LINE_MIN_DISTANCE = EVENT_MARKER_SIZE * 2 # Minimum distance between events to draw vertical connector line # Color Generation Constants PERSON_COLOR_SATURATION = 0.7 # High saturation for vibrant colors @@ -162,6 +163,19 @@ SELECTED_MARKER_COLOR = (0.2, 0.4, 0.9) # Blue highlight for selected person's # Default Colors DEFAULT_EVENT_COLOR = (0.5, 0.5, 0.5) # Gray for unknown event types YEAR_MARKER_COLOR = (0.5, 0.5, 0.5) # Gray for year marker tick marks +COLOR_BLACK = (0.0, 0.0, 0.0) # Black color used in multiple drawing operations + +# Label Drawing Constants +LABEL_TEXT_COLOR = (0.1, 0.1, 0.1) # Dark gray text color for event labels +LABEL_VERTICAL_OFFSET = 10 # Vertical offset to center label on marker +LABEL_HOVER_BACKGROUND_COLOR = (1.0, 1.0, 1.0, 0.8) # Semi-transparent white background for hovered labels +LABEL_HOVER_BORDER_COLOR = (0.7, 0.7, 0.7, 0.5) # Light gray border for hovered labels + +# Marker Drawing Constants +MARKER_BORDER_WIDTH = 1 # Line width for marker borders + +# Year Marker Constants +YEAR_MARKER_LABEL_OFFSET = 20 # Horizontal offset for year marker labels from timeline # Logger logger = logging.getLogger(__name__) @@ -2104,6 +2118,22 @@ class MyTimelineView(NavigationView): return True return person_handle in self.person_filter + def _get_events_for_person(self, person_handle: str, events_list: List['TimelineEvent']) -> List['TimelineEvent']: + """ + Filter events list to only include events for a specific person. + + Args: + person_handle: The handle of the person to filter events for. + events_list: List of timeline events to filter. + + Returns: + List[TimelineEvent]: Filtered list of events for the specified person. + """ + return [ + event for event in events_list + if self._person_matches_handle(event.person, person_handle) + ] + def _get_event_type_display_name(self, event_type: EventType) -> str: """ Get human-readable display name for an event type. @@ -2828,10 +2858,7 @@ class MyTimelineView(NavigationView): person_handle = event_data.person.get_handle() # Find all events for this person - person_events = [ - evt for evt in self.events - if self._person_matches_handle(evt.person, person_handle) - ] + person_events = self._get_events_for_person(person_handle, self.events) tooltip_text = self._format_person_tooltip(event_data.person, person_events) else: @@ -2912,7 +2939,7 @@ class MyTimelineView(NavigationView): y_end: Y coordinate of the timeline end. """ # Draw shadow - context.set_source_rgba(0.0, 0.0, 0.0, TIMELINE_SHADOW_OPACITY) + context.set_source_rgba(*COLOR_BLACK, TIMELINE_SHADOW_OPACITY) context.set_line_width(TIMELINE_LINE_WIDTH) context.move_to(timeline_x + TIMELINE_SHADOW_OFFSET, y_start + TIMELINE_SHADOW_OFFSET) context.line_to(timeline_x + TIMELINE_SHADOW_OFFSET, y_end + TIMELINE_SHADOW_OFFSET) @@ -3025,7 +3052,7 @@ class MyTimelineView(NavigationView): size: Size of the marker. shape: Shape type ('triangle', 'circle', etc.). """ - context.set_source_rgba(0.0, 0.0, 0.0, SHADOW_OPACITY) + context.set_source_rgba(*COLOR_BLACK, SHADOW_OPACITY) context.translate(SHADOW_OFFSET_X, SHADOW_OFFSET_Y) self._draw_shape(context, x, y, size, shape) context.fill() @@ -3076,7 +3103,7 @@ class MyTimelineView(NavigationView): context.save() # Get integer value from EventType object for dictionary lookup - event_type_value = event_type.value if hasattr(event_type, 'value') else int(event_type) + event_type_value = self._normalize_event_type(event_type) # Get color and shape color = EVENT_COLORS.get(event_type_value, DEFAULT_EVENT_COLOR) @@ -3100,8 +3127,8 @@ class MyTimelineView(NavigationView): self._draw_marker_gradient(context, x, y, marker_size, color, shape) # Draw border - context.set_source_rgba(0.0, 0.0, 0.0, BORDER_OPACITY) - context.set_line_width(1) + context.set_source_rgba(*COLOR_BLACK, BORDER_OPACITY) + context.set_line_width(MARKER_BORDER_WIDTH) self._draw_shape(context, x, y, marker_size, shape) context.stroke() @@ -3235,17 +3262,17 @@ class MyTimelineView(NavigationView): context.close_path() # Fill with semi-transparent background - context.set_source_rgba(1.0, 1.0, 1.0, 0.8) + context.set_source_rgba(*LABEL_HOVER_BACKGROUND_COLOR) context.fill() # Draw border - context.set_source_rgba(0.7, 0.7, 0.7, 0.5) - context.set_line_width(1) + context.set_source_rgba(*LABEL_HOVER_BORDER_COLOR) + context.set_line_width(MARKER_BORDER_WIDTH) context.stroke() # Draw text - context.set_source_rgb(0.1, 0.1, 0.1) # Dark gray text - context.move_to(x, y - 10) # Center vertically on marker + context.set_source_rgb(*LABEL_TEXT_COLOR) + context.move_to(x, y - LABEL_VERTICAL_OFFSET) PangoCairo.show_layout(context, layout) context.restore() @@ -3326,8 +3353,8 @@ class MyTimelineView(NavigationView): # Get text size in pixels text_width, text_height = layout.get_pixel_size() - context.set_source_rgb(0.0, 0.0, 0.0) # Black - context.move_to(timeline_x - 20 - text_width, y_pos - text_height / 2) + context.set_source_rgb(*COLOR_BLACK) + context.move_to(timeline_x - YEAR_MARKER_LABEL_OFFSET - text_width, y_pos - text_height / 2) PangoCairo.show_layout(context, layout) def draw_year_markers(self, context: cairo.Context, timeline_x: float, @@ -3406,12 +3433,9 @@ class MyTimelineView(NavigationView): vertical_line_x = self._get_person_vertical_line_x(person_index) # Find all events for this person - person_events = [ - event_data for event_data in events_with_y_pos - if self._person_matches_handle(event_data.person, person_handle) - ] + person_events = self._get_events_for_person(person_handle, events_with_y_pos) - if len(person_events) < 1: + if not person_events: continue # Sort by Y position @@ -3422,12 +3446,12 @@ class MyTimelineView(NavigationView): # Draw vertical connector line on the left side if len(person_events) > 1: - y_positions = [event_data.y_pos for event_data in person_events] - min_y = min(y_positions) - max_y = max(y_positions) + # Use first and last elements since list is sorted by y_pos + min_y = person_events[0].y_pos + max_y = person_events[-1].y_pos # Draw vertical line connecting all events - if max_y - min_y > EVENT_MARKER_SIZE * 2: + if max_y - min_y > CONNECTION_LINE_MIN_DISTANCE: context.move_to(vertical_line_x, min_y) context.line_to(vertical_line_x, max_y) context.stroke()