新年倒计时器

Question

请帮我改进我的代码。注意，它运行得很好。特别是我使用SFML的方式，我需要很多指导，因为我刚刚开始学习如何使用这个库。我还在学习C++。

main.cpp

#include <SFML/Graphics.hpp>
#include "counter.hpp"

std::string day(int num) { return (num!=1)? "s":""; } //the 's' in the word day(s) in the text displayed

int main()
{
    sf::RenderWindow window(sf::VideoMode(800,300), "New Year Counter");
    window.setVerticalSyncEnabled(true);
    window.setPosition(sf::Vector2i(sf::VideoMode::getDesktopMode().width/2 -400, sf::VideoMode::getDesktopMode().height/2 -150)); //put window in the middle of the screen

    sf::Font font;
    font.loadFromFile("Beyond Wonderland.ttf");
    sf::Text text;
    text.setFont(font);
    text.setCharacterSize(80);
    text.setFillColor(sf::Color::White);
    text.move(25, 10);

    sf::Text counter = text; //initialize with the same attributes as text so that I won't have to write everything again.

    while(window.isOpen())
    {
        sf::Event event;
        while(window.pollEvent(event))
        {
            if(event.type == sf::Event::Closed)
                window.close();
        }      

        TimeToNewYear year;
        window.clear(sf::Color::Black);
        text.setString("Until New Year "+year.get_newYear()+"\n"+year.get_daysLeft()+" day"+day(std::stoi(year.get_daysLeft()))+" left\n\t\tand");
        window.draw(text);
        counter.setString(year.get_counter());
        counter.setCharacterSize(120);
        counter.setPosition(360, 140);
        window.draw(counter);
        window.display();
    }
}

counter.hpp

#ifndef COUNTER_HPP
#define COUNTER_HPP
#include <string>
#include <ctime>

class TimeToNewYear
{
private:
    time_t m_nowTime = time(0);
    time_t m_tmny; //for mktime (time new year)
    tm *m_locTime = localtime(&m_nowTime);
    tm m_newYear {};
    int m_timeLeft;
    int m_daysLeft;
    std::string m_counter;

    void addNum(int num) //to display single digits with a 0 ex: 03
    {
        if(num<=9)
            m_counter += "0";
        m_counter += std::to_string(num);
    }
public:
    TimeToNewYear()
    { 
        m_newYear.tm_mday =1; 
        m_newYear.tm_year = 1+ m_locTime->tm_year; //set m_newYear
        m_tmny = mktime(&m_newYear);
        m_timeLeft = difftime(m_tmny, m_nowTime);
        //calculate days left
        m_daysLeft = m_timeLeft/(24*3600);
        m_timeLeft %= 24*3600;
    }

    std::string get_daysLeft() const { return std::to_string(m_daysLeft); }
    std::string get_newYear()  const { return std::to_string(m_newYear.tm_year +1900); } //tm_year is the years from 1900
    std::string get_counter()
    {
        addNum(m_timeLeft/3600);
        m_counter += ":";
        m_timeLeft %= 3600;
        addNum(m_timeLeft/60);
        m_counter += ":";
        addNum(m_timeLeft%60);
        return m_counter;
    }
};

#endif

Answer 1

我看到了一些可以帮助您改进代码的东西。

与实现

分离接口

接口进入一个头文件，实现(即，所有实际发出的字节(包括所有函数和数据)都应该位于一个单独的.cpp文件中。原因是您可能有多个源文件，包括.h文件，但只有对应的.cpp文件的一个实例。换句话说，将现有的counter.hpp文件分成一个.h文件和一个.cpp文件。

添加错误检查

只有在程序运行时指定的特定字体在当前工作目录中时，程序才能成功运行。如果字体文件不存在，程序将运行，但只在屏幕上创建一个黑匣子。我们可以做得更好！我建议你这样做：

const std::string fontfile{"Beyond Wonderland.ttf"};
if (!font.loadFromFile(fontfile)) {
    std::cerr << "Error: could not load font \"" << fontfile << "\"\n";
    return 1;
}

想到的是用户

用户可能希望使用不同的字体或不同的颜色。最好允许用户从命令行中指定这类内容。

消除了“幻数”

在许多情况下，代码使用“魔术数字”，如400和150，但没有明显的意义。作为命名常量，这将更好地使代码更易于理解和维护。

消除工作

按照当前编写代码的方式，代码在循环中每次迭代时都会创建和销毁TimeToNewYear类的副本。那真的没必要。更有意义的是，在循环之外创建所有东西，只更新需要在循环中更新的内容。

简化了类

当前的TimeToNewYear类拥有比它真正需要的更多的私有变量。我建议你重新考虑一下这门课。由于它的用途已经非常具体，您可以进一步对其进行专门化，并让它返回程序使用的两个格式化字符串。或者，可以通过为任意的未来日期和时间提供倒计时功能，使其更具一般性。这里有一个使用std::chrono标准库的替代方法。

class CountdownTimer
{
private:
    using tp = std::chrono::time_point<std::chrono::system_clock>;
    tp end;
    std::tm end_tm;
public:
    CountdownTimer(tp end);
    int daysLeft() const;
    int secondsLeft() const;
    std::string HMS_left() const;
    int endYear() const;
};

这里显示了两个成员函数，作为一个开始：

CountdownTimer::CountdownTimer(tp end) : end{end}
{
    time_t tt = std::chrono::system_clock::to_time_t(end);
    end_tm = *localtime(&tt);
}

int CountdownTimer::daysLeft() const { 
    using days = std::chrono::duration<int, std::ratio<24*60*60, 1>>;
    auto now = std::chrono::system_clock::now();
    return std::chrono::duration_cast<days>(end - now).count();
}

使用更好的命名

名为text和year的变量不是特别描述性的，甚至可能具有误导性。更好的名称更清楚地描述了变量的用途。

考虑其他系统资源

尽管与垂直同步同步(此代码所做的)同步在一定程度上减少了负载，但每次循环中都会发生大量的字符串创建和破坏，无论是否有任何视觉上的变化。我建议保持最后几秒钟的计数，并且只有在发生变化时才更新字符串。这里不会有任何重大变化，但养成将计算机所需工作最小化的习惯是很好的，特别是在循环调用的情况下。