[AGENT++] Code duplication in MibTable ...

[Frank Fock]

Hello Jens,

You are right, the redundancy can be eliminated.
It is there because of the code history. The
Array template has been introduced later and
the alternative OrderedList implementations have
been kept around for backward compatibility only.

I think, the best way would be to remove them
and use only the code for USE_ARRAY_TEMPLATE.

Best regards,
Frank

Jens Engel wrote:
> MibTable contains a lot of code duplication that depends on the internal
> MibTableRow column-container implementation.
> 
> BAD-EXAMPLE:
> void MibTable::get_required_columns(boolean* required, Vbx* pvbs)
> {
> #ifndef USE_ARRAY_TEMPLATE
>       OrderedListCursor<MibLeaf> cur;
>       int i;
>       for (cur.init(&generator.row), i=0; cur.get(); cur.next(), i++) {
>             if ((cur.get()->get_access() == READCREATE) &&
>                 (!cur.get()->has_default()))
>                   required[i] = TRUE;
>             else {
>                   required[i] = FALSE;
>                   if (pvbs)
>                         pvbs[i] = cur.get()->get_value();
>             }
>       }
> #else
>       for (int i=0; i<generator.row.size(); i++) {
>             if ((generator.row[i].get_access() == READCREATE) &&
>                 (!generator.row[i].has_default()))
>                   required[i] = TRUE;
>             else {
>                   required[i] = FALSE;
>                   if (pvbs)
>                         pvbs[i] = generator.row[i].get_value();
>             }
>       }
> #endif
> }
> 
> SLIGHTLY-IMPROVED-EXAMPLE (from existing code base, but why not used
> everywhere):
> ...
> #ifdef USE_ARRAY_TEMPLATE
>             ArrayCursor<MibLeaf> cur;
> #else
>             OrderedListCursor<MibLeaf> cur;
> #endif
> 
> DESIRED:
>       MibTableRow::Cursor_t cur;
>       ...
> 
> In my opinion, this code duplication is completely pointless and should be
> removed
> (as was partly done in some MibTable method bodies that replicates my
> solution below in its own context (SLIGHTLY-IMPROVED-EXAMPLE)).
> You only need to provide a public Cursor_t type definition dependent on the
> internal container class
> and remove all #ifdef sections in the MibTable class.
> 
> SOLUTION-EXAMPLE:
> class MibTableRow {
> ...
> 
> public:
> #  ifdef USE_ARRAY_TEMPLATE
>     typedef ArrayCursor<Agentpp::MibLeaf> Cursor_t;
> #  else
>     typedef OrderedListCursor<Agentpp::MibLeaf> Cursor_t;
> #  endif
>     // -- FRIENDLESS ACCESSOR: Allow row.column cursor iteration in a
> derived MibTable class without being a friend.
>     Cursor_t cursor(void) const { Cursor_t cur; cur.init(&row); return cur;
> }
> #endif
> };
> 
> USAGE-EXAMPLE WITH MY SOLUTION (changed from above):
> void MibTable::get_required_columns(boolean* required, Vbx* pvbs)
> {
>       MibTableRow::Cursor_t cur(generator.cursor());
>       for (int i=0; cur.get(); cur.next(), i++) {
>             if ((cur.get()->get_access() == READCREATE) &&
>                 (!cur.get()->has_default()))
>                   required[i] = TRUE;
>             else {
>                   required[i] = FALSE;
>                   if (pvbs)
>                         pvbs[i] = cur.get()->get_value();
>             }
>       }
> }
> 
> NOTE:
> MibTable::init_row() uses a ListCursor<> instead of a OrderedListCursor<>
> !?!
> If multiple cursor categories are needed, multiple typedefs and accessors
> should be provided.
> 
> Ciao,
> Jens Engel
> 
> 
> _______________________________________________
> AGENTPP mailing list
> AGENTPP at agentpp.org
> http://lists.agentpp.org/mailman/listinfo/agentpp

-- 
AGENT++
http://www.agentpp.com
http://www.mibexplorer.com
http://www.mibdesigner.com