329377 - Bugzilla::Object base class for objects

Status: RESOLVED FIXED

(Bugzilla :: Bugzilla-General, enhancement)

Description

[Max Kanat-Alexander]

We now have a lot of objects that have nearly-identical code. We have get_all_X, new, and soon new_from_list for Bugzilla::Component, Bugzilla::Version, Bugzilla::Product, Bugzilla::Classification, Bugzilla::Milestone, etc. All the code for these functions is nearly identical.

We could save a lot of coding by just creating one base class that handles a lot of the basics and have each object call that. It would also make creating new objects even easier.

Comment 1

[Max Kanat-Alexander]

Attachment: v1

Okay, here it is. This should make writing objects *way* easier.

You can see that I converted Bugzilla::Keyword to use it, just so that we could have something actually using it and I could be sure it worked.

It has POD so you should be able to read that and have some idea of how it works. Also, seeing how Bugzilla::Keyword uses it should help.

Comment 2

[Kevin Benton]

Comment on attachment 222795 [details] [diff] [review]
v1

Very nice job, Max.

Comment 3

[Frédéric Buclin]

Bugs and flags have no name; milestones and versions have neither an ID nor a name. How do you plan to use Bugzilla::Object in this case? Consider this as an r- till this problem is at least discussed.

Comment 4

[Max Kanat-Alexander]

(In reply to comment #3)
> Bugs and flags have no name; milestones and versions have neither an ID nor a
> name. How do you plan to use Bugzilla::Object in this case? Consider this as an
> r- till this problem is at least discussed.

  We'll handle that when we get there.

Comment 5

[Frédéric Buclin]

(In reply to comment #4)
>   We'll handle that when we get there.

I disagree. This must be discussed before the first version of Bugzilla::Object.

Comment 6

[Max Kanat-Alexander]

(In reply to comment #5)
> (In reply to comment #4)
> >   We'll handle that when we get there.
> 
> I disagree. This must be discussed before the first version of
> Bugzilla::Object.

  Okay. Then the answer is:

  We will either:

  1) Modify Bugzilla::Object so that it accepts other DB fields as the "name"
  2) Modify the database in some way to make the tables fit better with the object
  3) Override the "new" method in those objects to be sensible for those objects.
  4) Not use Bugzilla::Object for those objects.

  The nice part about object-oriented code is that it's easy to make those decisions when they have to be made, and not before.

Comment 7

[Frédéric Buclin]

Comment on attachment 222795 [details] [diff] [review]
v1

>Index: Bugzilla/Keyword.pm

> =head1 SUBROUTINES
> 
>+This is only a list of subroutines specific to C<Bugzilla::Keyword>.
>+See L<Bugzilla::Object> for more subroutines that this object 
>+implements.
> 
> =item C<keyword_count()> 

This file doesn't pass tests:

*** ERROR: =item without previous =over at line 80 in file Bugzilla/Keyword.pm
not ok 84 - Bugzilla/Keyword.pm has incorrect POD syntax --ERROR

There is a missing =over before =item C<keyword_count()>. Please fix that on checkin.



>Index: Bugzilla/Object.pm

>+sub new_from_list {

>+    if (@$id_list) {

Nit: I prefer "if (scalar(@$id_list))".


Your patch looks good and works fine. r=LpSolit with the fix on checkin.

Comment 8

[Max Kanat-Alexander]

Okay, I fixed the POD on checkin.

Checking in config.cgi;
/cvsroot/mozilla/webtools/bugzilla/config.cgi,v  <--  config.cgi
new revision: 1.19; previous revision: 1.18
done
Checking in Bugzilla/Keyword.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Keyword.pm,v  <--  Keyword.pm
new revision: 1.4; previous revision: 1.3
done
RCS file: /cvsroot/mozilla/webtools/bugzilla/Bugzilla/Object.pm,v
done
Checking in Bugzilla/Object.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Object.pm,v  <--  Object.pm
initial revision: 1.1
done
Checking in Bugzilla/Search/Quicksearch.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Search/Quicksearch.pm,v  <--  Quicksearch.pm
new revision: 1.5; previous revision: 1.4
done

Comment 9

[Max Kanat-Alexander]

Added to the release notes in bug 349423.