Должен ли я оставить эту статическую переменную или реорганизовать ее?

Question

Предыдущий разработчик разместил статическую строку под названием «Qry» в классе «god» в проекте, который я унаследовал.

Затем разработчик использовал эту статическую переменную в каждом отдельном месте всей программы, в которой построена и используется одна строка запроса db.

Например:

SomeGodClass.Qry = "select count(1) from AddressBook where Name = '" + txtName.Text.Trim(' ') + 
          "' and Group_Name = '" + txtGroupName.Text.Trim(' ') + "'"; 

int count = sqlHelper.ExecuteScalar(SomeGodClass.Qry); 

Таким образом, эта переменная ссылается ровно 626 раз, большинство с другим запросом быть назначены. Существуют и другие статические переменные, которые он использовал - вероятно, 50 из них, но это наиболее преобладающее.

Мой первый инстинкт - удалить эту статическую строку и переработать все 626 обычаев. Тем не менее, я не знаю, достаточно ли этой практики, чтобы тратить время на это.

Таким образом, мой вопрос: допустимо ли использование статической строки, особенно если принять во внимание объем работы по рефакторингу?

Answer 1

Не вижу проблем с использованием static здесь, по крайней мере, судя по коду, который я вижу. Но не используйте TextBox конкатенации!

Использовать вместо Parameters.

Это самый важный рефакторинг, о котором я могу думать, глядя на предоставленный код.

Why do we always prefer using parameters in SQL statements?

Answer 2

Вы определенно должны реорганизовать это. Похоже, что SomeGodClass.Qry был постоянным, но какая польза для него постоянна, если вы продолжаете переназначать его? Теперь вы никогда не знаете, какова его ценность, если только вы ее не перезаписали. Вместо этого используйте локальный переменный запрос и используйте параметры (например, Tigran)

Answer 3

Одна из основных проблем с использованием статического члена для кода, подобного этому, заключается в том, что на первый взгляд он выглядит хорошо (если немного избыточно) ,

SomeGodClass.Qry = /* Some Query */; 

int count = sqlHelper.ExecuteScalar(SomeGodClass.Qry); 

Он присваивает значение, а затем использует его, отлично. Проблема возникает, если вы начинаете вводить потоки в свое приложение. Внезапно статичность назначается и используется из нескольких потоков, и нет гарантии, что значение, используемое в Execute, является тем же самым, которое было назначено выше метода.

Насколько велика проблема это, очевидно, зависит от вашего приложения, если код-й в библиотеке и т.д. ...