View Issue Details
|ID||Project||Category||View Status||Date Submitted||Last Update|
|12074||Development||Other||public||2017-01-23 08:38||2017-04-25 11:48|
|Summary||12074: code readability/manageability improvement|
thank you for your good work on Linesurvey! I have small improvement for better code readability/manageability - there is used so much strings like 'A', 'B', 'C', ... in source in conditions for question types. I replaces those string by constants. Why? Modern IDE's can show you where constant/variable/class/whatever else is used in source, but searching for string like 'A' can't return the same comfortable result. I made it fo question types, but there are other string for graph types and in import/export. may be next time I'll improve other source code part this way.
Yes, I know, that best of all would be to create general question type class and inherit it for all question types. But this change will be hard to make, so I made only first step - constants.
I'll create pull request in a while.
Let's discuss it.
|Steps To Reproduce||This isn't any bug/issue report. This is created as announce of my pull request for improve code readability/quality. It doesn't fixes any bug/issue or add new feature. This is only code development improvement.|
|Tags||No tags attached.|
Really like the idea :) Maybe review contant name.
The idea : 1st part is ~ DB type, after put detail.
where is the best place to contact you? Here in issues tracker or in pull request discussion https://github.com/LimeSurvey/LimeSurvey/pull/629 ?
I created constant names like QT_A_ARRAY_5_CHOICE_QUESTIONS mainly to help me check my replacements ;) Character "A" is here for easier checking to replaced right string by right constant. Also a left the same constant values - for example QT_A_ARRAY_5_CHOICE_QUESTIONS has value "A", main reason for that was not to broke Limesurvey in case I forget somewhere replace "A" string.
Let me know your idea about Question class constants names and I'll rename it and make new pull request.
@jiripavlicek : yes pull request seems best, i'm not the only dev :).
@olle think it's best to have the old code (like you do)
@c-schmitz think it's best to make the pull for develop.
|Any ideas how should I change my pull request to be suitable for you? Thanks.|
|your PR is a great work. If you want to port it yourself, just do it in the develop branch, it will be released with LS3 in few months (with Twig engine for templates).|
|2017-01-23 08:38||jiripavlicek||New Issue|
|2017-01-23 08:52||DenisChenu||Severity||minor => feature|
|2017-01-23 08:58||DenisChenu||Note Added: 42791|
|2017-01-25 08:19||jiripavlicek||Note Added: 42825|
|2017-01-25 09:02||DenisChenu||Note Added: 42828|
|2017-02-09 08:05||jiripavlicek||Note Added: 42988|
||Note Added: 43440|