[Buoh-dev] Fechas de publicación

[Esteban Sánchez]

El mié, 28-09-2005 a las 19:56 +0200, Carlos Garcia Campos escribió:
> El mié, 28-09-2005 a las 18:50 +0200, Esteban Sánchez escribió:
> > Acabo de añadir una propiedad más al dialogo de propiedades, en el
> > caso
> > de que sea un comic-manager-date se muestra los días de la semana en
> > los
> > que se publica. Para ello he creado un método get_publication_days en
> > la
> > clase BuohComicManagerDate que devuelve una cadena "human-readable".
> > No
> > sé si es la solución más OO, pero lo he hecho así por dos razones:
> > 
> > 1- El que lo use no tiene porqué saber como lo almacena interiormente
> > 2- Desde fuera nunca se va a usar las "restricciones".
> 
> está perfecto, lo que no interesa de cara al exterior son los dias que
> sale publicado

No sé si te habrás liado, pero es justo al contrario :) Bueno, que da
igual :P

> > +static const gchar *
> > +get_dayweek (GDateWeekday d)
> 
> ponle el prefijo, aunque sea un función auxiliar y privada, por convenio
> con el resto del código. Añade también un prototipo al principio del
> fichero.
> 
> > +{
> > +       switch (d) {
> > +       case G_DATE_MONDAY:
> > +               return (_("Monday"));
> > +       
> > +       case G_DATE_TUESDAY:
> > +               return (_("Tuesday"));
> > +       
> > +       case G_DATE_WEDNESDAY:
> > +               return (_("Wednesday"));
> > +       
> > +       case G_DATE_THURSDAY:
> > +               return (_("Thursday"));
> > +       
> > +       case G_DATE_FRIDAY:
> > +               return (_("Friday"));
> > +       
> > +       case G_DATE_SATURDAY:
> > +               return (_("Saturday"));
> > +       
> > +       case G_DATE_SUNDAY:
> > +               return (_("Sunday"));
> 
> yo para esto propondría algo como:
> 
> static const gchar *day_names [] {
> 	NULL,
> 	N_("Monday"),
> 	.......
> };

No se me habría ocurrido, además empecé haciendolo con un switch, y
luego vi que era mejor una función, así que copié y pegué

> y simplemente devolver g_strdup (day_names[d]);
> 
> compruebas antes que el índice no se vaya de rango y listo

Si se sale de rango ¿no petaría antes en ejecución al pasarle algo fuera
del enumerado GDateWeekDay? Pero claro, una comprobación de más nunca
viene mal

> > +       
> > +       case G_DATE_BAD_WEEKDAY:
> > +       default:
> > +               return ("Bad weekday");
> 
> realmente nos interesa devolver esto? No tiene ninguna utilidad, además
> que no lo estas manejando, así que en caso de devolver esto, llegará al
> gui una cadena sin traducir que no pinta mucho. Creo que sería mejor
> devolver NULL y controlarlo después, para en caso de recibir NULL,
> simplemente ignorarlo. 

Idem a la anterior respuesta, no llegaría ahí (vale, no puedo asegurar quien usa 
la función, así que lo cambio), pero mi razonamiento fue ese.

> > +       }
> > +}
> > +
> > +gchar *
> > +buoh_comic_manager_date_get_publications_days (BuohComicManagerDate
> > *comic_manager)
> 
> hmm no sería publication_days? (dias de publicación no de publicaciones,
> no?)

Sí, me lié un poco con el idioma :P

> > +               if (!comic_manager->priv->restrictions[i]) {
> 
> nunca he entendido esto de las restricciones. Si son los dias de
> publicación, por que no se llama simplemente publication_days y tiene
> una lógica directa en vez de indirecta? Vamos que cada vez que veo esto
> de las restricciones me tengo que acordar que corresponde a los dias de
> pucblicación, pero teniendo en cuenta que FALSE significa que si se
> publica ese dia. 

Bueno, en principio se da por hecho que los comics salen todos los días,
así que los que no son así tienen restricciones. Esto parece lógico,
pero quizás al implementarlo seguí la misma lógica (sin hacerla
positiva) y salió el lío este :) Si me animo lo cambio porque
evidentemente es más ofuscado, pero como lo hice yo pues no cai en la
cuenta.

> > +                       if (prev == 0) {
> 
> si es booleana, == 0 te obliga a pensar, 0 era verdadero o falso? usa
> mejor TRUE o FALSE o simplemente !

Esto fue un desliz producto de reescribir el algoritmo (antes era un
gint). Prefiero no usar ni TRUE ni FALSE, si no directamente prev.

> > +                               if (aux->len) {
> 
> es necesario comprobar siempre esto antes de un append? en este punto
> nos hemos metido ya en un 4 nivel   for { if { if { if { Esto es una
> locura, si hay manera de mejorarlo bien sino pues na.

Es la única manera que he visto de hacerlo sin repetir código. Si lo
prefieres lo pongo así (no se me ocurren más ahora a bote pronto):

if (prev && aux->len) {
	g_string_append (aux, ", ");
	g_string_append (aux, get_dayweek (i));
	last_printed = i;
} else if (prev) {
	g_string_append (aux, get_dayweek (i));
	last_printed = i;
}

> Salu2

Mañana mando el parche con más calma.

Saludos!
-- 
Esteban Sánchez
 esteban at steve-0.com
 http://steve-o.org
 http://subanales.com/
 ------------------------------------------------
 PGP key: http://pgp.mit.edu:11371/pks/lookup?op=get&search=0xB6E0F8AF
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Esta parte del mensaje =?ISO-8859-1?Q?est=E1?= firmada
	digitalmente
Url : http://forge.novell.com/pipermail/buoh-dev/attachments/20050929/f032ec20/attachment.pgp