4
\$\begingroup\$

For a special writing and/or worldbuilding project I have, I created a special program.

In the story, different factions have different ideas on what should be considered the first year of their calendar system. For the sake of simplicity, the years are equally long, but the "first year" declarations are different.

This HTML-Javascript program uses a dynamically selected pair of lists where you can select conversion calendars. You then enter a year, hit a button, and the conversion happens.

The numbers are already tested by me and aside of one year of difference, are perfect.

What I ask is to give some insights of readibility, cleanness, and, if possible and conventient, any optimization.

HTML:

<!DOCTYPE html>
<html>

    <head>

        <script 
        <script src='/files/terminus-nation/convertdate.js'></script>
    </head>

    <body>

        <!-- A SIMPLE DATE CALCULATOR -->
        <div class="heading">
            <div class="datecalc">
                <h1> TN Date Calculator </h1>
                <h2>What a given year in a calendar system corresponds to, in another one?</h2>

                <input type="number" id="in_d" style="width:100px;"> in

                <select id="in_c"></select> to
                <select id="ou_c"></select>

                <button id="action" onClick="convertDate_onClick()">Convert</button>

                <p id="result"></p>
            </div>

        </div>

    </body>
    <script src='/files/terminus-nation/date-calc/main_js.js'></script>

</html>

convertdate.js, the conversion function, separated for flexibility:

var calendars = [
    ["BY", "Beacon Year"],
    ["KW", "Kvahk'er'Weplec"],
    ["RD", "Republic Date"],
    ["RY", "Resistance Year"],
    ["IC", "Initial Contact"],
    ["CD", "CyberDomini"],
    ["UD", "Union Day"]
];

//the core
function convertDate(in_cal, in_date, out_cal){


    in_date = parseInt(in_date);
    var orig = in_date;

    //each number shows, when a certain calender started compared to the others
    //e.g. 563159 in BY equals to 1 in KW and 100325 in KW equals to 1 in RD
    var DateOrder =  [563159, 100325, 3426, 1406, 2036, 1042];
    var in_id = -1;
    var ou_id = -1;

    function MinorConvert(string){

        var output = -2;
        for(i=0;i<calendars.length;i++){ if(string==calendar[i][0]){ output = i-1 }}
        return output;

    }

    in_id = MinorConvert(in_cal);
    ou_id = MinorConvert(out_cal);

    //-2 serves as an error code
    //it was implemented, when "calendar" wasn't added yet, but I'm afraid to eliminate
    if (in_id > -2 && ou_id > -2){

        //if the calendars different, it iterates through the calendar array
        //if the calendars are identical, it doesn't change the input
        if (in_id != ou_id){
            if(in_id<ou_id)      {  for(s=in_id;s<ou_id;s++){ in_date-= DateOrder[s+1]; }
                                        }else{  for(s=ou_id;s<in_id;s++){ in_date+= DateOrder[s+1]; }
            };
        };


    }else{ window.alert(in_cal + " to " + out_cal + " doesn't seem to be a valid conversation.") };


    //input is the returning value, either changed or unchanged (see comments at the condition above)
    return in_date;



}

main_js.js, the only file affecting the HTML directly:

//initialization
fillList("in_c");
fillList("ou_c");


//for select-list autocomplete
function fillList(id){

    //first already added, now add the rest
    for(i=0;i<calendars.length;i++){

        var option = document.createElement("option");
    option.value = calendars[i][0];
    option.text = calendars[i][1];
    document.getElementById(id).appendChild(option);
    }

}



//called by button click
function convertDate_onClick(){

    var in_c =  document.getElementById("in_c").value;
    var in_d = document.getElementById("in_d").value;
    var ou_c = document.getElementById("ou_c").value;

    document.getElementById("result").innerHTML = in_d + " in " + in_c + " equals to " + convertDate(in_c, in_d, ou_c) + " in " + ou_c;



}

I omitted CSS dependency as apparently the file works without that.

\$\endgroup\$
2
  • \$\begingroup\$ You can increase readability by refactoring the indentation of your JavaScript files. Especially the part starting with if (in_id != ou_id){ is hard to read. \$\endgroup\$ – insertusernamehere Jan 14 '17 at 10:36
  • 1
    \$\begingroup\$ @insertusernamehere oops, originally, at that part, the two loops were intended to be exactly below each other. But somehow, it was always different in Notepad++ than later in Atom. \$\endgroup\$ – Zoltán Schmidt Jan 14 '17 at 18:45
1
\$\begingroup\$

You're doing a lot of conversions there ... plus the formatting of the code doesn't help readability.

First step is to clean up the formatting:

pick up a few style rules and stick to it. Here's my pick, which follow my personal preferences:

    var calendars = [
        ["BY", "Beacon Year"],
        ["KW", "Kvahk'er'Weplec"],
        ["RD", "Republic Date"],
        ["RY", "Resistance Year"],
        ["IC", "Initial Contact"],
        ["CD", "CyberDomini"],
        ["UD", "Union Day"]
    ];
    
    //the core
    function convertDate(in_cal, in_date, out_cal) {
    
        in_date = parseInt(in_date);
        var orig = in_date;
    
        //each number shows, when a certain calender started compared to the others
        //e.g. 563159 in BY equals to 1 in KW and 100325 in KW equals to 1 in RD
        var DateOrder =  [563159, 100325, 3426, 1406, 2036, 1042];
        var in_id = -1;
        var ou_id = -1;
    
        function MinorConvert(string) {
    
            var output = -2;
            for (i = 0; i < calendars.length; i++) {
            	if (string == calendars[i][0]) {
            		output = i-1;
            	}
            }
            return output;
    
        }
    
        in_id = MinorConvert(in_cal);
        ou_id = MinorConvert(out_cal);
    
        //-2 serves as an error code
        //it was implemented, when "calendar" wasn't added yet, but I'm afraid to eliminate
        if (in_id > -2 && ou_id > -2) {
    
            //if the calendars different, it iterates through the calendar array
            //if the calendars are identical, it doesn't change the input
            if (in_id != ou_id){
                if (in_id < ou_id) {
                	for (s = in_id; s < ou_id; s++) {
                		in_date -= DateOrder[s+1];
                	}
                } else {
                	for (s = ou_id; s < in_id; s++) {
                		in_date += DateOrder[s+1];
                	}
                };
            };
    
    
        } else {
        	console.error(in_cal + " to " + out_cal + " doesn't seem to be a valid conversation.")
        };
    
        //input is the returning value, either changed or unchanged (see comments at the condition above)
        return in_date;
    }

in particular, spaces between operators and keywords in general, proper newlines and indentation helps me a lot when glancing at the code trying to understand the flow.

Second step, clean up and optimize:

Because of the data structure you picked, you're forced to convert N times depending on the distance between the two calendars. It would be more convenient to pick a system reference for the dates and convert the date against that single reference. You'll reduce conversions from N to 2 for every case.

The array-of-arrays structure forces you to get the values anonymously, like calendars[i][0] which doesn't make immediately clear what you're using. Again, i'm interested in understanding a piece of code i've never seen before at the first glance.

So, i've picked a dictionary structure with a couple properties for each calendar, and i've used the UD's year zero as the point of view of the system. This greatly simplifies the code at a very small cost for the data structure complexity:

// SYSTEM POINT OF VIEW: YEAR 0 IS UNION DAY
var calendars = {
	"BY": {
		name: "Beacon Year",
		shift: -671394
	},
	"KW": {
		name: "Kvahk'er'Weplec",
		shift: -108235
	},
	"RD": {
		name: "Republic Date",
		shift: -7910
	},
	"RY": {
		name: "Resistance Year",
		shift: -4484
	},
	"IC": {
		name: "Initial Contact",
		shift: -3078
	},
	"CD": {
		name: "CyberDomini",
		shift: -1042
	},
	"UD": {
		name: "Union Day",
		shift: 0
	}
};

//the core
function convertDate(in_cal, in_date, out_cal) {

    var out_date = NaN;

    if (in_cal !== out_cal) {
        if (in_cal in calendars && out_cal in calendars) {

            // convert the in_date from in_cal to POV calendar (UD)
            var pov_date = in_date + calendars[in_cal].shift;
            // convert pov year to out_cal
            out_date = pov_date - calendars[out_cal].shift;

        } else {
            console.error(in_cal + " to " + out_cal + " doesn't seem to be a valid conversation.")
        };
    }

    //input is the returning value, either changed or unchanged (see comments at the condition above)
    return isNaN(out_date) ? in_date : out_date;
}

of course this makes a lot of structural changes, so i'd suggest having a suite of unit tests in order to check the correctness of the conversion with the new code.

\$\endgroup\$
2
  • \$\begingroup\$ The thing is that I did not want to use a point of view as it would require for me to calculate a shift for each calendars by hand. I really wanted to avoid it, hence my method. I forgot mentioning it, though, so without this consideration, your solution is definitely better, thank you very much! \$\endgroup\$ – Zoltán Schmidt Jan 30 '17 at 17:26
  • \$\begingroup\$ aren't your offset also calculated by hand? the only difference i see is that your method calculates the offset between each other, while mine compares all against one common system. \$\endgroup\$ – alebianco Jan 30 '17 at 17:28

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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