[Freeciv-Dev] Re: [PATCH] cleanup city_dialog_update_information()

[Mike Kaufman <mkaufman@xxxxxxxxxxxxxx>]

On Sun, Feb 03, 2002 at 12:26:29AM +0100, Raimar Falke wrote:
> On Sat, Feb 02, 2002 at 12:22:19PM -0600, Mike Kaufman wrote:
> > here's a straightforward patch to reduce the number of tests needed for
> > the happiness dialog in citydlg.c, plus it adds an enum and a couple of
> > defines to make things look nice.
> > 
> > It also moves some function prototypes around to help civworld a bit.
> > 
> > -mike
> 
> > +enum { OVERVIEW_PAGE, UNITS_PAGE, WORKLIST_PAGE,
> > +  HAPPINESS_PAGE, CMA_PAGE, TRADE_PAGE, MISC_PAGE
> > +};
> > +
> > +enum info_style { NORMAL, ORANGE, RED };
> >  
> > +#define NUM_INFO_STYLES 3
> > +#define NUM_PAGES 8             /* the number of pages in city dialog 
> > notebook 
> > +                                 * (+1) if you change this, you must add 
> > an 
> > +                                 * entry to misc_whichtab_label[] */
> 
> You get the NUM_* for free if you define it at the end of the enum:
> 
> enum info_style { NORMAL, ORANGE, RED, NUM_INFO_STYLES};

so you do.. ok.

> > -  /* someone could add the info_label_style[1] style */
> > -  /* for better granularity here                     */
> > +  /* someone could add the info_label_style[ORANGE] style */
> > +  /* for better granularity here                          */
> 
> This isn't the offical comment style.

sure, I originally did these comments 6 months ago. can change.

> > -  style = (pcity->pollution >= 10) ? 2 : 0;
> > -  gtk_widget_modify_style(pdialog->overview.info_label[POLLUTION],
> > -                     info_label_style[style]);
> > -  if (pdialog->pcity->owner == game.player_idx) {
> > -    gtk_widget_modify_style(pdialog->happiness.info_label[POLLUTION],
> > -                       info_label_style[style]);
> > -  }
> > +  style = (pcity->pollution >= 10) ? RED : NORMAL;
> > +  gtk_widget_modify_style(info_label[POLLUTION], info_label_style[style]);
> >  }
> 
> Why can the "if (pdialog->pcity->owner == game.player_idx)" test be
> removed?

cause you catch the necessary test up in refresh_city_dialog() You only
need to do it once.

-mike