MyDictionary Web Application

auburnhairΛογισμικό & κατασκευή λογ/κού

13 Δεκ 2013 (πριν από 3 χρόνια και 6 μήνες)

73 εμφανίσεις

MyDictionary
W
eb
A
pplication


We have a simple 3
-
tier web application that lets users search a dictionary and see
corresponding definitions, as well as see the most popular search terms.


A colleague of yours has implemented the application (see code
below) and has asked you for a
thorough code/quality review.



Architecture / Requirements



The basic components of the MyDictionary website are illustrated below. Orange arrows show
desired user flow (visit home page, see popular searches, enter search
term, press Search, and
view results). Assume that the View components are wireframes given to you by your project
manager to serve as lightweight requirements for how the application should look and behave.







Home.html

(HTML template file; assume %
some_variable% blocks refer to injected view data)



<html>

<body>

<h1>MyDictionary<h1>

<form action="
On_Search
" method="
POST
">

<p>Find definition:</p>

<input type="textbox" name="
word
" />

<input type="button" value="Search" />

</form>

<div class=”popular
”>

<p>Most popular searches:</p>

<ul>

%mostPopular%

</ul>

</body>

</html>



Results.html
(HTML template file)



<html>

<body>

<h1>MyDictonary<h1>

<p>Search results for “<strong>
%word%
</strong>”:

<ul>

%definitions_list%

</ul>

</body>

</html>





Server
Handler For Home Page

(C#; assume all Utils.* calls magically work)



function OnLoad()

{

// Find the most popular words (judged by most searches). Example result:

// Word





T
otal

//
---------------

-----

// cool





2

// chillax





250

// confuzzled



125

// hippopotomus



200

// ...







...



string q = "SELECT
Word
, COUNT(*) AS
Total
FROM
WordLookups
GROUP BY
Word
";

// Run the query to get popular words; gets array of rows, each
of which

// contain an array of column values from DB result set

// (e.g. results[1][0] = ‘chillax’, results[2][1] = ‘125’, etc)

string[][] results = Utils.RunSqlQuery(q);

// This call magically sorts by most popular (e.g. ‘chillax’ now first, etc)

Utils.SortByBiggestTotal(results);


// Create HTML for the 3 most popular words list

string pop = "";

for (
int i = 0; i <= 3; i++) {

// Add the popular word to list

pop += "<li style='font
-
weight:bold;'>" + results[i][0] + "</li>";

}


// Render the home page

var view =
Utils.GetView("/Views/Home.html");

view["mostPopular"] = pop;

Utils.RenderPage(view);

}





Since we're only concerned of getting the top 3 popular searches (although this can be configurable)
, I
would have to re
-
factor the SQL string to something like this...
(note: COUNT(1
) is faster than COUNT(*)
in my experience using ORACLE)


SELECT

TOP 3 Word, COUNT(1) as Total FROM WordLookups GROUP BY WORD ORDER BY
Total DESC;


This call is no longer needed or not necessary as the
result is already
sorted....Also, the result is also limited
to however you want it, in this case 3

-

this will also
improve performance.


jus
t get the length or count of the "results" since it's
zero based ... it would be something like this....

for (int i = 0; i < results.length; i++)


Add the following string

to match up the UI
design requirement
.....(asterisk)

" * "

Server Handler for Search Result Page
(C#; assume all Utils.* calls magically work)



function OnSearch()

{

// Read the word the user wants

to look up

string word = Utils.GetFromQueryString("word");


// Look up definitions for this word in the database dictionary

string q = “SELECT FROM Definitions WHERE Word = '" + word + "'";

string[] defs =
Utils.RunSqlQuery(q);


// Record this lookup in the DB to factor it into the most popular list

Utils.RunSqlQuery("INSERT INTO WordLookups (Word, LookupTime)

VALUES ('" + word + "', '" + DateTime.Now + "');");


string definitionsList = "";

for (int i = 0, j

= defs.Length; i < j; i++) {

definitionsList += "<li>" + defs[i] + </li>";

}


// Put together the result page and inject view data to show the user

// their original word and its
definitions

var view = Utils.GetView("Views/Results.html");

view["word"] = word;

view["definitionsList"] = definitionsList;

Utils.RenderPage(view);

}




Questions



After reviewing your co
-
worker’s code, what feedback/advice would you have for them?

ANS:

See my yellow callout box. Pretty much I would advice to follow best practices in
coding. Always think of how to avoid SQL injection by using parameterized queries or
stored procedures



Are there any obvious defects in the code you reviewed? Any performanc
e issues?

ANS:

See my yellow callout box. Pretty much I would advice to follow best practices in
coding. Always think of how to avoid SQL injection by using parameterized queries or
stored procedures
.



If you were a malicious hacker determined to attack the MyDictionary website, is it
possible to do so if your co
-
worker’s code was deployed to production?

ANS:

YES! Big time. I describe the code as "SQL INJECTION" friendly.



If you were in charge of designi
ng and implementing this simple application from scratch,
what (if anything) would you do differently?

This is prone to SQL injection (big time). It's always best to use parameterized query or better yet use
Stored Procedure.

Another thing, is the SELECT statement

itself is wrong syntactically and logically as
well.

Parameterized Query something like this.... SqlCommand objCommand = new SqlCommand("SELECT
Word, Definition FROM Words WHERE Word = @word, connection);

objCommand.Parameters.Add("@word",
word
);

So in th
is case since the actual code is encapsulated in the Utils, I would have to either add a new
'overloaded' method RunSqlQuery that will accept 2 string parameters, 1st is the actual string query
and the 2nd one is the parameter that is used in the WHERE cla
use in this case, 'word'.

Assu
ming Utils has as a way to run/execute Stored Procedure. I would recommend using it.

same here.... see my comment above

Add the
line item number

to match up with
the UI requirement.
Something like this.....

int j = 0; for (int i=0, i < defs.Length; i++) {
definitionsList += string.Format("<li{0}.
{1}></li>", ++j, defs[i]); }

(note: if ther
e's no match found, a text
message should be displayed.. something
like " no definitions found...")

ANS:

From the UI tier's perspective, I prefer files with an extension of .ASPX. Also I
would make the web project ASP.NET MVC. Also I would decouple all
the
database
operations

(e.g. select, update, insert, etc.) and better yet if I want to be more elaborate,
I would make use of the Entity Framework that .NET 3.5/4.0. Of course all these have to
placed in the middle
-
tier.




Q
uality Assurance
Questions



Describe your strategy to create a test suite around this application.

ANS:

Visual Studio allows you to create a test project to your existing web app. I'll make
use of this great feature as this is already part and integrated in VS.



We learn that our bare
ly
-
used dictionar
y app will soon be published in
TechCrunch,
which will likely lead to 100,000 new visitors in a short time. Describe your test strategy
to make sure that our app can handle this increased load.

ANS:

First of all assuming everything works f
ine and everything is in place

and in order
.
My strategy for
performance and load testing would be the following:

1. I would have to have a STAGING environment setup
. Ideally the STAGING
environment has to mimic the actual PRODUCTION but in most cases this

is not
possible for a number of reasons (e.g. money :))

2. I would have to coordinate with a number of users to help in the
load/stress/performance
testing if possible.

3. In addition to the actual
"concurrent"
users, I would setup in the STAGING web
server
a stress/load testing tool (bet it Microsoft or 3rd party for as long as it meets my
requirement) and configure it in such a way to simulate 100,000 concurrent users. And
then monitor everything from the web server to the database server (if hosted
in a
different machine) and checks where the bottlenecks are if there are any.

4. If there are issues found during the load/stress testing. Figure out what causes them
and address them accordingly.
Issues could come from different sources, the code itself
or from the hardware.
If it is coming from code
then it's a sign to rethink and refactor
code
. If it is coming from the hardware one obvious thing to do is to upgrade the
hardware. If this is not possible, maybe it's better to just take it easy on the numb
er of
concurrent users. I'll probably lower it down to half (50,000) and so on and so forth....