Readability: Complex nested conditions in message analysis

[labtgbot]

Readability Analysis: Complex Nested Conditions in Message Analysis

🔍 Issue Description

After analyzing the src/telegram/handlers.ts file, I identified readability issues in the analyzeMessage method with deeply nested conditional logic that makes the code difficult to maintain and understand.

⚠️ Severity: Medium

🎯 Affected Code

• src/telegram/handlers.ts - Lines 155-250 (analyzeMessage method)
• Complex nested switch statements and conditionals

🚨 Readability Issues Identified

1. Deeply Nested Logic

   if (message.isGroup) {
     switch (this.config.group_policy) {
       case "disabled":
         return { /* complex return */ };
       case "admin-only":
         if (!isAdmin) {
           return { /* complex return */ };
         }
         break;
       case "allowlist": {
         const chatIdNum = Number(message.chatId);
         if (!Number.isInteger(chatIdNum) || !this.config.group_allow_from.includes(chatIdNum)) {
           return { /* complex return */ };
         }
         break;
       }
     }
   }

2. Mixed Responsibilities

   • Single method handles policy validation, permission checking, and rate limiting
   • Hard to test individual components
   • Difficult to add new policies without changing the core logic

3. Magic Numbers and Strings

   • Direct policy string comparisons
   • No constants for policy values
   • Error messages scattered throughout the logic

💡 Recommended Fixes

1. Extract policy validators

   class MessagePolicyValidator {
     validateDM(message: TelegramMessage, isAdmin: boolean): ValidationResult {
       switch (this.config.dm_policy) {
         case "disabled":
           return { shouldRespond: false, reason: "DMs disabled" };
         case "admin-only":
           return { shouldRespond: isAdmin, reason: isAdmin ? undefined : "DMs restricted to admins" };
         case "allowlist":
           return { 
             shouldRespond: isAdmin || this.config.allow_from.includes(message.senderId),
             reason: isAdmin || this.config.allow_from.includes(message.senderId) ? undefined : "Not in allowlist"
           };
         case "open":
           return { shouldRespond: true };
       }
     }
     
     validateGroup(message: TelegramMessage, isAdmin: boolean): ValidationResult {
       // Extracted group validation logic
     }
   }

2. Use builder pattern for context

   class MessageContextBuilder {
     private context: Partial<MessageContext> = {};
     
     withPolicy(policy: PolicyType): this {
       this.context.policy = this.validatePolicy(policy);
       return this;
     }
     
     withPermissions(isAdmin: boolean): this {
       this.context.shouldRespond = this.context.policy?.shouldResponse && isAdmin;
       return this;
     }
     
     build(): MessageContext {
       return this.context as MessageContext;
     }
   }

3. Constants for policy values

   const POLICIES = {
     DM: ["disabled", "admin-only", "allowlist", "open"] as const,
     GROUP: ["disabled", "admin-only", "allowlist", "open"] as const,
   } as const;
   
   type PolicyType = typeof POLICIES.DM[number];

📖 Expected Improvements

• Better testability with separated concerns
• Easier maintenance with clear, focused methods
• Reduced complexity through extracted validators
• Improved developer experience with consistent patterns

🔧 Implementation Priority

Medium - Improves code quality and maintainability

---

Generated by GitHub Dev Assistant during readability analysis
Suggested fix: Extract policy validation and use builder pattern
Review required before implementation