倒计时AppIndicator

Question

我决定将我用C编写的一个旧的Unity翻译成Ruby，这样我就可以用它来练习编码了(我已经厌倦了我一直在学习的完全初学者的课程，我会完成的！)C指示器本身有点粗糙，我是在一个下午写的，所以我认为它也反映在Ruby指示器上。

这项工作仍在进行中。目前，定时器只能通过配置文件(在自述和示例文件中描述)进行配置。设置文件后，用户可以启动指示符，单击图标"Start timer.“和“从配置文件中获取”。这个窗口弹出的原因是我在考虑创建一个更友好的用户界面，用户可以输入相同的参数。

您可以配置4个参数:启用、通知和通知延迟(发出notify-send调用以提醒用户还剩多少时间)、初始定时器和持久计时器(前者只设置计时器将在多少秒处启动，而后者将在关闭和再次打开应用程序后使定时器保持不变)。持久计时器的工作方式是编写一个文件，该文件在计时器应该结束时具有划时代的时间，因此可以恢复。

指示灯的图标颜色在某些范围内变化(超过10分钟为蓝色，超过5分钟为橙色，5分钟以下为红色，而当它达到0时为黑色)。一旦达到0，什么都不会发生(我计划实现某种通知，可能是警报)。

GitHub回购

#!/usr/bin/ruby

require 'ruby-libappindicator'

# DEBUG FUNCTION
DEBUG=true
def debug(message)
    if DEBUG==true
        puts "DEBUG: "+message
    end
end

# SOME CONSTANTS
BLUE_ICON_RANGE = 600       #10 minutes
ORANGE_ICON_RANGE = 300     #5 minutes
# So here is how it goes: Timer > 10 minutes icon is blue. 10 minutes > Timer > 5 minutes icon is orange. Timer < 5 minutes
# icon is red. Timer is 0 icon is black.

# MAIN CLASS
class CountdownI_Class < AppIndicator::AppIndicator
    # Class variables
    @notify_delay       #Delay between the timer notifications
    @enable_notify      #Enable notifications?
    @persistent_timer   #Should the timer be persistent between calls? (Keeping track of the same timer every time you launch)

    @target_timer       #The epoch time where the timer will be over
    @countdown_timer    #The seconds remaining

    @indicator_icons    #Array with the different paths to the icons the indicator will use

    @is_running = false #Did we start the timer?

    # Initialization (Set the indicator and the default variables values)
    def initialize(name, icon, category)
        super

        #Gtk + AppIndicator Setup
        #Getting icons paths
        @indicator_icons = []
        @indicator_icons[0] = File.realpath("./Icons/IconBlack.png")
        @indicator_icons[1] = File.realpath("./Icons/IconBlue.png")
        @indicator_icons[2] = File.realpath("./Icons/IconOrange.png")
        @indicator_icons[3] = File.realpath("./Icons/IconRed.png")

        mainmenu = Gtk::Menu.new

        #Start the timer
        mainmenu_start = Gtk::MenuItem.new("Start timer...")
        mainmenu_start.signal_connect("activate"){
            #We parse the mainmenu_start menu item as an argument because if we start the counter
            #we need to disable this item.
            self.start_timer(mainmenu_start)
        }

        mainmenu.append(mainmenu_start)
        mainmenu_start.show()

        #Quit
        mainmenu_quit = Gtk::MenuItem.new("Quit")
        mainmenu_quit.signal_connect("activate"){
            self.quit_timer()
        }

        mainmenu.append(mainmenu_quit)
        mainmenu_quit.show()

        set_menu(mainmenu)
        set_status(AppIndicator::Status::ACTIVE)

        #Default variables values
        @notify_delay=300
        @enable_notify=false
        @countdown_timer=30
        @persistent_timer=false
    end

    # Read the configuration file if it's present and readable (if it isn't present creates the default one)
    def read_config()
        #Checks if configuration file exists and if it's readable. If it doesn't exist, write one with default
        #values. If it does exist but isn't readable, leave it there and use default values. Else, just use the
        #values from the file.
        if(File.exists?(File.realpath("./Config")+"/CountdownI.config"))
            debug("Config file exists!")
            if(File.readable?(File.realpath("./Config/CountdownI.config")))
                debug("Config file is readable!")
                debug("Reading:")
                config_file = File.open(File.realpath("./Config/CountdownI.config"),"r")

                while(line = config_file.gets)
                    if(!line.start_with?("//") && line.chomp.length>0)
                        param_value_list = line.split("=")
                        debug("Parameter: #{param_value_list[0]} - Value: #{param_value_list[1]}")

                        case param_value_list[0]
                            when "NOTIFY DELAY"
                                @notify_delay=param_value_list[1].to_i
                            when "ENABLE NOTIFY"
                                if(param_value_list[1].chomp == "TRUE")
                                    @enable_notify=true
                                else
                                    @enable_notify=false
                                end
                            when "INITIAL TIMER"
                                @countdown_timer=param_value_list[1].to_i
                            when "PERSISTENT TIMER"
                                if(param_value_list[1].chomp == "TRUE")
                                    @persistent_timer=true
                                else
                                    @persistent_timer=false
                                end
                        end
                    end
                end
                config_file.close
            else
                debug("Config file not readable. Using default values...")
            end
        else
            debug("Config file not present. Writing the default one...")
            config_file = File.open(File.realpath("./Config")+"/CountdownI.config","w")

            # Write default configuration file
            config_file.write("//The configuration file is simple:\n//NOTIFY DELAY=<number> being number in the range 60-1200 seconds\n//ENABLE NOTIFY=TRUE/FALSE anything other than that means false.\n//INITIAL TIMER=<number> the initial countdown timer\n//PERSISTENT TIMER=TRUE/FALSE anything other than that means false.\n\nNOTIFY DELAY=300\nENABLE NOTIFY=FALSE\nINITIAL TIMER=30\nPERSISTENT TIMER=FALSE\n")

            config_file.close
        end

        # Check the values and use default ones if anything is odd:
        if(@notify_delay < 60 || @notify_delay > 1200)
            @notify_delay = 300
        end
        if(@countdown_timer <= 0)
            @countdown_timer = 30
        end
    end

    def set_timer()
        debug("Current epoch time = "+Time.new.strftime("%s"))

        # If the timer is not persistent, just calculate the target timer from the current epoch time
        if(@persistent_timer == false)
            debug("Persistent timer is Off!")

            @target_timer = Time.new.to_i + @countdown_timer

            debug("Countdown timer = "+@countdown_timer.to_s)
            debug("Target timer = "+@target_timer.to_s)
        else
        # If the timer is persistent, we check the target timer from the restore file if it exists or create one if it doesn't
            debug("Persistent timer is On!")
            debug("Restore file path: "+File.realpath("./")+"/countdown.restore")

            # File is there, we just restore the target timer
            if(File.exists?(File.realpath("./")+"/countdown.restore"))
                debug("Restore file is present")
                if(File.readable?(File.realpath("./")+"/countdown.restore"))
                    debug("Restore file is readable")

                    restore_file = File.open(File.realpath("./countdown.restore"),"r")

                    while(line = restore_file.gets)
                        @target_timer = line.to_i
                    end

                    @countdown_timer = @target_timer - Time.new.to_i

                    debug("Target timer from restore file = "+@target_timer.to_s)
                    debug("Countdown timer = "+@countdown_timer.to_s)

                    restore_file.close
                else
                    #This error shouldn't happen if the user didn't play around with chmod/chown...
                    puts("[ERROR]: You don't have permissions to read the restore file...")
                    exit(1)
                end
            else
            # File isn't there, we should create it
                debug("Restore file not present!")

                @target_timer = Time.new.to_i + @countdown_timer

                debug("Countdown timer = "+@countdown_timer.to_s)
                debug("Target timer = "+@target_timer.to_s)

                # Effectively writes the restore file
                self.write_restore_file
            end
        end
    end

    def write_restore_file()
        debug("Writing restore file:")
        if(File.exists?(File.realpath("./")+"/countdown.restore"))
            debug("Restore file exists already")
            if(File.writable?(File.realpath("./")+"/countdown.restore"))
                debug("Restore file is writable")

                restore_file = File.open(File.realpath("./countdown.restore"),"w")

                restore_file.write(@target_timer.to_s)

                restore_file.close
            else
                #This error shouldn't happen if the user didn't play around with chmod/chown...
                puts("[ERROR]: You don't have permissions to write to the restore file...")
                exit(1)
            end
        else
            debug("Restore file doesn't exist. Writing it...")

            restore_file = File.open(File.realpath("./")+"/countdown.restore","w")

            restore_file.write(@target_timer.to_s)

            restore_file.close
        end
    end

    def remove_restore_file()
        debug("Removing restore file:")
        if(File.exists?(File.realpath("./")+"/countdown.restore"))
            debug("Restore file is present")
            if(File.writable?(File.realpath("./")+"/countdown.restore"))
                debug("Restore file is writable, so probably deletable too")
                debug("Deleting restore file...")

                File.delete(File.realpath("./countdown.restore"))
            else
                #This error shouldn't happen if the user didn't play around with chmod/chown...
                puts("[ERROR]: You don't have write permissions to the restore file...")
                exit(1)
            end
        else
            debug("Restore file is not present. Not deleting anything")
        end
    end

    def update_timer()
        @countdown_timer = @target_timer - Time.new.to_i

        if(@countdown_timer < 0)
            @countdown_timer = 0
        end

        if(@countdown_timer > BLUE_ICON_RANGE)
            self.set_icon(@indicator_icons[1])
        elsif(@countdown_timer > ORANGE_ICON_RANGE)
            self.set_icon(@indicator_icons[2])
        elsif(@countdown_timer > 0)
            self.set_icon(@indicator_icons[3])
        else
            self.set_icon(@indicator_icons[0])
        end
    end

    def start_timer(caller_menuitem)
        #We will display a window where the user can set the timer
        #or choose to get the parameters from the config file.

        start_window = Gtk::Window.new()
        start_window.set_border_width(10)

        start_from_config_btn = Gtk::Button.new("Get from config file")
        start_from_config_btn.signal_connect("clicked"){
            #Read the configuration file and set the parameters
            self.read_config()

            debug("VALUES: ")
            debug('Enable notify: '+@enable_notify.to_s)
            debug('Notify delay: '+@notify_delay.to_s)
            debug('Initial timer: '+@countdown_timer.to_s)
            debug('Persistent timer: '+@persistent_timer.to_s)

            debug("Setting timer:")

            #Set the timer
            self.set_timer()

            #Timeout function that will update the indicator
            GLib::Timeout.add(1000){
                self.update_timer()

                self.set_label("Time left: "+@countdown_timer.to_s+" seconds", "CountdownI")

                true
            }

            #Timeout function that will issue the 'notify-send' commands
            if(@enable_notify)
                GLib::Timeout.add(@notify_delay*1000){
                    debug("Notify timeout!")

                    if(@countdown_timer > BLUE_ICON_RANGE)
                        Kernel::system("notify-send --icon='"+@indicator_icons[1]+"' 'CountdownI: "+@countdown_timer.to_s+" seconds left!'")
                    elsif(@countdown_timer > ORANGE_ICON_RANGE)
                        Kernel::system("notify-send --icon='"+@indicator_icons[2]+"' 'CountdownI: "+@countdown_timer.to_s+" seconds left!'")
                    elsif(@countdown_timer > 0)
                        Kernel::system("notify-send --icon='"+@indicator_icons[3]+"' 'CountdownI: "+@countdown_timer.to_s+" seconds left!'")
                    end

                    #I don't think we can use "return" here... Maybe because it's not a function, but a block of code?
                    if(@countdown_timer <= 0)
                        false
                    else
                        true
                    end
                }
            end

            @is_running = true

            start_window.destroy()

            #The menuitem from the indicator is now inactive
            caller_menuitem.set_sensitive(false)
        }

        start_window.add(start_from_config_btn)

        start_window.show_all
    end

    def quit_timer()
        #If we didn't start the timer yet, we don't need to bother with the restore file
        if(@is_running == true)
            #If the time is up, we are in the persistent mode and there is a restore file, remove it...
            if(CountdownI.instance_variable_get("@persistent_timer") && CountdownI.instance_variable_get("@countdown_timer")<=0)
                CountdownI.remove_restore_file()
            end
        end

        Gtk.main_quit()
    end
end

# Program flow

Gtk.init()

#Create the indicator object
CountdownI = CountdownI_Class.new("CountdownI", File.realpath("./Icons/IconBlack.png"), AppIndicator::Category::APPLICATION_STATUS)

Gtk.main()

Answer 1

有很多地方可以改进代码，而且很容易重写大量代码，因为它需要重新编写，以适应Ruby标准和习惯用法。

basic风格

我建议您阅读红宝石风格指南，并修复那些简单的东西--缩进、方法调用(不需要空的“()”)、间距、单引号与双引号、条件词周围的参数等等。

在对象中的思考

很明显，您来自于一种更程序化的语言，但您必须取消这些习惯，学习新的面向对象的代码设计方法。

您只有一个对象，但是这个程序至少应该有几个对象。我建议你浏览一下设计模式实例，也许可以买一本随附的书。

如果您的代码在逻辑上被分解成不同的对象，这些对象具有有意义的关系和明确定义的责任，那么从远处阅读并在将来维护它就会容易得多。

分解对象

..into其他对象和/或小方法。

通常，您应该尽可能少地在初始化方法中进行处理。初始化一个类不应该启动一堆东西来创建这个上帝对象。对此，一个简单的临时解决方案是将所有您可以使用的东西移到一个单独的方法中，并适当地命名它。

类似于：

class MyClass
  def initialize(name, item, category)
    @name, @item, @category = name, item, category
    # very minimal processing - set variables, etc, that's it
  end

  def self.build(name, item, category)
    new(name, item, category).build
  end

  def build
    # throw bulk initialization processing in here
  end
end

# usage
MyClass.build(name, item, category)

当您移动初始化代码时，请考虑将其分解为更小的描述性方法。

例如，您可以将您的@indicator_icons提取到一个方法中。

def indicator_icons
  @indicator_icons ||= begin
    [
      File.realpath("./Icons/IconBlack.png"),
      File.realpath("./Icons/IconBlue.png"),
      ...
    ]
  end
end

不能对菜单对象多加评论，因为它有点难读。我要说的一件事是，您似乎在类和对象上下文中穿插(在初始化中使用self )。小心点。

修复属性

您指定属性的方式是错误的，实际上不应该产生任何影响。如果您想要定义属性以及访问或修改它们的方法，您可以使用attr_reader :list, :of, :attributes或attr_writer :list, :of, :attributes，或者对两个attr_accessor :list, :of, :attributes都使用。有关更多信息，请参见这里。

提取更多！

另一个提取的主要机会是您的配置文件解析。如果您可以使用不需要太多解析(yaml等)的不同格式，那就太好了！如果不是，您应该考虑将解析器提取到它自己的对象中。

然后，您将得到类似的内容(注意，没有空的())：

def config
  @config ||= Config.parse('CountdownI.config')
end

上面的代码还有几个注意事项。我已经删除了read_，因为这是多余的。在Ruby中，我们忘记了read_，get_，is_，set_。简明扼要，描述清楚。方法名清楚地指示它将返回您的配置。

我还使用了回忆录 (@config ||=)。这是为了防止该方法多次解析配置。它将解析一次并将变量设置为解析的返回，然后简单地在后续调用中返回变量值。习惯这一点，并使用它使您的代码更有效率。

提取更多！

另一个提取的机会是你的条件。

你有：if(@enable_notify)

首先，正如本文的基本样式部分所指出的，您不应该使用括号。应该是if @enable_notify。更好的是，您可以将其解压缩到谓词法中：

def notify?
  @enable_notify
end

if notify?
  ...
end

在关闭

我要说的主要问题是：

• 基本语法/ruby样式是非读样式指南。
• 单个对象做的太多了--将责任分散到其他对象中。
• 将对象分解为小的描述性方法

希望这能有所帮助！