Closed (fixed)
Project:
Simplenews
Version:
7.x-1.x-dev
Component:
Code
Priority:
Normal
Category:
Task
Assigned:
Reporter:
Created:
24 Jan 2011 at 14:17 UTC
Updated:
11 Jan 2012 at 13:20 UTC
In simplenews we're dealing with data from DB, forms and have e.g. default loader functions.
Also some data merge happens in between.
Currently not all of this is consistent.
I think we should make cleaner API for object creation and avoid typecasts (object) (array) as much as possible.
The mixture is source of many issues we ran into recently.
Comments
Comment #1
miro_dietikerPossibly we should check this. It might already be covered by current tests from Berdir.
Comment #2
berdirI have a local cleanup/todo-removal branch that I need to finish and push as a patch, that deals with a number of the existing (object) array('mail' => $mail) declarations by replacing them with the existing _simplenews_user_load() function that automatically creates a an object with just the mail if there is no corresponding user.
Apart from that, not sure what you are exactly talking about here. The API's look quite clean and D7-like (compared to the 6.x-2.x branch for example).
The main thing that I noticed are some duplicating/overlapping/confusingly named functions. Two examples:
simplenews_get_subscription($acount): Doesn't return a *subscription* but a subscriber object. Should probably be renamed to _get_subscriber, or better, _load_subscriber_by_account(), which is similar to e.g. user_load_by_mail() and so on.
simplenews_get_mailing_lists($show_all) is a wrapper of simplenews_categories_load_multiple() with either the hidden condition set or not.
Comment #3
miro_dietikerI didn't check the current codebase.
Generally in 6.x we had a mixture of object / array notations and they sometimes clashed...
If this is clean now, we can leave it that way currently.
In past, sometimes we had issues because some data objects changed its type in some specific situations. This can even lead to fatal PHP errors and debugging is a pain because e.g. only in certain conditions an expected "object" was an array or vice versa...
Just close if this is known to be clean now and i'm happy. ;-)
Comment #4
berdirI didn't notice more than what I'm trying to clean up in my todo-branch for which I'll open a separate issue. Also opened a separte issue for the function naming stuff: #1387624: Rename/merge duplicating/confusingly named functions