HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMinor

Small jQuery mobile plugin to handle touch events

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
eventshandlepluginjquerysmallmobiletouch

Problem

Why this plugin:

I am developing a mobile app - at some point, I felt like this would be a good idea to give the users the possibility to control everything in the app with touch gestures, hence the need for a plugin able to recognize more than the basic swipe events.

What it does:

When the user starts touching the screen and moving his finger (or mouse on desktop btw), the code tries to identify his move as one of the eight cardinal directions (North, South, West, East, NorthWest, SouthWest, SouthEast, NorthEast). If he changes direction, then the next direction he takes is added to the list to produce an output like : North->East->South...

Why I am posting here:

This code is going to be run quite a lot of times in the app (every time the user moves his finger on the screen), and I would love to be sure that it is not going to pose performance issues, or if I overlooked some best practices I am not aware of, and of course, any constructive feedback is very welcome.

The part of the code to attach the event and handle touchStart and touchStop is not mine, it is taken from here, but I am including it for plugin completeness (also modified some very minor things).

The code:

```
// jquerymobile-unicorn_swipe
// ----------------------------------
// Copyright (c)2012 Donnovan Lewis
// Copyright (c)2014 Romain Le Bail
// Distributed under MIT license (http://opensource.org/licenses/MIT)
//
//credits to Donnovan Lewis for the material taken from https://github.com/blackdynamo/jquerymobile-swipeupdown

(function () {
// initializes touch events
var supportTouch = $.support.touch,
touchStartEvent = supportTouch ? "touchstart" : "mousedown",
touchStopEvent = supportTouch ? "touchend" : "mouseup",
touchMoveEvent = supportTouch ? "touchmove" : "mousemove";
$.event.special.unicorn = {
setup: function () {
var thisObject = this;
var $this = $(thisObject);

$this.bind(touchStartEvent, function (event) {

Solution

Interesting plugin,

some observations:

  • Use JsHint, there are a ton of minor issues with this code that you should clean up



  • Use JsBeautifier, there is quite a bit of inconsistent formatting



  • 'unicorn' sounds like fun, 'typezor' not so much



  • It seems this code does not handle simultaneous touch events ?



-
From a style perspective,

-
Too much horizontal stretching, why ?

var path = []; var derived_path = []; var segs = [[]]; var seg_types =[[]]; var max_freqs = [[]];


  • Naming is often too cryptic, plus JavaScript should use lowerCamelCase ( max_count -> maxCount )



  • Nested ternary operators are too hackish



  • if(i!=0){fire=true;}; could be fire = fire || !i; though I might have considered that too hackish if you had actually written that ;)



  • Commenting : simply stuff is commented like / starting point / but the harder part ( for me ) where you derive a direction does not have enough comments



All in all, I think this code needs some more polishing before I would use it in a project.

Code Snippets

var path = []; var derived_path = []; var segs = [[]]; var seg_types =[[]]; var max_freqs = [[]];

Context

StackExchange Code Review Q#55979, answer score: 3

Revisions (0)

No revisions yet.