0
\$\begingroup\$

This isn't so much, HOW do I make it work, as this code works, rather how could I make it better. The way this code works is it guides a person to the correct answer by getting "Yes", "No" or "I don't know" (in some cases from the person).

It's intended to run in HTML5/JavaScript (kind of like an online interactive guide). This code actually works, but it uses the JavaScript "prompt" for the input, and frankly, it's pretty ugly. Any suggestions on how to make it either look or work better?

First the JavaScript, as it's the main engine:

function mc1() {
var n = prompt("Do you have Medicare benefits (Y=Yes, N=No, I=I don't know)", " ");


if( n == "Y"){
   mcs2();
}
else if (n="N"){
    mcs1a();
   }

else if (n="I"){
   mcs1b();
 }

else {
alert("Enter a valid choice Y,N or I!");
mc1();
} 
}


function mcs1a(){
var n1=prompt("Have you applied and been denied Medicare Coverage (Y= Yes,  N= No,  I = I don't know)", "");
if (n1=="Y"){
mc3a();
}
else if (n1=="N"){
mc2b();
}
else if (n1=="I"){
mc2b();
}
else {
alert("Enter a valid choice:  Y, N or I!");
}
}


function mcs1b(){
alert("Refer to Social Security Administration to check status, (CODE E01M1)!");
}

function mcs2(){
var n2=prompt("Do you have Medicare Part A AND B ( Y=Yes, N=No, I=I don't know)","");
if (n2=="Y"){
mcs3();
}
if (n2=="N"){
alert("Refer to Social Security to apply for the other part (Code E01M1)!");
}
if (n2=="I"){
alert("Refer to Social Security Administration to check status (Code E01M1)!");
}
else {
alert("Enter a valid choice: Y, N, or I!");
}
}

function mcs3(){
var n3=prompt("Would you like to apply for HF MEdicare (Y=Yes, N=No)","");
if (n3=="Y"){
mcs4a();
}
if (n3=="N"){
mcs4();
}
else {
alert("Enter a valid choice: Y or N!");
}
}

function mcs4(){
var n4=prompt("Do you have a caretaker relative who claims you on their taxes (Y=Yes, N=No)","");
if (n4=="Y"){
alert("Follow standard prequalification with caretaker relative as the income");
}
if (n4=="N"){
alert("Refer to the HRA/LDSS/SLDSS to apply for benefits (CODE E01H)");
}
else {
alert("Enter a valid choice: Y or N!");
}
}

function mcs4a(){
alert("Warm transfer to Medicare Advertising to apply for HealthFirst Medicare (Code WTS)");}

mc1();
<html>
<head />
<body> 
<script src="testdrop.js"></script>

</body>
</html>

\$\endgroup\$
2
  • 3
    \$\begingroup\$ "message": "Uncaught ReferenceError: mc2b is not defined", \$\endgroup\$
    – hjpotter92
    Dec 15, 2018 at 22:46
  • \$\begingroup\$ mc3a is also undefined. \$\endgroup\$
    – Graipher
    Dec 17, 2018 at 11:20

2 Answers 2

1
\$\begingroup\$

Bad names!!

  • Very poor naming, so much so that your code is almost unreadable and definitely unusable.
  • Use constants for variables that do not change. eg var n1= should be const n =
  • Always! indent your code.
  • Spaces between operators var n1=prompt( should be var n1 = prompt(
  • else on same line as closing } eg } else if (
  • Use === rather than == and !== rather than !=
  • The code is broken and throws errors because you have missing function calls. Next time you post on code review please make sure that the code works, or clearly mark code stubs.
  • A lot of repeated code could be put together as a function.

eg Ask question function

function askQuestion(message, yesCall, noCall, unsureCall) {
    const result = prompt(message, "").toLowerCase();
    if ( result === "y") { yesCall() }
    else if (result === "n") { noCall() }
    else if (result === "i") { unsureCall() }
    else { askQuestion(message, yesCall, noCall, unsureCall) }
}

Then you can ask the question

function mcs1() {
    askQuestion(
        "Do you have Medicare benefits (Y=Yes, N=No, I=I don't know)",
         mcs2, mcs1a, mcs1b
    );
}

User friendly

When creating interfaces you should never have ask the user to correct themselves due to improper use of the interface.

If you ask for a Y or N and you can not control the keys, don't respond with "Enter a valid choice Y,N or I!" as it is a pointless and frustrating step for the user. Just ask the question again. "Try again... Do you have Medicare benefits? Y/N"

Best is to ensure that the user can only make correct selections.

Use String.toLowerCase() or String.toUpperCase() to eliminate the need to differentiate between upper and lower case input.

Use a dialog with clickable options, or vet the key inputs with a keyboard event listener (not available on prompt)

Alert and Prompt are evil

Don't use prompts or alerts. Use the HTML5 element dialog (or use standard HTML) and create custom dialogs.

Custom dialogs are non blocking and let you control the inputs, style, and much more. Giving a much better feel and flow to the interrogation.

See example how to use <dialog> element. (Note: full support for <dialog> element is only on Chrome for the moment. A polyfill can be found on MDN dialog reference page.)

Separate questions from code.

You have coded the questions into the source such that if you make a change to the questions you will need to make changes to the source code.

These types of apps (survey or interrogations) are generally a class of finite state machine. The state (a question or message) will always follow a previous known state, and the following state will depend on the current answer.

Seperate the state flow from the logic in such a way that you do not need to code the questions. You should be able to present any list of questions as a dataset (eg JSON) without needing to change any of the source code.

If the data structure is well defined, it also allows for non coders to create and change the interrogation.

An example.

The example may be a little advanced but have a look anyways, take what you can and feel free to ask questions in the comments.

  • It uses the dialog element to create the dialogs.
  • Async functions and promises to wait for user input.
  • Event listeners to get user clicks.
  • Defines a state machine via the object interrogation that uses named properties to connect states. The function interrogate takes a state name and use it to determine how to present and act on that state.

setTimeout(()=>interrogate("start"),0);

/* Structure of interrogation All names are case sensitive [] means optional
  name: { // state name
    [action]: string, // one of two actions "reset" or "showResult"
    [question]: string,  //  The question if included the must have Y,N and/or [I]
    [Y]: string  // name of state for yes result
    [N]: string  // name of state for no result
    [I]: string  // name of state for unsure result
    [message]: string, // Message to display. This is only optional if question is defined
    [next]: string // name of next state. If omitted then defaults to "exit"
  }
*/

const interrogation = {
    results: "",
    start: {
        action: "reset",
        question: "Do you have Medicare benefits?",
        Y: "partsAB",
        N: "deniedMedicare",
        I: "refSocialSecurityAdministration"
    },
    partsAB: {
        question: "Do you have Medicare Part A AND B",
        Y: "applyForHF",
        N: "unknown",
        I: "unknown"      
    },
    applyForHF: {
        question: "Would you like to apply for HF MEdicare",
        Y: "applyForHFInfo",
        N: "haveCaretaker"
    },
    haveCaretaker: {
        question: "Do you have a caretaker relative who claims you on their taxes",
        Y: "haveCaretakerInfo",
        N: "dontHaveCaretakerInfo", 
    },
    deniedMedicare: {
        question: "Have you applied and been denied Medicare Coverage",
        Y: "unknown",
        N: "unknown",      
    },   
    startAgain: {
        question: "Would you like to restart?",
        Y: "start",
        N: "exit",              
    },
    unknown: {
        message: "Arrrghhhh. computer meltdown.. the pain, the pain... :P",
        next: "startAgain",
    },
    refSocialSecurity: {
        message: "Refer to Social Security to apply for the other part (Code E01M1)!"
    },
    refSocialSecurityAdministration: {
        message: "Refer to Social Security Administration to check status (Code E01M1)!"    
    },
    haveCaretakerInfo: {
        message: "Follow standard prequalification with caretaker relative as the income."
    },
    dontHaveCaretakerInfo: {
        message: "Refer to the HRA/LDSS/SLDSS to apply for benefits (CODE E01H)",
    },
    applyForHFInfo: {
        message: "Transfer to Medicare Advertising to apply for HealthFirst Medicare"
    },
    exit: {
        action: "showResult",
        message: "Thank you for your participation.",
    },
}

async function askQuestion(message, includeNotSure) {
    return new Promise(selectionMade => {
        quesionMenuEl.addEventListener("click", event => {
            selectionMade(event.target.value);
            event.preventDefault();  // stops the dialog from permanently closing
            questionDialog.close();
        }, {once: true});
        questiomMessageEl.textContent = message;
        notSureButton.style.display = includeNotSure ? null : "none"; //null is default 
        questionDialog.showModal();
    });
}
async function showMessage(message) {
    return new Promise(selectionMade => {
        messageMenuEl.addEventListener("click", event => {
            messageDialog.close();
            event.preventDefault(); // stops the dialog from permanently closing
            selectionMade("OK");
        }, {once: true});
        messageMessageEl.textContent = message;
        messageDialog.showModal();
    });
}
function interrogate(stateName = "exit") {
    const state = interrogation[stateName];
    if(state.action === "reset") { interrogation.results = "" }
    else if(state.action === "showResult") { console.log("Result: " + interrogation.results) }
    if (state.question) { 
        askQuestion(state.question, state.I !== undefined)
            .then(result => {
                if (result !== undefined) {
                    interrogation.results += result;
                    interrogate(state[result]);
                } else {
                    interrogate(stateName); // clicked outside button
                }
            });            
    } else if (state.message) {
        showMessage(state.message)
            .then(() => {
                if (stateName !== "exit") { interrogate(state.next) } 
            });
    }
}
<dialog id="questionDialog">
  <form method="dialog">
    <p id = "questiomMessageEl"></p>
    <menu id="quesionMenuEl">
      <button value="Y">Yes</button>
      <button value="N">No</button>
      <button value="I" id="notSureButton">Not sure</button>
    </menu>
  </form>
</dialog>
<dialog id="messageDialog">
  <form method="dialog">
    <p id="messageMessageEl"></p>
    <menu id="messageMenuEl">
      <button value="ok">OK</button>
    </menu>
  </form>
</dialog>

\$\endgroup\$
1
  • \$\begingroup\$ Blindman67 Thank you. That answers the question that I was asking. Thank you ! \$\endgroup\$
    – KoshVorlon
    Dec 17, 2018 at 17:58
0
\$\begingroup\$

Indent your code and watch the newlines and spacing.
There is a difference between == and =, you use both in mc1().
The names of your functions are not explaining what the functions do.
You might want to allow lowercase letters as input, this is a design choice. Your prompt specifies capitals, so it should suffice.
As an interactive guide, HTML/JS buttons might be better, but that is more a question for https://ux.stackexchange.com/

\$\endgroup\$

Not the answer you're looking for? Browse other questions tagged or ask your own question.