StarWing83, 你的耐心真让我佩服!!!

这是之前StarWing83帮我改的程序,但有个人对这段程序做了如下评语,大家觉得他说的有根据吗?如果没有请告诉我他说的哪不对!我好反驳他!急!


#include "CLF_DateTime.h"
#include <iostream>
using namespace std;

lnline void CLF_DateTime::carry( int& v1,int& v2,int v3 )
{
    v1+=v2/v3; //Remark From Terence:  在沒有測試的情況下導入數據 ( 數據完整性)
    v2%=v3; //Remark From Terence:  在沒有測試的情況下導入數據 ( 數據完整性)
}

lnline bool CLF_DateTime::isleap( int y )
{
    return y%100?!(y%4):!(y%400);   //Remark From Terence:  x ? a : b  這種語法是不建議使用的. (可讀性, 可維護性)
                                                    //Remark From Terence:  在判斷式內運算是很差的語法. (可讀性, 可維護性)
 
}

bool CLF_DateTime::isleap()
{
    return isleap(tm_year+TM_TIMEBASE);
}

CLF_DateTime::CLF_DateTime(int year/*=1900*/,int month/*=1*/,int day/*=1*/, int hour/*=0*/,int min/*=0*/,int sec/*=0*/)
{
    tm_year=year-TM_TIMEBASE; //Remark From Terence:  在沒有測試的情況下導入數據 (數據完整性 )
    tm_mon=month-1;//Remark From Terence:  在沒有測試的情況下導入數據 ( 數據完整性)
    tm_mday=day;//Remark From Terence:  在沒有測試的情況下導入數據 ( 數據完整性)
    tm_hour=hour;//Remark From Terence:  在沒有測試的情況下導入數據 ( 數據完整性)
    tm_min=min; //Remark From Terence:  在沒有測試的情況下導入數據 (數據完整性 )
    tm_sec=sec; //Remark From Terence:  在沒有測試的情況下導入數據 (數據完整性 )
}

char* CLF_DateTime::display( char* strdisplay )
{
    strftime(strdisplay,80,"%Y-%m-%d %H:%M:%S",this); //Remark From Terence:  在沒有測試strdisplay指針下使用指針 ( 指針使用安全)
    return strdisplay;
}

char* CLF_DateTime::setdisplay( char* strdisplay )
{
    tm_min+=tm_sec>=30; //Remark From Terence:  在判斷式內運算是很差的語法. (可讀性, 可維護性) (若在校時使用這類語法會評係劣級)
    tm_sec=0;
    carry(tm_hour,tm_min,60);
    carry(tm_mday,tm_hour,24);
    tm_mday--;
    switch (tm_mon+1)
    {
    case 1:
    case 3:
    case 5:
    case 7:
    case 8:
    case 10:
    case 12:
        carry(tm_mon,tm_mday,31);
        break;
    case 2:
        carry(tm_mon,tm_mday,28+isleap());
        break;
    default:
        carry(tm_mon,tm_mday,30);
    }
    tm_mday++;
    carry(tm_year,tm_mon,12);
    return display(strdisplay);

    //Remark From Terence:  你把四捨五進變成什麼, 我說那是要可以設定進位精度的. 進位至分, 進位至時, 進位至日..............
     //Remark From Terence:  潤年 潤月 的計算已在 time 庫考慮了, 不用自己去做. 而且這樣做也不精確.
      
}

int CLF_DateTime::compare( CLF_DateTime& pb )
{
    int t=0;
    if (t=tm_year-pb.tm_year) //Remark From Terence:  在判斷式內運算是很差的語法. (可讀性, 可維護性) (若在校時使用這類語法會評係劣級)
        goto ret; //Remark From Terence:  在goto是很差的語法.它破壞了程式的邏輯結構 (可讀性, 可維護性) (只在scriping language 用)
    if (t=tm_mon -pb.tm_mon ) //Remark From Terence:  在判斷式內運算是很差的語法. (可讀性, 可維護性) (若在校時使用這類語法會評係劣級)
        goto ret; //Remark From Terence:  在goto是很差的語法.它破壞了程式的邏輯結構 (可讀性, 可維護性) (只在scriping language 用)
    if (t=tm_mday-pb.tm_mday) //Remark From Terence:  在判斷式內運算是很差的語法. (可讀性, 可維護性) (若在校時使用這類語法會評係劣級)
        goto ret; //Remark From Terence:  在goto是很差的語法.它破壞了程式的邏輯結構 (可讀性, 可維護性) (只在scriping language 用)
    if (t=tm_hour-pb.tm_hour) //Remark From Terence:  在判斷式內運算是很差的語法. (可讀性, 可維護性) (若在校時使用這類語法會評係劣級)
        goto ret; //Remark From Terence:  在goto是很差的語法.它破壞了程式的邏輯結構 (可讀性, 可維護性) (只在scriping language 用)
    if (t=tm_min -pb.tm_min ) //Remark From Terence:  在判斷式內運算是很差的語法. (可讀性, 可維護性) (若在校時使用這類語法會評係劣級)
        goto ret; //Remark From Terence:  在goto是很差的語法.它破壞了程式的邏輯結構 (可讀性, 可維護性) (只在scriping language 用)
    if (t=tm_sec -pb.tm_sec ) //Remark From Terence:  在判斷式內運算是很差的語法. (可讀性, 可維護性) (若在校時使用這類語法會評係劣級)
        goto ret; //Remark From Terence:  在goto是很差的語法.它破壞了程式的邏輯結構 (可讀性, 可維護性) (只在scriping language 用)
    return t; //Remark From Terence:  多點回傳, 是很差的語法.它破壞了程式的邏輯結構 (可讀性, 可維護性)
ret:
    return t>0?1:-1; //Remark From Terence:  在判斷式內運算是很差的語法. (可讀性, 可維護性) (若在校時使用這類語法會評係劣級)
                           //Remark From Terence:  多點回傳, 是很差的語法.它破壞了程式的邏輯結構 (可讀性, 可維護性)
}

首先，我应该承认。我没有判断一些函数的输入参数。改过如下：

CLF_DateTime::CLF_DateTime(int year/*=1900*/,int month/*=1*/,int day/*=1*/, int hour/*=0*/,int min/*=0*/,int sec/*=0*/) {
    assert(year>=TM_TIMEBASE);
    assert(month>=0&&month<=11);
    assert(day>=1&&day<=31);
    assert(hour>=0&&hour<=23);
    assert(min>=0&&min<=59);
    assert(sec>=0&&sec<=59);
    tm_year=year-TM_TIMEBASE;
    tm_mon=month-1;
    tm_mday=day;
    tm_hour=hour;
    tm_min=min;
    tm_sec=sec;
}

char* CLF_DateTime::display( char* strdisplay )  {
    assert(strdisplay!=NULL);
    strftime(strdisplay,80,"%Y-%m-%d %H:%M:%S",this); //Remark From Terence:  在沒有測試strdisplay指針下使用指針 ( 指針使用安全)
    return strdisplay;
}

因为原先的代码就没有做这件事情，所以我忽略了，这是我的问题。因为是从别人的代码开始入手，我从一开始就忽略了一些必要的安全手法。平时写代码的时候我是很注意的，但是还不够。以后我会注意这方面的问题。

其次，对于闰年的判断，的确是很差的语法，不过相对于y%400==0||(y%100!=0 && y%4==0)，我觉得在可读性上面是差不多的。作为一个子模块。我坚持自己的语法规则。现在的VS2005支持指向性变量类型判断。所以我抛弃了一些老的类型判断方法。（比如BOOL型写if(var)，而int型写if(var!=0)，指针写if(var!=NULL)等等。在不引起歧义的地方，我通常对普通变量写if(var)，对指针写if(var!=NULL，这是我的风格。如果引起维护困难，我表示抱歉并改正。但是目前我并没有发现有什么不好。）

原先我的代码没有判断闰年的部分。而且我并没有得到对于setdisplay函数的任何要求。至始至终我没有看到对于整个项目的解释和要求（可以参看我前面的帖子）。我是根据单调黑白的代码提供的功能完成这个代码的。如果功能有出入不应该由我负责。

对compare的评语我不同意。首先这个函数是为了效率考虑才这么写的。原先我没有使用goto（参见我上面的帖子），使用goto是为了避免编译重复代码(这里是t>0?1:-1），而且，两个return放在后面，前面全是判断，结构化非常清晰，也十分利于调试和维护，我觉得在风格上面没有错误。goto不算洪水猛兽，正确的使用会带来效率的提升和清晰的结构化编程。对于“if (t=tm_year-pb.tm_year)”，Terence说是很差的语法。请提供一个同样高效的，并且利于维护和调试的compare，我乐于学习和进步。

其次是？：的问题。同上，对于？：我并不持否定态度。的确，滥用？：（比如嵌套使用）的确会影响代码的可读性。但是在很多情况，当使用if的时候会有重复代码的情形下，？：会提供更好的代码清晰性。我承认对代码没有做足够的优化，很多？：是可以用if代替的。但是并不是一看到？：就必须持否定的态度。事物都是有两面性的。t=sgn(m)?10:-10;绝对比if(sgn(m)!=0)t=10;else t=-10;要清晰可读得多。请参看下面我对？：的理解。

最后是carry问题。carry是私有内联函数。根据约定私有内联函数不需要判断输入参数的有效性。而且我在前面的回帖中特别强调过，carry没有做有效性检查，只是简单将多出来的时间加到上级时间里面去。如果需要检查，可以自己写代码（请参看上面的回帖），因为我并没有得到关于本程序的任何文档要求，所以这不是我的责任。

因为我仍在学习之中，如果有不妥和错误的地方请谅解。我会努力改正。但是对于不合理的东西我也应该有自己的判断。我也对糟糕的代码和难以理解维护的风格深通恶绝。但是糟糕的风格不代表仅仅是使用了goto和？：。任何事物的存在都有其合理性。我们不能一味禁止。我的理解是，对于元操作（比如上面的例子t=sgn(m)?10:-10;）使用？：可以增强代码的可读性和可维护性（你不需要改掉两个t=）。对于有固定出口的多重if代码，使用goto导向唯一出口可以增强代码的可读性和可维护性。其余的情况。我绝不会使用这两种语法。对于我的理解有误的地方，请指正。
最后，谢谢Terence耐心的评论和指责。我认识到了自己的不足，这样才能有所进步。对于Terence的劳动。我表示感激。谢谢。

[[italic] 本帖最后由 StarWing83 于 2007-12-19 17:18 编辑 [/italic]]

_VALIDATE_RETURN_ERRCODE(
    (
        ( tb->tm_mday >= 1 ) &&
        (
            // Day is in valid range for the month
            ( ( _days[ tb->tm_mon + 1 ] - _days[ tb->tm_mon ] ) >=
              tb->tm_mday ) ||
            // Special case for Feb in a leap year
            (
                ( IS_LEAP_YEAR( tb->tm_year + 1900 ) ) &&
                ( tb->tm_mon == 1 ) &&
                ( tb->tm_mday <= 29 )
            )
        )
    ),
    EINVAL
)

又及，上面是CRT对于日期（天）错误的判断，如果对assert仍然不满意，可以使用这个。

在吸取Terence的意见以及要求后，我的实现如下，如果还不能满足要求，请提供详细的要求文档。谢谢。
在CLF.h中：

#ifndef _CLF_H_
#define _CLF_H_
#include <ctime>

#define _YEAR_BASE          1900
#define _MAX_YEAR          138                  /* 2038 is the max year */

class CLF_DateTime : public tm {
public:
    enum FType {FT_SEC,FT_MIN,FT_HOUR,FT_DAY,FT_MON};
    CLF_DateTime(int year=_YEAR_BASE,int month=1,int day=1,
                 int hour=0,int min=0,int sec=0);
    ~CLF_DateTime();
    void SetTime(int year,int month,int day,int hour,int min,int sec);
    time_t GetTime();
    void Format(FType type);
    int Compare(CLF_DateTime& pb);
    char* Display(char* strdisplay);
    friend std::ostream& operator<<(std::ostream& o,CLF_DateTime& t);
};

//内联函数
inline time_t CLF_DateTime::GetTime() {
    return mktime(this);
}
#endif

在CLF.cpp中：

#include <iostream>
using namespace std;
#include "CLF.h"
#include <assert.h>//注：请使用自己的断言头文件

#define STR_MAX 100

//使用了CRT的方法
const char _days[]={31,29,31,30,31,30,31,31,30,31,30,31};

//如果v1超过了v3指定的值，则v2进位
inline void _format( int& v1,int& v2,int v3 ) {
    if (v1>=v3)v2++;
    v1=0;
}

CLF_DateTime::CLF_DateTime( int year/*=_YEAR_BASE*/,int month/*=1*/,int day/*=1*/,
                            int hour/*=0*/,int min/*=0*/,int sec/*=0*/ ) {
    SetTime(year,month,day,hour,min,sec);
}
CLF_DateTime::~CLF_DateTime() {
}
void CLF_DateTime::SetTime( int year/*=_YEAR_BASE*/,int month/*=1*/,int day/*=1*/,
                            int hour/*=0*/,int min/*=0*/,int sec/*=0*/ ) {
    assert(year>=_YEAR_BASE && year<=_YEAR_BASE+_MAX_YEAR);
    assert(month>=1 && month<=12);
    assert(day>=1 && day<=_days[month-1]);
    assert(hour>=0 && hour<=23);
    assert(min>=0 && min<=59);
    assert(sec>=0 && sec<=59);
    tm_year=year-_YEAR_BASE;
    tm_mon=month-1;
    tm_mday=day;
    tm_hour=hour;
    tm_min=min;
    tm_sec=sec;
}
char* CLF_DateTime::Display( char* strdisplay ) {
    assert(strdisplay!=NULL);
    strftime(strdisplay,80,"%Y-%m-%d %H:%M:%S",this);
    return strdisplay;
}
//对时间四舍五入
void CLF_DateTime::Format(FType type ) {
    for (;;) {
        _format(tm_sec,tm_min,30);
        if (type==FT_SEC)break;
        _format(tm_min,tm_hour,30);
        if (type==FT_MIN)break;
        _format(tm_hour,tm_mday,12);
        if (type==FT_HOUR)break;
        _format(tm_mday,tm_mon,_days[tm_mon]/2);
        tm_mday++;
        if (type==FT_DAY)break;
        _format(tm_mon,tm_year,6);
        break;
    }
    //规格化时间
    time_t temp=mktime(this);
    *this=(CLF_DateTime&)*localtime(&temp);
}
//比较函数，如果pb大返回-1，反之返回1，如果相等返回0
int CLF_DateTime::Compare( CLF_DateTime& pb ) {
    if (&pb==this)return 0;
    double temp=difftime(GetTime(),pb.GetTime());
    if (temp>0)return 1;
    if (temp<0)return -1;
    return 0;
}
ostream& operator<<( ostream& o,CLF_DateTime& t ) {
    char str[STR_MAX];
    return o<<t.Display(str);
}

在test.cpp中：

#include <iostream>
#include "CLF.h"
using namespace std;

int main(){
    CLF_DateTime t1(2000,2,29,23,59,59);
    CLF_DateTime t2(2000,2,12,11,13,12);
    //t2.Format(CLF_DateTime::FType(100));//不会出错
    t2.Format(CLF_DateTime::FT_DAY);
    cout<<t2<<endl;
    cout<<(t2);
    getchar();
}

[[italic] 本帖最后由 StarWing83 于 2007-12-20 09:37 编辑 [/italic]]

评论得很有理嘛,但是我更佩服LS的。赖心真是好啊~~

程序员要有脾气，也要有修养。阿门………………

呵呵~~~~~~~

兄弟 这是这个程序要完成的功能明细:
3. Reture String with specific format (reference to CTime:Format)
        e.g.

        CLF_DateTime a = CLF_DateTime(2007, 11, 4, 22, 29, 54, 9987);

        a.Format("yyyy-mm-dd HH:MM:SS");

        

        output:

        2007-11-04 22:29:54

4. Provide rounding

        CLF_DateTime a = CLF_DateTime(2007, 11, 4, 22, 29, 54, 9987);

        CLF_DateTime b = a.Rounding(MINUTE);

        

        b will be "2007-11-04 22:30:00.0000"

5. Provide Compare functions  

        CLF_DateTime a = CLF_DateTime(2007, 11, 4, 22, 29, 54, 9987);

        CLF_DateTime b = CLF_DateTime(2007, 11, 4, 22, 30, 15, 0000);

        int iresult;

 

        iresult = CLF_DateTime::Compare(a, b, MINUTE, ROUNDING);

 

        Results:

        (a>b, iresult = 1)

        (a=b, iresult = 0)

        (a<b, iresult = -1)


以下是terence再次做的评论(供大家学习参考):
/////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
對於programme
1. assert 只會在 Debug mode 下實現, 在Release mode 會自動除去.  你的programme 不 Release 的嗎?

2. if(y%400==0||(y%100!=0 && y%4==0)
     我沒有說這不對, 我是說 C 已做了潤年的工作
     況且, 這段 code 也不好理解, 三個月之後自己也看不懂了.
3. goto 的好與不好, 不是我們說的. 多看書吧.

    if (t=tm_year-pb.tm_year) 就是在 if 還運術, 出了bug後是很難Debug的. 以行為單位的 Debug 跟本不知是那裡出錯. 邏輯錯了更難改.

4. "t=sgn(m)?10:-10;绝对比if(sgn(m)!=0)t=10;else t=-10;要清晰可读得多"?

   //Checking the sign of m
   if(sgn(m) == 0)
   {// m is zero , 只有m是零, sgn(m)才會零
          t = -10;
   }else
   {// m is not zero
          t = 10;
   }

   你能看出上面一段的 bug 嗎? 這才叫清晰可讀, 可維護, 程式不是只是給你自己看的.
   binary file size 不會比用 t=sgn(m)?-10:10 多.
   執行速度不會比用 t=sgn(m)?-10:10 慢.
   因為他們確實用相同的機器碼.


   //Checking the sign of m
   if(sgn(m) >= 0)
   {// m is positive number or zero
          t = -10;
   }else
   {// m is negative number
          t = 10;
   }
     x ? a :b 只用在單元測試和內部測試用. (我們叫 quick and ugly)
////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
还有一个事 您写的这个最新的程序 我运行了 得到的结果不是需求所要的结果啊  您再看看?

谢谢!!

意见我接受。但是类的几个函数的声明拿来，要不然没法写。我怎么会知道构造函数最后一个参数是什么东西？？
还有，你到底是包装的time.h还是MFC的CTime类？
最后，那个Compare是静态函数么？最后一个参数是什么意思？