[AD] [ alleg-Bugs-3056932 ] al_current_time does not fit Allegro naming convention

[ Thread Index | Date Index | More lists.liballeg.org/allegro-developers Archives ]


Bugs item #3056932, was opened at 2010-09-01 04:48
Message generated for change (Settings changed) made by tjaden
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=105665&aid=3056932&group_id=5665

Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: Core Library
Group: 4.9
>Status: Closed
>Resolution: Fixed
Priority: 3
Private: No
Submitted By: Mateusz Jończyk (mat2)
Assigned to: Nobody/Anonymous (nobody)
Summary: al_current_time does not fit Allegro naming convention

Initial Comment:
Should there be al_get_current_time() instead of al_current_time() ?

----------------------------------------------------------------------

Comment By: Trent Gamblin (trentg)
Date: 2010-09-01 23:26

Message:
Another possibility: we can put the is last for those two but we end up
with some different names:

al_bitmap_is_compatible
al_bitmap_is_sub_bitmap

----------------------------------------------------------------------

Comment By: Trent Gamblin (trentg)
Date: 2010-09-01 23:24

Message:
IMO, there is no easy solution in all cases, even Peters solution doesn't
work in all cases. Look at these:

al_is_path_present
al_path_is_present

Both of these work. But

al_is_fs_entry_file

This doesn't sound right to me.

However, if we put the is last, we have

al_is_sub_bitmap

This doesn't really work with is last.

My preferred solution, is to put is last in most cases, but where it is
not grammatically correct, put it first. That just leaves al_is_sub_bitmap
and al_is_compatible_bitmap with the is coming first.

----------------------------------------------------------------------

Comment By: Elias Pschernig (elias)
Date: 2010-09-01 23:14

Message:
<quote>Do we really need to have this discussion again?
It's been discussed several times now, and on each occasion there has
been
no agreement to change things. I don't see that changing now.
</quote>
The discussion is not whether to change things this time, but *how* to
change them - with an actual A5 release getting close not changing is not
an option any longer :)

The API right now has these function matching al_*_is_*:

al_is_path_present
al_event_queue_is_empty
al_is_compatible_bitmap
al_fs_entry_is_file
al_fs_entry_is_directory
al_is_audio_installed
al_is_keyboard_installed
al_is_bitmap_drawing_held
al_is_bitmap_locked
al_is_mouse_installed
al_is_sub_bitmap
al_is_opengl_extension_supported
al_timer_is_started

And there is no discernible pattern. Myself I'd prefer the al_is_
basically because I don't care either way and al_verb_* just would be a
simple solution. If there is some other way to explain the word order I'm
happy with that as well, whether that's always putting the _is_ last or
coming up with a rule like Peter's. I just want to have each of them
explained before having them in the official 5.0.0 API, otherwise I would
feel that we are not releasing a well thought-through API.

----------------------------------------------------------------------

Comment By: Evert Glebbeek (eglebbk)
Date: 2010-09-01 22:50

Message:
Do we really need to have this discussion again?
It's been discussed several times now, and on each occasion there has been
no agreement to change things. I don't see that changing now.

> So therefore I'd forget about the linguistic issues and just say
> al_verb_*. It's much easier to remember "al_verb_*" than "al_verb_*
except
> if the verb happens to be _is_". Exceptions are bad :p
I disagree, strongly. I find the un-grammatical construct much harder to
remember myself. Onbiously there is no objective truth here.

----------------------------------------------------------------------

Comment By: Elias Pschernig (elias)
Date: 2010-09-01 21:08

Message:
It depends on how you read if statements.

// Is the event queue empty?
if (al_is_event_queue_empty()) {

// Check if the event queue is empty.
if (al_event_queue_is_empty())

Myself I tend to read if statements as questions (skipping the "if" as
just a language directive indicating a condition point) and so
linguistically the _is_ in the back sounds wrong. Also queues have a
variable name:

// Is the event queue A empty?
if (al_is_event_queue_empty(A))

// Check if the event queue A is empty.
if (al_event_queue_is_empty(A))

And now in both cases the placement of the "A" is completely off.

So therefore I'd forget about the linguistic issues and just say
al_verb_*. It's much easier to remember "al_verb_*" than "al_verb_* except
if the verb happens to be _is_". Exceptions are bad :p

----------------------------------------------------------------------

Comment By: Peter Wang (tjaden)
Date: 2010-09-01 11:01

Message:
Might as well call it al_get_time() then. I wouldn't mind a #define
al_current_time myself.

As Evert said, noun_is_property sounds better in if statements. I would
prefer to standardise on that form *for properties*, e.g.
al_event_queue_is_empty, al_bitmap_is_locked, al_timer_is_started.

For queries which are not about properties of objects, we can keep the
al_is_ form. e.g. al_is_audio_installed, al_is_sub_bitmap,
al_is_compatible_bitmap, al_is_bitmap_drawing_held ("bitmap" doesn't really
need to be there).

This is just an idea. It might be too subtle a distinction.

----------------------------------------------------------------------

Comment By: Elias Pschernig (elias)
Date: 2010-09-01 05:29

Message:
Yes, it should be al_get_current_time(). I'll change it unless someone sees
a problem.

With the _is_ there are more cases, for example:
al_event_queue_is_empty -> al_is_event_queue_empty

I would agree and say always using al_is_* makes most sense for them but I
do remember a discussion about this once so will wait for more input.

----------------------------------------------------------------------

Comment By: Evert Glebbeek (eglebbk)
Date: 2010-09-01 05:27

Message:
Agreed taht it should be al_get_current_time().
Disagree about the others, to various degrees. Something like "if
(al_is_timer_started())" just reads wrong to me. Similar with
"al_was_mouse_button_held()", it sounds contrived.
Those changes, I think, actually make things harder because they don't
read natural.

----------------------------------------------------------------------

Comment By: Mateusz Jończyk (mat2)
Date: 2010-09-01 05:01

Message:
Maybe al_was_mouse_button_held(), al_was_key_down(),
al_turn_keycode_to_name() ?

----------------------------------------------------------------------

Comment By: Mateusz Jończyk (mat2)
Date: 2010-09-01 04:58

Message:
Other entries:

al_timer_is_started()                          ->    
al_is_timer_started()


al_fs_entry_is_file ()                                        -> 
al_is_fs_entry_file()             (???)
al_fs_entry_is_directory ()                               ->
al_is_fs_entry_directory()      (???)

Everything else does fit to the convention( except al_mouse_button_down(),
al_key_down(), al_keycode_to_name(), but I don't see a sensible way of
"correcting" them)

----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=105665&aid=3056932&group_id=5665




Mail converted by MHonArc 2.6.19+ http://listengine.tuxfamily.org/