We should cleanup the module code.

Many callbacks can be moved into a separated file and would reduce complexity of the base file.

Also we should limit the code execution complexity for regular calls. Especially if advpoll isn't even involved.

CommentFileSizeAuthor
#6 advpoll-cleanup-821044-6.patch145.96 KBjacob.embree
#3 Screen Shot 2015-02-21 at 5.08.49 PM.png54.06 KBChrisKennedy
#2 advpoll-cleanup-821044-2.patch167.93 KBjacob.embree

Comments

miro_dietiker’s picture

miro_dietiker
Location Switzerland
commented
Status: Active » Needs work

Committed a first cleanup.

Moved things out of advpoll.module.
Created .pages.inc and .admin.inc files.

Seems to be well checked, everything still works as expected.
Needs more cleanup!

jacob.embree’s picture

jacob.embree commented
Version: 6.x-1.x-dev » 7.x-3.x-dev
Issue summary: View changes
Status: Needs work » Needs review
StatusFileSize
new advpoll-cleanup-821044-2.patch167.93 KB

First attempt here. Plenty more to do. Part of the coder review involved fixing #2407473: Choice form element key is not concatenated correctly.

ChrisKennedy’s picture

ChrisKennedy commented
Status: Needs review » Needs work
StatusFileSize
new Screen Shot 2015-02-21 at 5.08.49 PM.png54.06 KB

Whoops, ignore this comment - meant to post it to the tests issue.

ChrisKennedy’s picture

ChrisKennedy commented

Ok, actually trying this patch it looks like it doesn't apply cleanly to the codebase - appreciate the work here though. I'm getting hunk failures in advpoll.info, advpoll_field.info, and advpoll_ranking.info right now.

tripper54’s picture

tripper54 commented
Status: Needs work » Closed (fixed)

Closing this as most of this work has been done through other commits.

jacob.embree’s picture

jacob.embree commented
Title: Cleanup code - separate into files » Cleanup code
Status: Closed (fixed) » Needs review
StatusFileSize
new advpoll-cleanup-821044-6.patch145.96 KB

Here is an updated patch. A lot was done, but there's a lot left.

tripper54’s picture

tripper54 commented
Status: Needs review » Fixed

Great work, thanks for taking the time to do this.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.