patternjavascriptMinor
Caesar cipher implementation
Viewed 0 times
implementationcaesarcipher
Problem
I am not very familiar with JavaScript and I'am a little confused with the object oriented peculiarities of it. I am trying to create a Caesar shift using the concepts of objects, methods and properties in JavaScript. I am using an HTML form to take input from user and OnClick return the encoded cipher text to user. This is what I have. I'm pretty much sure about my logic but I guess my object creation and method calls fall through. Am I doing it right?
function Caesar(order){
this.order = order;
this.encode = encode;
function encode(input){
this.output = '';
for (var i = 0; i = 65 && this.c = 97 && this.c
Enter Plaintext :
Enter Shift: --How do I get the input from here and create my caesar object and pass the constructor the shift from this text box?
--How do I call the encode() method with input from the plaintext text box?
Solution
Your code is reviewable, even if it does not work, from a once over:
-
Indent your code (!!), this is bad:
This is better:
-
Indent your code (!!), this is bad:
function Caesar(order){
this.order = order;
this.encode = encode;
function encode(input){
this.output = '';
for (var i = 0; i = 65 && this.c = 97 && this.c <= 122) this.output += String.fromCharCode((this.c - 97 + this.key) % 26 + 97); // Lowercase
else this.output += input.charAt(i);
}
return answer.innerHTML= output;
}This is better:
function Caesar(order){
this.order = order;
this.encode = encode;
function encode(input){
this.output = '';
for (var i = 0; i = 65 && this.c = 97 && this.c <= 122)
// Lowercase
this.output += String.fromCharCode((this.c - 97 + this.key) % 26 + 97);
else
this.output += input.charAt(i);
}
return answer.innerHTML= output;
}Caesaris an unfortunate name
- It does not really say what it does ( one can guess )
- it should not start with a capital C, it is not a constructor
- The
Caesaris completely wrong
- it puts
encodeinthis.encodeand then never usesthis.encode
- if
encodewere called, it setthis.outputbut then in the end you putoutputinanswer.innerHTML
- you try to access
answer, but you never declared it withvar, much less pointed it to the answer element withgetElementById
- You hardcode magical numbers
- Some developers know
65isA, you should put a comment about that
- Same for
97,26and122
order-> what doesorderdo ? Should be commented, it is not obvious
onclick ="encode()"
Code Snippets
function Caesar(order){
this.order = order;
this.encode = encode;
function encode(input){
this.output = '';
for (var i = 0; i < input.length; i++) {
var this.c = input.charCodeAt(i);
if (this.c >= 65 && this.c <= 90) this.output += String.fromCharCode((this.c - 65 + this.order) % 26 + 65); // Uppercase
else if (this.c >= 97 && this.c <= 122) this.output += String.fromCharCode((this.c - 97 + this.key) % 26 + 97); // Lowercase
else this.output += input.charAt(i);
}
return answer.innerHTML= output;
}function Caesar(order){
this.order = order;
this.encode = encode;
function encode(input){
this.output = '';
for (var i = 0; i < input.length; i++) {
var this.c = input.charCodeAt(i);
if (this.c >= 65 && this.c <= 90)
// Uppercase
this.output += String.fromCharCode((this.c - 65 + this.order) % 26 + 65);
else if (this.c >= 97 && this.c <= 122)
// Lowercase
this.output += String.fromCharCode((this.c - 97 + this.key) % 26 + 97);
else
this.output += input.charAt(i);
}
return answer.innerHTML= output;
}Context
StackExchange Code Review Q#45609, answer score: 8
Revisions (0)
No revisions yet.