Javascript app review

So this is an app I'm working on, and I'd like to get some feedback on it. I'm leaving some key parts out, as I don't want everyone everywhere to have access to the complete code. The main pieces that I would like you guys to look at are still there though.

```
// Does a remote AJAX request that scrapes the URL and parses it
function doAjax(url) {
$.getJSON(URL,

function (data) {
if (data.results[0]) {
$("#content").html(""); // Reset
var number = $(filterData(data.results[0])).find("#gtv_leftcolumn table:gt(1)");
for (var i = 0; i ]*>/g, '');
data = data.replace(/[\r|\n]+/g, '');
data = data.replace(//g, '');
data = data.replace(/]>[\S\s]?/g, '');
data = data.replace(/]>[\S\s]?/g, '');
data = data.replace(//, '');
data = data.replace(/]*>/g, '');
return data;
}

// On Load
doAjax("http://www.codekraken.com/testing/snowday/wgrz.html");
$("#info").click(showInfo);
$(".info").click(closeInfo);
$("#reload").click(reaload);
$("#clearsearch").click(clearSearchBox);
$(".clear").click(clearFavorite);
setFavorite();

// You can clear the favorite item you set in setFavorite()
function clearFavorite() {
localStorage.removeItem("favorite");
localStorage.removeItem("favorite-status");
$(".star-inside").removeClass("favorite");
$(".clear span").text("");
}

// Clear search box
function clearSearchBox() {
$("#id_search").val("");
$('#id_search').trigger('keyup');
}

// Show info box
function showInfo() {
// Redacted
}

// Close info box
function closeInfo() {
// Redacted
}

// Reload AJAX request
function reload() {
closeInfo();
doAjax("URL");
}

// Set favorite item. This enables you to swipe on any .row element, and once it swipes, it sets the row you swipe on as the favorite. Swiping again unfavorites it. I mainly want help on this, as far as cleaning it up.
function setFavorite() {
var threshold = {
x: 30,
y: 10
};
var orig

Alright, figure I'll go ahead and say this right off the bat: I am no JS/JQuery guru. However, there were a few things that I saw that you might want to take a look at. This is by no means complete, but hopefully it will help.

You are violating the Arrow Anti-Pattern. This means your code is too heavily indented and should be refactored to remove some of that indentation. For example, the first if statement could be reversed and return early. This would then make an else statement unnecessary thus reducing a whole level of indentation from your entire function.

if( ! data.results[ 0 ] ) {
    console.log( 'error' );
    return;
}

//rest of code...

You can use empty() instead of explicitly clearing a field. This makes it a bit more obvious what you are trying to do wihtout needing comments everywhere trying to explain it.

$( '#content' ).empty();

Caching your selectors avoids having to compile them again and again, saving you processing power.

var $filterData = $( filterData( data.results[ 0 ] ) );
var number = $filterData.find( '#gtv_leftcolumn table:gt(1)' );

I don't know if this next suggestion will actually work, but it seems like it should. If it is possible to .find() from a .find(), in other words chaining finds, then I would try reusing your find results.

var name = number.find( ".maintext p:eq(" + i + ")" ).text();

I think you can get away with just loosley querying the length instead of explicitly. But that might just be preference.

if( ! number.length ) {

I don't like your filterData() function at all. First off, it seems like you should be able to refactor it to avoid redefining the same variable again and again. Second, I'm just not sure I see the point. To me it looks like this is just removing unwanted tags from something. No idea what, but it seems like you should be able to use a selector to chose what part of that document you want without having to regex it. Because I don't know exactly what you are trying to do here I can't make a good suggestion, sorry.

The following is violating the "Don't Repeat Yourself" (DRY) Principle. It should be refactored to avoid this. There may be other places that also violate this principle, but this is the first that popped out at me. The first thing that came to mind was to use a switch, but it quickly became apparent that once your abstract the first instance it really only had one other instance. This means you could have gotten away with a single if statement from the beginning. Another problem, not immediately obvious is that element is not always defined. Because you only define it in if statements there is a possibility that it may not be assigned and your script does not gracefully fail to compensate. Either declare a default value, or gracefully return to show an error has occurred. I went with the former.

//original code
if ($(event.target).attr("class") === "row-inside") {
    var element = $(event.target);
}
if ($(event.target).attr("class") === "row-l") {
    var element = $(event.target).parent();
}
if ($(event.target).attr("class") === "row-r") {
    var element = $(event.target).parent();
}

//refactored with default
//now with multiple uses
var element = $( event.target );
if( element.attr( 'class' ) !== 'row-inside' ) {
    element = element.parent();
}

I think its typically accepted that styling should only be done in CSS. I believe a better way would be to add and/or remove a class to get this same effect.

$(element).css("margin-left", "-75px");

Part of what I believe tucaz was saying was to do something like this:

$(document).on( {
    'touchmove' : function () {
        touchMove();
    },
    'touchstart' : function () {
        touchStart();
    }
}, '.row' );

There's more to his answer, I'm sure, this is just the only part I'm sure of. The above creates an event object that can easily be added to. It also has the added benefit of grouping related functionality. Something you might want to consider is adding namespaces to your events. They already seem to have a pseudo-namespace, but you might want to explicitly declare it. This helps differentiate your custom events from normal ones, while also allowing you to keep track of their purpose.

'touch:move' : function() {

This last section is going to be a general blanket statement. You have too few functions for too much functionality. This violates the Single Responsibility Principle. In other words, functions should only do one thing, everything else should be delegated to another function. The functions you do have are bulky and difficult to follow because of this. Adding more functions should help you to make your code more legible and will make it easier to extend this later. This is also key in ensuring your code does not violate DRY.

While I'm sure there are things I missed, I hope some of this helps.

StackExchange Code Review Q#17750, answer score: 5