|
|
|
Two things about reset_columns()
1) implementation looks buggy. If not table is passed, then the whole $this->columns should be = array() AFAIK. Easy to fix.
2) then, the method itself, I don't know in what situation we are going to use this, apart from all DDL stuff. So, IMO it would be easier to add one param to get_columns($table, $usecache = true). And them with that parameter:
a) if $usecache = true ==> current behaviour (using cache). Normal situation for DML stuff using get_columns()
b) if $usecache = false ==> never use the cache + reset it (for requested table) after execution. And we'd add that second parameter to all the DDL stuff using get_columns().
How does it sound. IMO it's easier that adding reset_columns() here and there. Just one param to control behaviour (cache/no cache).
Ciao :-)
|
|
Description
|
Two things about reset_columns()
1) implementation looks buggy. If not table is passed, then the whole $this->columns should be = array() AFAIK. Easy to fix.
2) then, the method itself, I don't know in what situation we are going to use this, apart from all DDL stuff. So, IMO it would be easier to add one param to get_columns($table, $usecache = true). And them with that parameter:
a) if $usecache = true ==> current behaviour (using cache). Normal situation for DML stuff using get_columns()
b) if $usecache = false ==> never use the cache + reset it (for requested table) after execution. And we'd add that second parameter to all the DDL stuff using get_columns().
How does it sound. IMO it's easier that adding reset_columns() here and there. Just one param to control behaviour (cache/no cache).
Ciao :-) |
Show » |
made changes - 25/May/08 10:54 AM
| Field |
Original Value |
New Value |
|
Parent
|
|
MDL-14679
[ 26259
]
|
|
Issue Type
|
Bug
[ 1
]
|
Sub-task
[ 5
]
|
made changes - 25/May/08 05:03 PM
|
Status
|
Open
[ 1
]
|
In Progress
[ 3
]
|
made changes - 25/May/08 05:34 PM
|
Resolution
|
|
Fixed
[ 1
]
|
|
Status
|
In Progress
[ 3
]
|
Resolved
[ 5
]
|
made changes - 26/May/08 12:32 AM
|
QA Assignee
|
|
stronk7
|
|
Status
|
Resolved
[ 5
]
|
Closed
[ 6
]
|
|