0

Here's my code. It is a draft of a German Enigma machine. I'm trying to make the message that I will actually run through the machine except that it doesn't create that message until I run the function the second time. The weird thing is that I know the function runs because I see parts of it execute, but as far as the code is concerned toWorkWith is empty on the first run and filled on the second?

 

function encode(){
        var alphabet = ["A","B","C","D","E","F","G","H","I","J","K","L","M","N","O","P","Q","R","S","T","U","V","W","X","Y","Z"]
        var rotor_1 = {"A":["E"],"B":["K"],"C":["M"],"D":["F"],"E":["L"],"F":["G"],"G":["D"],"H":["Q"],"I":["V"],"J":["Z"],"K":["N"],"L":["T"],"M":["O"],"N":["W"],"O":["Y"],"P":["H"],"Q":["X"],"R":["U"],"S":["S"],"T":["P"],"U":["A"],"V":["I"],"W":["B"],"X":["R"],"Y":["C"],"Z":["J"]};
        var rotor_2 = {"A":["A"],"B":["J"],"C":["D"],"D":["K"],"E":["S"],"F":["I"],"G":["R"],"H":["U"],"I":["X"],"J":["B"],"K":["L"],"L":["H"],"M":["W"],"N":["T"],"O":["M"],"P":["C"],"Q":["Q"],"R":["G"],"S":["Z"],"T":["N"],"U":["P"],"V":["Y"],"W":["F"],"X":["V"],"Y":["O"],"Z":["E"]};
        var rotor_3 = {"A":["B"],"B":["D"],"C":["F"],"D":["H"],"E":["J"],"F":["L"],"G":["C"],"H":["P"],"I":["R"],"J":["T"],"K":["X"],"L":["V"],"M":["Z"],"N":["N"],"O":["Y"],"P":["E"],"Q":["I"],"R":["W"],"S":["G"],"T":["A"],"U":["K"],"V":["M"],"W":["U"],"X":["S"],"Y":["Q"],"Z":["O"]};
        var reflector = {"A":["A"],"B":["B"],"C":["C"],"D":["D"],"E":["E"],"F":["F"],"G":["G"],"H":["H"],"I":["I"],"J":["J"],"K":["K"],"L":["L"],"M":["M"],"N":["N"],"O":["O"],"P":["P"],"Q":["Q"],"R":["R"],"S":["S"],"T":["T"],"U":["U"],"V":["V"],"W":["W"],"X":["X"],"Y":["Y"],"Z":["Z"]};
        document.simulator.encoder.value.toUpperCase();
        var message = document.simulator.encoder.value.trim();
        message.toUpperCase();
        document.simulator.encoder.value = message.toUpperCase();
        var code = []
        //Turns the rotors
        function updateRotorState(rotorNum){
                var rotor1state = document.simulator.rotor1.value.toUpperCase();
                var rotor2state = document.simulator.rotor2.value.toUpperCase();
                var rotor3state = document.simulator.rotor3.value.toUpperCase();
                if(rotorNum == 1){
                        var rotorPos = alphabet.indexOf(rotor1state);
                       
                        var newPos = rotorPos + 1;
                       
                        if(rotor1state == "V"){
                                document.simulator.rotor1.value=alphabet[newPos]
                                updateRotorState(2);
                        }
                        if(rotorPos == 25){
                                newPos = 0;
                        }
                        document.simulator.rotor1.value = alphabet[newPos];
                }
                if(rotorNum == 2){
                        var rotorPos = alphabet.indexOf(rotor2state);
                        var newPos = rotorPos + 1;
                        if(rotor2state == "E"){
                                document.simulator.rotor2.value = alphabet[newPos];
                                updateRotorState(3);
                        }
                        if(rotorPos == 25){
                                newPos = 0;    
                        }
                        document.simulator.rotor2.value = alphabet[newPos];
                }
                if(rotorNum == 3){
                        var rotorPos = alphabet.indexOf(rotor3state);
                        var newPos = rotorPos + 1;
                        if(rotorPos == 25){
                                newPos = 0;    
                        }
                        document.simulator.rotor3.value = alphabet[newPos]
                        //Eventually need to add code to make next rotor turn over
                }
        }
        //Turns the message into a stripped output. Removes all non letter characters including spaces
        function workingMessageGen(message){
                var workingMessage = ""
                var messageArray = message.split('');
                for(var char in messageArray){
                        for(var letter in alphabet){
                                if(messageArray[char] == alphabet[letter]){
                                        workingMessage += alphabet[letter];
                                }
                        }
                }
                return workingMessage;
        }
        toWorkWith = workingMessageGen(message);
        for(var letter in message){
                updateRotorState(1);
        }
        document.simulator.decoder.value=toWorkWith;
}
Michael Mior
  • 28,107
  • 9
  • 89
  • 113
ChapmIndustries
  • 1,401
  • 4
  • 17
  • 19
  • Missing `;` after `var code = []` and after `document.simulator.rotor3.value = alphabet[newPos]` and `workingMessage = ""` – Alex W Jan 16 '13 at 04:38
  • @AlexW good to know but that would not cause the current problem. Any other ideas? – ChapmIndustries Jan 16 '13 at 04:39
  • @ChapmIndustries It's a lot easier to find out what's wrong if your code is nice and robust. Sometimes a simple semicolon, comma, etc can be the wrench in the Javascript engine. I would suggest matching up your brackets as well and seeing what output you're getting on the Javascript console. – Alex W Jan 16 '13 at 04:41
  • @AlexW do I have mismatched brackets? Usually dreamweaver freaks out at me for that. – ChapmIndustries Jan 16 '13 at 04:45
  • My guess is that you're trying to read the value of `toWorkWith` before you run the function `encode()` – Alex W Jan 16 '13 at 04:49
  • @Mogsdad that's document as in the html document. And message is defined on line 8... – ChapmIndustries Jan 16 '13 at 04:52
  • @ChapmIndustries See below. You're missing a `var` in front of `toWorkWith` – Alex W Jan 16 '13 at 04:54

1 Answers1

0

I'm not sure what you expect to happen, but there are some flaws in the code:

  • JavaScript string values are immutable - they are no objects. Calling functions like toUpperCase will not change the variables, but return the new value. document.simulator.encoder.value.toUpperCase() and message.toUpperCase() are useless therefore.
  • Missing semicolon after var code = [] (though it should not matter, you can never be sure)
  • toWorkWith is missing a var declaration - seems not to be intended
  • for(var char in messageArray) - never enumerate array properties! Use a for loop to iterate its indizes (see Why is using "for...in" with array iteration a bad idea?)
  • for(var letter in message) - since message is a string, do not try to enumerate its properties! a) does this not work in IE b) you might catch enumerable properties on String.prototype. Instead, use a normal for loop and message.length.
  • All over your script, you are mixing code logic and DOM accessing. Do you use the DOM to retrieve, show, store values? Do it in 3 steps: read input from DOM, execute logic, write output.
Community
  • 1
  • 1
Bergi
  • 630,263
  • 148
  • 957
  • 1,375
  • The third bullet is probably the problem – Alex W Jan 16 '13 at 04:51
  • I didn't post the html that goes with this but yes I store variables in the html form. It's an interactive thing so that way the user can see things change before their eyes. – ChapmIndustries Jan 16 '13 at 04:55
  • Sure, but still that does not mean that you should mix logic with representation. Use the MVC pattern. – Bergi Jan 16 '13 at 04:56
  • The code is supposed to mimic a physical machine so it is kind of supposed to do that. Can you explain why I shouldn't? – ChapmIndustries Jan 16 '13 at 04:58
  • Fixed bullet 3 and still have the same problem. – ChapmIndustries Jan 16 '13 at 04:59
  • You can be sure when it comes to missing semicolons. The rules for this are well-defined. Although I agree that you're better off just putting in semicolons to avoid worrying about this :) – Michael Mior Jan 16 '13 at 05:11
  • Small thing... in loops e.g. in `workingMessageGen()` should `break` after finding match. Those lookups would probably be more efficient and readable using String `index()` and `charAt()` operations. – Mogsdad Jan 16 '13 at 05:12
  • Better yet, one-liner to replace whole function: `toWorkWith = message.replace(/[^A-Z]/g, "");` – Mogsdad Jan 16 '13 at 05:23