Jump to content
Sign in to follow this  
Tesset

Debugging and optimizing code

Recommended Posts

I'm finally just about finished with the storybook program I was working on over the summer. The goal was to make a fancier way to read my stories by having a book which the user would click through to different pages. You can see a working sample of it here.

 

There's just a couple of bugs that I can't seem to fix. The biggest one is that, occasionally, a page repeats the first couple of characters from the previous page. Unfortunately, I have not been able to figure out why it happens, nor how to fix it.

 

fg6v.png

 

Here's the full code, if you need it. I'm going to pull out the relavant functions, though, so don't worry about reading it.

 

[hide]

var story; //The html text of the story. Except, only sort of html, since only <br>'s are acceptable.
var firstChar=0; //The first character of the left page.
var lastChar=0; //The last character of the right page.

//Opens the story to the storybook.
//var path: file path to pull story text from.
function openBook(path){
    //Create a translucent background to dim the main site.
    var back = document.createElement("div");
	back.setAttribute("id", "dimmer");
	back.style.backgroundColor="rgba(0,0,0,0.75)"; //Sets color to black and 75% opacity
	back.style.width="100%";
	back.style.height="100%";
	back.style.position="fixed";
	back.style.left="0px";
	back.style.top="0px";
	document.body.appendChild(back);
	
	//Creates bookmark object, which will allow user to save his position in the book.
	var bookmark = document.createElement("div");
	bookmark.setAttribute("id","bookmark");
	bookmark.onclick=bookmarkPage;
    //Moves bookmark up/down to emphasize that it's clickable.
	bookmark.onmouseover=function(){bookmark.style.top="-10px";}
	bookmark.onmouseout=function(){bookmark.style.top="0px";}
	bookmark.style.width="125px";
	bookmark.style.height="600px";
	bookmark.style.position="fixed";
	bookmark.style.margin="auto auto";
	bookmark.style.right="0px";
	bookmark.style.left="125px";
	bookmark.style.top="0px";
	bookmark.style.bottom="90px";
	bookmark.style.backgroundImage='url("http://static.tumblr.com/a14d5c1e7ef40629024ff55c737c4a7c/x7vxvm9/jjPms2aj0/tumblr_static_bookmark.png")';
	bookmark.style.backgroundPositionY="top";
	bookmark.style.backgroundRepeat="no-repeat";
    back.appendChild(bookmark);
	
	//Main storybook container. 786*582 px
	var div = document.createElement("div"); 
	div.setAttribute("id","storybook");
	div.style.width = "786px";
	div.style.height = "582px";
	div.style.position="fixed";
	div.style.right="0px";
	div.style.left="0px";
	div.style.top="0px";
	div.style.bottom="0px";
	div.style.margin="auto auto";
	div.style.fontSize="16px";
	div.style.backgroundImage='url("http://static.tumblr.com/000d4fa9d90bfcf43fce181a6bddcc7c/x7vxvm9/IDMms2ak2/tumblr_static_storybook.png")';
	back.appendChild(div);
	
	//Left and right pages, respectively. Each is 350*528, with a bit of space between.
	var leftPage = document.createElement("div");
	leftPage.setAttribute("id","leftPage");
	leftPage.style.height="528px";
	leftPage.style.width="350px";
	leftPage.style.top="25px";
	leftPage.style.left="12px";
	leftPage.style.position="absolute";
	div.insertBefore(leftPage);
	
	var rightPage = document.createElement("div");
	rightPage.setAttribute("id","rightPage");
	rightPage.style.height="528px";
	rightPage.style.width="350px";
	rightPage.style.top="25px";
	rightPage.style.right="12px";
	rightPage.style.position="absolute";
	div.appendChild(rightPage);
	
	//Clicking this button will close the storybook program.
	var closeButton = document.createElement("div");
	closeButton.setAttribute("id","closeButton");
	closeButton.onclick=closeBook;
	closeButton.style.backgroundImage='url("http://static.tumblr.com/31529799c067d0559bc6ada471ada49a/x7vxvm9/okvms2aj3/tumblr_static_closebutton.png")';
	closeButton.style.width="28px";
	closeButton.style.height="28px";
	closeButton.style.position="fixed";
	closeButton.style.margin="auto auto";
	closeButton.style.right="0px";
	closeButton.style.left="-760px";
	closeButton.style.top="-582px";
	closeButton.style.bottom="0px";
	closeButton.style.cursor="pointer";
	back.appendChild(closeButton);
	
	//Sits on left page, goes to previous page.
	var prevButton = document.createElement("div");
	prevButton.setAttribute("id","prevButton");
	prevButton.onclick=prevPage;
	prevButton.onmouseover=function(){this.style.backgroundImage="url('http://static.tumblr.com/0dfec6b59cfb1fdc56cb0bed3c70e8b1/x7vxvm9/3uOms2ajh/tumblr_static_left_page_turn.png')"}
	prevButton.onmouseout=function(){this.style.backgroundImage="url('http://static.tumblr.com/c5e192b9d0af1287d97a19266e86b05a/x7vxvm9/XxTms2aj9/tumblr_static_left_page_arrow.png')"}
	prevButton.style.width="70px";
	prevButton.style.height="110px";
	prevButton.style.position="absolute";
	prevButton.style.bottom="0px";
	prevButton.style.left="0px";
	prevButton.style.backgroundImage="url('http://static.tumblr.com/c5e192b9d0af1287d97a19266e86b05a/x7vxvm9/XxTms2aj9/tumblr_static_left_page_arrow.png')";
	div.appendChild(prevButton);
	
	//Sits on right page, goes to next page.
	var nextButton = document.createElement("div");
	nextButton.setAttribute("id","nextButton");
	nextButton.onclick=nextPage;
	nextButton.onmouseover=function(){this.style.backgroundImage="url('http://static.tumblr.com/e671d833b2a84c6fc6bf0625ace14aed/x7vxvm9/y00ms2ajq/tumblr_static_right_page_turn.png')"}
	nextButton.onmouseout=function(){this.style.backgroundImage="url('http://static.tumblr.com/5304bd201b95f803f192ab6263470907/x7vxvm9/Ltcms2ajm/tumblr_static_right_page_arrow.png')"}
	nextButton.style.width="70px";
	nextButton.style.height="110px";
	nextButton.style.position="absolute";
	nextButton.style.bottom="0px";
	nextButton.style.right="0px";
	nextButton.style.backgroundImage="url('http://static.tumblr.com/5304bd201b95f803f192ab6263470907/x7vxvm9/Ltcms2ajm/tumblr_static_right_page_arrow.png')";
	div.appendChild(nextButton);
	
	//Open the story from the given file path.
	openStory(path);
	//If there's a bookmark cookie, open it. This will set firstChar to whatever it was when the user bookmarked, so firstPage will open that page instead of the page starting at firstChar=0
	openBookmark();
	//Open the first two pages after firstChar.
	firstPage();
}

//Close the book by removing the background dimmer, under which all the other elements are child nodes.
function closeBook(){
	document.body.removeChild( document.getElementById("dimmer") );
}

//Save a bookmark containing the firstChar which the user was on. Expires in 30 days.
function bookmarkPage(){
    var exdate=new Date();
    exdate.setDate(exdate.getDate() + 30);
	document.cookie="firstChar="+firstChar+"; expires="+exdate.toUTCString();
	closeBook();
}

//Checks if there is a bookmark cookie. If so, firstChar gets set there.
function openBookmark(){
	var fcStart, fcEnd, cookieString;
	cookieString=document.cookie; //Gets page's cookies.
	fcStart=cookieString.indexOf("firstChar="); //Finds where the firstChar cookie starts.
    //If no firstChar cookie is there, don't continue.
	if (fcStart==-1){
	   return null;
    }
	
	//Figure out where the string that firstChar should be set to starts and ends. Looks relative to the first instance of "firstChar=".
	fcStart=cookieString.indexOf("=",fcStart)+1;
	fcEnd=cookieString.indexOf(";",fcStart);
	if (fcEnd==-1)
		fcEnd=cookieString.length;
	
	//Get the substring with firstChar's value, convert to an integer.
    firstChar=parseInt( cookieString.substring(fcStart,fcEnd) );
	
}

//Opens the story and saves it to the global story.
//var path: file path to pull story text from.
function openStory(path){
	xmlhttp = new XMLHttpRequest();
	xmlhttp.open("GET",path,false);
	xmlhttp.send(null);
	var lines = xmlhttp.responseText.split("[[zxjz]]");
    story=lines[1];
}



//Open the pages which start at firstChar.
function firstPage(){
    //end: last character of the first page +1. Where the second page will start.
    //insertSizedString: returns the number of characters on the first page, which is how far from firstChar the second page should start.
	var end=firstChar+insertSizedString( story.substring(firstChar) ,document.getElementById("leftPage") );
	//If the first page is not also the last page
	if (end<story.length-1)
		lastChar=end+ insertSizedString( story.substring(end) , document.getElementById("rightPage") );
	else{//If first page is also the last page
        //Remove the button to go to the next page.
		document.getElementById("nextButton").style.visibility="hidden";
		lastChar=story.length-1;
		//Clear previous page from the right page.
		document.getElementById("rightPage").innerHTML="";
	}
	
	//If the firstChar is at the beginnning of the story, remove the button to go to the previous page.
	if (firstChar<=120){
		document.getElementById("prevButton").style.visibility="hidden";
		firstChar=0;
	}
}

//Go to the page starting at lastChar. Operates from the same principle as firstPage().
function nextPage(){
	firstChar=lastChar; 
    var end=firstChar+insertSizedString( story.substring(firstChar) ,document.getElementById("leftPage") );
	
	if (end<=story.length-1){
		lastChar=end+insertSizedString( story.substring(end) , document.getElementById("rightPage") );
        if (lastChar>=story.length-1)
            document.getElementById("nextButton").style.visibility="hidden";
	}else{
		document.getElementById("nextButton").style.visibility="hidden";
		lastChar=story.length-1;
		document.getElementById("rightPage").innerHTML="";
	}
	
	if (lastChar>=story.length*.9)
	   alert(lastChar+"/"+story.length);
	
	document.getElementById("prevButton").style.visibility="visible";
}

//Opens the page which ends right before firstChar.
function prevPage(){
	lastChar=firstChar;
	firstChar=lastChar-insertSizedStringReversed( story.substring(0,lastChar),document.getElementById("leftPage"),document.getElementById("rightPage") );
	
	document.getElementById("nextButton").style.visibility="visible";
	if (firstChar==0){
		document.getElementById("prevButton").style.visibility="hidden";
	}
}

function insertSizedString(str, element){
	var cont=element.appendChild(document.createElement("div"));
	
	cont.style.width=element.style.width;
	cont.innerHTML=str;
		
	while (cont.clientHeight>element.clientHeight*2){
		cont.innerHTML=cont.innerHTML.substring(0, cont.innerHTML.length/2);
	}
	
	cont.innerHTML=cont.innerHTML.substring(0, cont.innerHTML.lastIndexOf(" "));
	
	while ( cont.clientHeight>element.clientHeight ){
		cont.innerHTML=cont.innerHTML.substring(0, cont.innerHTML.lastIndexOf(" "));
	}
	element.innerHTML=cont.innerHTML;
	
	//alert(element.innerHTML+" "+element.innerHTML.length);
	
	return element.innerHTML.length;
}

function insertSizedStringReversed(str,element1,element2){
	var cont=element2.appendChild(document.createElement("div"));
	cont.style.width=element2.style.width;
	cont.innerHTML=str;
	
	while (cont.clientHeight>(element2.clientHeight+element1.clientHeight)*2.05){
		cont.innerHTML=cont.innerHTML.substring( cont.innerHTML.length/2-1 );
	}
	
	while ( cont.clientHeight>(element2.clientHeight+element1.clientHeight) ){
		cont.innerHTML=cont.innerHTML.substring( cont.innerHTML.indexOf(" ")+1 );
	}
	var bothPages=cont.innerHTML;
	
	while ( cont.clientHeight>element1.clientHeight ){
		cont.innerHTML=cont.innerHTML.substring(0, cont.innerHTML.lastIndexOf(" "));
	}
	element1.innerHTML=cont.innerHTML;
	element2.innerHTML=bothPages.substring(cont.innerHTML.length);
	
	return element1.innerHTML.length+element2.innerHTML.length;
}
[/hide]

 

Essentially, the important pieces are the two functions for going to the next page. story, firstChar, and lastChar are all global variables, containing the text of the story as a string, the location in the string of the first character of the left page, and the last character of the right page, respectively.

//Go to the page starting at lastChar. Operates from the same principle as firstPage().
function nextPage(){
	firstChar=lastChar; 
	//end: last character of the first page +1. Where the second page will start.
    //insertSizedString: returns the number of characters on the first page, which is how far from firstChar the second page should start.
    var end=firstChar+insertSizedString( story.substring(firstChar) ,document.getElementById("leftPage") );
	//If the first page is not also the last page
	if (end<=story.length-1){
		lastChar=end+insertSizedString( story.substring(end) , document.getElementById("rightPage") );
        /*(This is tabbed in)*/if (lastChar>=story.length-1){document.getElementById("nextButton").style.visibility="hidden"; }
	}else{//If first page is also the last page
	   //Remove the button to go to the next page.
		document.getElementById("nextButton").style.visibility="hidden";
		lastChar=story.length-1;
		//Clear previous page from the right page.
		document.getElementById("rightPage").innerHTML="";
	}
	
	//Debugging
	if (lastChar>=story.length*.9)
	   alert(lastChar+"/"+story.length);
	
	document.getElementById("prevButton").style.visibility="visible";
}

//Inserts a substring of str into element that is exactly as long as can fit into element without overflow.
function insertSizedString(str, element){
	var cont=element.appendChild(document.createElement("div"));//Create a container under the element to test the dimensions of the text.
	
	cont.style.width=element.style.width;
	cont.innerHTML=str; //The system works backwards, from the full str until it fits into element.
	
	//Since the function is O(n^2) otherwise, cut the problem to a more reasonable length. This isn't perfect - there's still a noticeable pause - but there's an infinite loop otherwise
	while (cont.clientHeight>element.clientHeight*2){
		cont.innerHTML=cont.innerHTML.substring(0, cont.innerHTML.length/2);
	}
	
	cont.innerHTML=cont.innerHTML.substring(0, cont.innerHTML.lastIndexOf(" "));
	
	//Remove parts of the string, at word breaks, until the container is only as tall as element, at which point all the text will fit.
	while ( cont.clientHeight>element.clientHeight ){
		cont.innerHTML=cont.innerHTML.substring(0, cont.innerHTML.lastIndexOf(" "));
	}
	element.innerHTML=cont.innerHTML;
	
	//Return the length of the inserted string, so that the system knows where to start the next page.
	return element.innerHTML.length;
}
I know the problem has to do with how end is getting assigned - somehow, despite the fact that firstChar is assigned correctly and the correct length is being returned, end is not in the correct position. I thought it might have to do with the fact that I have non breaking spaces to mark tabs, but it isn't consistent between pages that start with them/have them and pages that don't. After that, I'm not sure - it's not caused by formatting marks, or anything else I can determine.

 

On a slightly different note, this program is a little slow. There's a pause when you change pages. I'm sure it's because, as I noted in the comments, the insertSizedString() function is O(n2), because substring appears to just be a loop. Originally, insertSizedString() ran forward, adding words until cont was too tall, then removing the last word and assigning the result to element, but that came with a huge number of bugs, so I scrapped it. I can't come up with another system by which to solve this problem, so I'm running with this unless someone can show me a better way, or show me a way to make this more efficient.

 

There's a few other bugs, but I think I'm just going to leave them be if fixing this doesn't fix those. They're not very noticeable, and they're rare, so I'm not worried about them. But this one is, and I need it to be fixed. So can anyone offer any help?

 

edit: Oh. I broke the code when I posted. One second. >.>


My skin is finally getting soft
I'll scrub until the damn thing comes off

Share this post


Link to post
Share on other sites

What's going on with the if brackets in nextPage ? Edit: got it

Share this post


Link to post
Share on other sites

Oh, sorry. I copy pasted it in from my editor, but the tabs must not have come correctly. I put some brackets and comments just to clear things up, I guess.


My skin is finally getting soft
I'll scrub until the damn thing comes off

Share this post


Link to post
Share on other sites

I'm way too tired to go through your code right now, but it looks like you're wasting a lot of time iterating through each individual character. At the very least parse each word individually.

 

Also use <p> to divide texts into paragraphs instead of <br/>.

Share this post


Link to post
Share on other sites

The dropbox link is giving a 403 forbidden, so the example doesn't work.

Share this post


Link to post
Share on other sites

I'm way too tired to go through your code right now, but it looks like you're wasting a lot of time iterating through each individual character. At the very least parse each word individually.

 

Also use <p> to divide texts into paragraphs instead of <br/>.

I am iterating through words, not characters. That's what the lastIndexOf(" ") does.

 

I'm using <br>s because I don't want the extra space after each paragraph that I would get with a <p>.

 

@Markup: Oh. I figured it would just work. :/ I'll fix it in a couple hours when I get home.


My skin is finally getting soft
I'll scrub until the damn thing comes off

Share this post


Link to post
Share on other sites

I figured out a really easy way to do this :L

 

http://jsfiddle.net/ERnxH/

 

I only put in the core functionality, so you'll still want to add basic error checking (such as not allowing users to read page -1 or 20000).

Share this post


Link to post
Share on other sites

I figured out a really easy way to do this :L

 

http://jsfiddle.net/ERnxH/

 

I only put in the core functionality, so you'll still want to add basic error checking (such as not allowing users to read page -1 or 20000).

Oh wow. That's... I wish I had found that method before I spent the whole summer coding my method... :/

 

Anyway, I think I'm going to convert that into JS (since I don't really know jQuery, and I want to put this on a resume as well) and add the functionality I need, which shouldn't be too tough. Thanks for this Mere. I'll post again if I have any troubles.


My skin is finally getting soft
I'll scrub until the damn thing comes off

Share this post


Link to post
Share on other sites

I figured out a really easy way to do this :L

 

http://jsfiddle.net/ERnxH/

 

I only put in the core functionality, so you'll still want to add basic error checking (such as not allowing users to read page -1 or 20000).

Did you test this on firefox? Isn't working for me

 

 

You need to change insertBefore to [email protected] 690

TypeError: Not enough arguments to Node.insertBefore.

div.insertBefore(leftPage);

Firefox again

Share this post


Link to post
Share on other sites

 

I figured out a really easy way to do this :L

 

http://jsfiddle.net/ERnxH/

 

I only put in the core functionality, so you'll still want to add basic error checking (such as not allowing users to read page -1 or 20000).

Did you test this on firefox? Isn't working for me

 

 

You need to change insertBefore to [email protected] 690

TypeError: Not enough arguments to Node.insertBefore.

div.insertBefore(leftPage);

Firefox again

 

You really need to update your browser.

Share this post


Link to post
Share on other sites

 

 

I figured out a really easy way to do this :L

 

http://jsfiddle.net/ERnxH/

 

I only put in the core functionality, so you'll still want to add basic error checking (such as not allowing users to read page -1 or 20000).

Did you test this on firefox? Isn't working for me

 

 

You need to change insertBefore to [email protected] 690

TypeError: Not enough arguments to Node.insertBefore.

div.insertBefore(leftPage);

Firefox again

 

You really need to update your browser.

 

23.0.1

Share this post


Link to post
Share on other sites

 

You need to change insertBefore to [email protected] 690

TypeError: Not enough arguments to Node.insertBefore.

div.insertBefore(leftPage);
Firefox again

 

Alright, I got on a computer with a copy of Firefox, and got everything up so it's running and looks the same there.

 

As a side note, I *did* test my program on IE as well as Chrome. I didn't have a copy of Firefox installed though, so I hadn't yet. Guess that'll teach me. :rolleyes:

 

 

 

 

I figured out a really easy way to do this :L

 

http://jsfiddle.net/ERnxH/

 

I only put in the core functionality, so you'll still want to add basic error checking (such as not allowing users to read page -1 or 20000).

Did you test this on firefox? Isn't working for me

 

You really need to update your browser.

 

23.0.1

 

Yeah, it's not working for me in ff either, neither on the fiddle nor where I was running it. :/

 

I also had an issue in Chrome where the text was getting cut off as I progressed through the pages. I was trying to fix it, but I wasn't quite sure why it was happening.


My skin is finally getting soft
I'll scrub until the damn thing comes off

Share this post


Link to post
Share on other sites

Edit: solution to original problem

Well your problem is that when you set innerHTML it strips \r\n to \n. To fix just use .replace("\r",""); on the book data after you've retrieved it.

function openStory(path){
    xmlhttp = new XMLHttpRequest();
    xmlhttp.open("GET",path,false);
    xmlhttp.send(null);
    var lines = xmlhttp.responseText.split("[[zxjz]]");
    story=lines[1];
    story = story.replace("\r","");
} 

Share this post


Link to post
Share on other sites

Nope. Didn't do anything. I don't know why it would - there aren't any carriage returns in the page I'm reading in, it's all just pure HTML on one line.


My skin is finally getting soft
I'll scrub until the damn thing comes off

Share this post


Link to post
Share on other sites

Nope. Didn't do anything. I don't know why it would - there aren't any carriage returns in the page I'm reading in, it's all just pure HTML on one line.

There are definitely carriage returns, at least on firefox. You've readded the insertBefore error aswell

Share this post


Link to post
Share on other sites

Okay so replace only replaces the first occurence. The fix is: story = story.replace(/\r/g,"");

Share this post


Link to post
Share on other sites

And another one I've found:

 

Edit: this one was caused because it doesn't have an opening <i> highlighted by the red </i>

kNW8zg1.png

 

element.innerHTML.substring(1199, 1210);
"y. <br> &nb"
str.substring(1199, 1210);
"y.</i> <br>"
str.substring(1199, 1220);
"y.</i> <br>  &nb"

Share this post


Link to post
Share on other sites

So the real fix honest is:

var fixstr = document.createElement("div");
fixstr.innerHTML = story;
story = fixstr.innerHTML;

Share this post


Link to post
Share on other sites

Oh. Well, that seems to have worked perfectly. That makes sense too, since it strips away anything that doesn't translate into the HTML that would be screwing up the way the function works. Awesome.

 

Thanks Markup. I really appreciate you helping me out with this. You too Mere.

 

I'm going to bed now, if I find any problems later, I'll make another post. Thanks guys.


My skin is finally getting soft
I'll scrub until the damn thing comes off

Share this post


Link to post
Share on other sites

Something else too, if it ends between a tag then it replaces the last few chars with an end tag?:

>>> element.innerHTML.length

1493
>>> element.innerHTML.substring(1480, 1493);
"nbsp;<i>I</i>"
>>> str.substring(1480, 1493);
"nbsp;<i>I mus"

Share this post


Link to post
Share on other sites

I figured out a really easy way to do this :L

 

http://jsfiddle.net/ERnxH/

 

I only put in the core functionality, so you'll still want to add basic error checking (such as not allowing users to read page -1 or 20000).

 

Going back to this. It doesn't work on firefox because firefox overflows to the right not downwards. Changing height for width and top for left makes it work, although it's not aligned properly.

 

Edit: http://jsfiddle.net/ERnxH/12/

$(document).ready(
    function(){
        page = 1;
    }
);
$("#next").click(
    function(){
        var width = $("#book").outerWidth()
        $("#container").css("left",(-width * page) + "px");
        page += 1;
    }
);
$("#back").click(
    function(){
        page -= 1;        
        var width = $("#book").outerWidth()
        $("#container").css("left",(-width * (page-1)) + "px");
        console.log($("#container").css("left"));
    }
);

This works on firefox, and accounts for padding/border which is the hardcoded 16.

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now
Sign in to follow this  

×
×
  • Create New...

Important Information

By using this site, you agree to our Terms of Use.