[Buoh-dev] Fechas de publicación

[esteban at steve-0.com (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 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