4
\$\begingroup\$

I am a backbone.js newbie. I have created a color picker application using backbone.js. I am trying to figure out if I have nailed the MVC concept. Please review my code and let me know if I can do better.

<!DOCTYPE html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8" />
<title>MVC Design - Color Picker</title>
<script src="https://ajax.googleapis.com/ajax/libs/jquery/1.6.1/jquery.min.js"></script>
<script src="http://ajax.cdnjs.com/ajax/libs/underscore.js/1.1.6/underscore-min.js"></script>
<script src="http://ajax.cdnjs.com/ajax/libs/backbone.js/0.3.3/backbone-min.js"></script>
<script src="http://ajax.cdnjs.com/ajax/libs/json2/20110223/json2.js"></script>
<script>
 $(function(){
 //Model for RGB
 var RGB = Backbone.Model.extend({
 defaults: {R: 255, G: 255, B: 255}
 });
 //End of Model for RGB
 //Model for HEX
 var HEX = Backbone.Model.extend({
 defaults: {HEX: "000000"}
 });
 //End of Model for HEX
 //Model for HSV
 var HSV = Backbone.Model.extend({
 defaults: {H: 0, S: 0, V: 1}
 });
 //End of Model for HSV
 //Model for CMYK 
 var CMYK = Backbone.Model.extend({defaults: {C: 0, M: 0, Y: 0, K:0}});
 //End of Model for CMYK
 //Begin RGB View 
 RView = Backbone.View.extend({
 id: "RGB",
 view: {}, 
 initialize: function(){
 this.render();
 },
 render: function(){
 $("#R").val(rgb.get("R"));
 $("#G").val(rgb.get("G"));
 $("#B").val(rgb.get("B"));
 },
 events: {
 "change" : "globalChange"
 },
 globalChange: function(){
 //set all the textbox values
 set();
 var res_hsv = rgb2hsv(rgb.get("R"),rgb.get("G"),rgb.get("B"));
 var res_cmyk = rgb2cmyk (rgb.get("R"),rgb.get("G"),rgb.get("B")); 
 $("#R").val(rgb.get("R"));
 $("#G").val(rgb.get("G"));
 $("#B").val(rgb.get("B"));
 $("#H").val(res_hsv[0]);
 $("#S").val(res_hsv[1]);
 $("#V").val(res_hsv[2]);
 $("#C").val(res_cmyk[0]);
 $("#M").val(res_cmyk[1]);
 $("#Y").val(res_cmyk[2]);
 $("#K").val(res_cmyk[3]);
 $('#HEX').css({'background-color' : '#'+rgbToHex(rgb.get("R"),rgb.get("G"),rgb.get("B"))});
 //set all the textbox values
 set();
 //alert(rgb2cmyk (rgb.get("R"),rgb.get("G"),rgb.get("B")));
 //alert(rgb2hsv(rgb.get("R"),rgb.get("G"),rgb.get("B")));
 //alert("#"+rgbToHex(rgb.get("R"),rgb.get("G"),rgb.get("B")));
 //alert(rgb.get("R")+", "+rgb.get("G")+", "+rgb.get("B"));
 }
 });
 //End RGB View 
 //Begin HSV View
 HSV_View = Backbone.View.extend({
 id: "HSV",
 view: {},
 initialize: function(){
 this.render();
 },
 render: function(){
 $("#H").val(hsv.get("H"));
 $("#S").val(hsv.get("S"));
 $("#V").val(hsv.get("V"));
 },
 events: {
 "change" : "globalChange"
 },
 globalChange: function(){
 //set all the textbox values
 set();
 var res_rgb = hsv2rgb(hsv.get("H"), hsv.get("S"), hsv.get("V"));
 var res_cmyk = rgb2cmyk(res_rgb[0],res_rgb[1],res_rgb[2]); 
 $("#R").val(res_rgb[0]);
 $("#G").val(res_rgb[1]);
 $("#B").val(res_rgb[2]);
 $("#H").val(hsv.get("H"));
 $("#S").val(hsv.get("S"));
 $("#V").val(hsv.get("V"));
 $("#C").val(res_cmyk[0]);
 $("#M").val(res_cmyk[1]);
 $("#Y").val(res_cmyk[2]);
 $("#K").val(res_cmyk[3]);
 $('#HEX').css({'background-color' : '#'+rgbToHex(res_rgb[0],res_rgb[1],res_rgb[2])});
 //set all the textbox values
 set();
 // alert(hsv2rgb(hsv.get("H"), hsv.get("S"), hsv.get("V")));
 }
 }); 
 //End HSV View
 // Begin CMYK View
 CMYK_View = Backbone.View.extend({
 id: "CMYK",
 view: {},
 initialize: function(){
 this.render();
 },
 render: function(){
 $("#C").val(cmyk.get("C"));
 $("#M").val(cmyk.get("M"));
 $("#Y").val(cmyk.get("Y"));
 $("#K").val(cmyk.get("K"));
 },
 events: {
 "change" : "globalChange"
 },
 globalChange: function(){
 //set all the textbox values
 set();
 var res_rgb = cmyk2rgb(cmyk.get("C"), cmyk.get("M"), cmyk.get("Y"), cmyk.get("K"));
 var res_hsv = rgb2hsv(res_rgb[0], res_rgb[1], res_rgb[2]);
 $("#R").val(res_rgb[0]);
 $("#G").val(res_rgb[1]);
 $("#B").val(res_rgb[2]);
 $("#H").val(res_hsv[0]);
 $("#S").val(res_hsv[1]);
 $("#V").val(res_hsv[2]);
 $("#C").val(cmyk.get("C"));
 $("#M").val(cmyk.get("M"));
 $("#Y").val(cmyk.get("Y"));
 $("#K").val(cmyk.get("K"));
 $('#HEX').css({'background-color' : '#'+rgbToHex(res_rgb[0],res_rgb[1],res_rgb[2])});
 //set all the textbox values
 set();
 //alert("sending"+" "+cmyk.get("C")+" "+cmyk.get("M")+" "+cmyk.get("Y")+" "+cmyk.get("K"));
 //alert(cmyk2rgb(cmyk.get("C"), cmyk.get("M"), cmyk.get("Y"), cmyk.get("K"))); 
 }
 });
 // End CMYK View 
 // Models
 var rgb = new RGB();
 var hex = new HEX();
 var hsv = new HSV();
 var cmyk = new CMYK();
 // End of Models
 // Views
 var rgb_view = new RView({ el: $("#RGB") }); 
 var cmyk_view = new CMYK_View({ el: $("#CMYK") }); 
 var hsv_view = new HSV_View({ el: $("#HSV") }); 
 //End of Views
 function set(){
 rgb.set({ R: $("#R").val() });
 rgb.set({ G: $("#G").val() });
 rgb.set({ B: $("#B").val() });
 hsv.set({ H: $("#H").val() });
 hsv.set({ S: $("#S").val() });
 hsv.set({ V: $("#V").val() });
 cmyk.set({ C: $("#C").val() });
 cmyk.set({ M: $("#M").val() });
 cmyk.set({ Y: $("#Y").val() });
 cmyk.set({ K: $("#K").val() });
 }
 //CMYK to RGB
 function cmyk2rgb(c, m, y, k){
 var computedR = 0;
 var computedG = 0;
 var computedB = 0;
 c = c / 100;
 m = m / 100;
 y = y / 100;
 k = k / 100;
 computedR = 1 - Math.min( 1, c * ( 1 - k ) + k );
 computedG = 1 - Math.min( 1, m * ( 1 - k ) + k );
 computedB = 1 - Math.min( 1, y * ( 1 - k ) + k );
 computedR = Math.round( computedR * 255 );
 computedG = Math.round( computedG * 255 );
 computedB = Math.round( computedB * 255 );
 return [computedR,computedG,computedB];
 }
 // End of CMYK to RGB
 //HSV to RGB
 function hsv2rgb(h,s,v){
 var computedR = 0;
 var computedG = 0;
 var computedB = 0;
 h = h / 360;
 s = s / 100;
 v = v / 100;
 if (s == 0) {
 computedR = v * 255;
 computedG = v * 255;
 computedB = v * 255;
 }
 else {
 var_h = h * 6;
 var_i = Math.floor(var_h);
 var_1 = v * (1 - s);
 var_2 = v * (1 - s * (var_h - var_i));
 var_3 = v * (1 - s * (1 - (var_h - var_i)));
 if (var_i == 0) {var_r = v; var_g = var_3; var_b = var_1}
 else if (var_i == 1) {var_r = var_2; var_g = v; var_b = var_1}
 else if (var_i == 2) {var_r = var_1; var_g = v; var_b = var_3}
 else if (var_i == 3) {var_r = var_1; var_g = var_2; var_b = v}
 else if (var_i == 4) {var_r = var_3; var_g = var_1; var_b = v}
 else {var_r = v; var_g = var_1; var_b = var_2};
 computedR = var_r * 255;
 computedG = var_g * 255;
 computedB = var_b * 255;
 computedR = Math.round(computedR);
 computedG = Math.round(computedG);
 computedB = Math.round(computedB);
 } 
 return [computedR,computedG,computedB];
 }
 // End of HSV to RGB
 //RGB to CMYK
 function rgb2cmyk (r,g,b) {
 var computedC = 0;
 var computedM = 0;
 var computedY = 0;
 var computedK = 0;
 //remove spaces from input RGB values, convert to int
 var r = parseInt( (''+r).replace(/\s/g,''),10 ); 
 var g = parseInt( (''+g).replace(/\s/g,''),10 ); 
 var b = parseInt( (''+b).replace(/\s/g,''),10 ); 
 if ( r==null || g==null || b==null ||
 isNaN(r) || isNaN(g)|| isNaN(b) )
 {
 alert ('Please enter numeric RGB values!');
 return;
 }
 if (r<0 || g<0 || b<0 || r>255 || g>255 || b>255) {
 alert ('RGB values must be in the range 0 to 255.');
 return;
 }
 // BLACK
 if (r==0 && g==0 && b==0) {
 computedK = 1;
 return [0,0,0,1];
 }
 computedC = 1 - (r/255);
 computedM = 1 - (g/255);
 computedY = 1 - (b/255);
 var minCMY = Math.min(computedC,
 Math.min(computedM,computedY));
 computedC = (computedC - minCMY) / (1 - minCMY) ;
 computedM = (computedM - minCMY) / (1 - minCMY) ;
 computedY = (computedY - minCMY) / (1 - minCMY) ;
 computedK = minCMY;
 return [computedC,computedM,computedY,computedK];
 }
 //End of RGB to CMYK
 //RGB to HSV
 function rgb2hsv (r,g,b) {
 var computedH = 0;
 var computedS = 0;
 var computedV = 0;
 //remove spaces from input RGB values, convert to int
 var r = parseInt( (''+r).replace(/\s/g,''),10 ); 
 var g = parseInt( (''+g).replace(/\s/g,''),10 ); 
 var b = parseInt( (''+b).replace(/\s/g,''),10 ); 
 if ( r==null || g==null || b==null ||
 isNaN(r) || isNaN(g)|| isNaN(b) ) {
 alert ('Please enter numeric RGB values!');
 return;
 }
 if (r<0 || g<0 || b<0 || r>255 || g>255 || b>255) {
 alert ('RGB values must be in the range 0 to 255.');
 return;
 }
 r=r/255; g=g/255; b=b/255;
 var minRGB = Math.min(r,Math.min(g,b));
 var maxRGB = Math.max(r,Math.max(g,b));
 // Black-gray-white
 if (minRGB==maxRGB) {
 computedV = minRGB;
 return [0,0,computedV];
 }
 // Colors other than black-gray-white:
 var d = (r==minRGB) ? g-b : ((b==minRGB) ? r-g : b-r);
 var h = (r==minRGB) ? 3 : ((b==minRGB) ? 1 : 5);
 computedH = 60*(h - d/(maxRGB - minRGB));
 computedS = (maxRGB - minRGB)/maxRGB;
 computedV = maxRGB;
 return [computedH,computedS,computedV];
 }
 // End of RGB to HSV
 //RGB to HEX
 function rgbToHex(R,G,B) {return toHex(R)+toHex(G)+toHex(B);}
 function toHex(n) {
 n = parseInt(n,10);
 if (isNaN(n)) return "00";
 n = Math.max(0,Math.min(n,255));
 return "0123456789ABCDEF".charAt((n-n%16)/16)
 + "0123456789ABCDEF".charAt(n%16);
 }
 // End of RGB to HEX
 });
 </script>
 </head>
 <body>
 <div id="RGB">
 <table>
 <tr><td>R</td><td><input type="text" id="R" /></td><td>G</td><td><input type="text" id="G" /></td><td>B</td><td><input type="text" id="B" /></td></tr>
 </table>
 </div>
 <div id="HSV">
 <table>
 <tr><td>H</td><td><input type="text" id="H" /></td><td>S</td><td><input type="text" id="S" /></td><td>V</td><td><input type="text" id="V" /></td></tr>
 </table>
 </div>
 <div id="CMYK">
 <table>
 <tr><td>C</td><td><input type="text" id="C" /></td><td>M</td><td><input type="text" id="M" /></td><td>Y</td><td><input type="text" id="Y" /></td><td>K</td><td><input type="text" id="K" /></td></tr>
 </table>
 </div>
 <canvas id="HEX" width="50" height="50" style=" border: 1px solid wheat; background-color: #fff;">
 Your browser does not support the HTML5 canvas tag.
 </canvas>
 </body>
 </html>
Jamal
35.2k13 gold badges134 silver badges238 bronze badges
asked May 8, 2013 at 17:57
\$\endgroup\$
1
  • \$\begingroup\$ I'd recommend you checkout some Backbone data binding library such as Thorax, Epoxy or stick-it \$\endgroup\$ Commented Aug 2, 2014 at 15:17

1 Answer 1

2
\$\begingroup\$

Interesting question,

MVC

It seems, you did not nail the MVC concept perfectly.

In essence, you should have 1 single render routine for both RGB, HSV and CMYK since all these are just outputs of the same value from the model. It does not make sense to split that out. ( If you do this exercise you will notice the removal of a ton of copy pasted code).

Readability

Your indenting is terrible, please use a tool like jsbeautifier

Inconsistencies

  • You are using 2 ways to store RGB values:

    {R: 255, G: 255, B: 255} and [computedR,computedG,computedB]

    sticking to one convention would be far more readable ( I would go for the first convention )

  • You should split out the check for valid values of r, g and b these validity checks do not belong in conversion routines (plus, they are copy pasted right now)

JsHint

  • You are missing a ton of semicolons
  • In function rgb2cmyk (r,g,b) { you do not need to use var r since r is already known ( as a parameter ).

Naming

  • This, is terrible as well:

    var_h = h * 6;
var_i = Math.floor(var_h);
var_1 = v * (1 - s);
var_2 = v * (1 - s * (var_h - var_i));
var_3 = v * (1 - s * (1 - (var_h - var_i)));

    Do not prefix your variables with var_, it adds no value at all and hinders readability. Also variables 1 through 3 are terrible variable names.

  • Similarly, this

    var computedR = 0;
var computedG = 0;
var computedB = 0;

    has the prefix of computed which also adds very little (there is plenty of context to tell the reader that these values are computed.) I would have gone for 

    var red, blue, green;

    since you do nothing with the 0 value anyway.

answered Aug 5, 2014 at 19:58
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.