My task was to create a menu that displays three items (A,B,C)
, and has two nav buttons <-
and ->
. So for example, a menu of (A, B, C, D, E)
starts off showing A,B,C
and can press ->
twice to display C,D,E
.
My questions beyond general code review feedback are:
- How would you view the "programmer" behind this code in terms of ability/thought?
- How would you feel if you were working on legacy code and for whatever reason, had to edit a menu or add to a menu or debug this code?
- (Maybe overlap as the first question) - if you were a hiring manager on an interview, what kind of impression would this code give you?
I feel as though I have a ton to learn so I'm not expecting great reviews or anything, so your brutal honesty is appreciated.
Here are the flaws I noticed myself:
- Img paths are hard-coded as
../images/misc/foo.png
instead of of having the include path defined as a constant - The container with
id=dataPreserve
may seem unclear in its purpose/messy? This was my first time using jQuery'sdata
function and I discovered that updating the entire container with.html();
, also overwrote the data (!), and that I had to leave an outer-most container untouched during.html();
s so as to preserve jQuery.data
.
Here is the code:
HTML page
<div id="mbpCont">asdf
</div>
<script>
var uniqueName = 'mbp';
var subtitles = ['NAME 1', 'NAME 2', 'NAME 3', 'NAME 4', 'NAME 5'];
var icons = ['ph.png', 'ph2.png', 'ph3.png', 'ph4.png', 'ph.png'];
var descripts = ['Ut enim ad minim veniam, quis nostrud exercitation.', 'Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur.', 'Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.', 'Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. ', 'final string'];
var links = ['prism.php', 'rp9.php', 'sa.php', 'ap.php', 'dd.php'];
var pos = 0;
var length = subtitles.length;
var isMoving = false;
mbpSlider = new Slider(uniqueName, subtitles, icons, descripts, links, pos, length, isMoving);
</script>
(sliders.js)
function Slider(uniqueName, subtitles, icons, descripts, links, pos, length, isMoving){
var thisSlider = '#'+uniqueName+'Cont';
$(thisSlider).html('<div class="sliderContainer" id="'+uniqueName+'">\
<div class="sliderArrowContainer">\
<a href="javascript:void(0);" id="matSliderLeft"><img class="sliderLeft" id="'+uniqueName+'Left" src="../images/misc/lArrowGrey.png"></a>\
</div>\
<div id="dataPreserve'+uniqueName+'" style="display:inline-block; *display:inline;zoom:1;">\
<div class="sliderBlock">\
<div class="sliderIconHolder"><img class="sliderImg" src="../images/misc/'+icons[0]+'"></div>\
<div class="sliderSubtitle" id="'+uniqueName+'">'+subtitles[0]+'</div>\
<div class="sliderText">'+descripts[0]+'</div>\
</div>\
<div class="sliderBlock">\
<div class="sliderIconHolder"><img class="sliderImg" src="../images/misc/'+icons[1]+'"></div>\
<div class="sliderSubtitle">'+subtitles[1]+'</div>\
<div class="sliderText">'+descripts[1]+'</div>\
</div>\
<div class="sliderBlock">\
<div class="sliderIconHolder"><img class="sliderImg" src="../images/misc/'+icons[2]+'"></div>\
<div class="sliderSubtitle">'+subtitles[2]+'</div>\
<div class="sliderText">'+descripts[2]+'</div>\
</div>\
</div>\
<div class="sliderArrowContainer">\
<a href="javascript:void(0);" id="matSliderRight"><img class="sliderRight" id="'+uniqueName+'Right" src="../images/misc/rArrowGreen.png"></a>\
</div>\
</div>');
$('#'+uniqueName).data( { subtitles: subtitles, icons: icons, descripts: descripts, links: links, pos: pos, length:length, isMoving:isMoving } );
}
function formatId(eventTarget){
if (eventTarget){
var formatted = eventTarget.substr(0,3);
return formatted;
} else {return null;}
}
function canGoLeft(cur){
var pos = getPos(cur);
//alert('checking left: pos = '+pos);
if (pos > 0) {
return true;
} else {
return false;
}
}
function canGoRight(cur){
var pos = getPos(cur);
var max = getMaxLength(cur);
max = max - 3;
if (pos < max) {
return true;
} else {
return false;
}
}
function getPos(cur){
var pos = $('#'+cur).data('pos');
return pos;
}
function getMaxLength(cur){
var max = $('#'+cur).data('length');
return max;
}
function isUpdating(cur){
var moving = $('#'+cur).data('isMoving');
if (moving){
return true;
} else {
return false;
}
}
function setMoving(cur, state){
if (state){
$('#'+cur).data('isMoving', true);
} else {
$('#'+cur).data('isMoving', false);
}
}
function updateDisplay(cur, direction){
if (updatePosition(cur, direction)){
updateHtml(cur);
setMoving(cur, false);
} else {
//alert('Error at updateDisplay');
return null;
}
}
function updatePosition(cur, direction){
var pos = getPos(cur);
pos = pos+direction;
//alert('new pos should be ' + pos);
if (pos>=0){
$('#'+cur).data('pos', pos);
return true;
} else {
return false;
}
}
function updateHtml(cur){
var thisSlider = '#dataPreserve'+cur;
var pos = getPos(cur);
updateArrows(cur);
$(thisSlider).hide().html('<div class="sliderBlock">\
<div class="sliderIconHolder"><img class="sliderImg" src="../images/misc/'+icons[pos]+'"></div>\
<div class="sliderSubtitle" id="'+cur+'">'+subtitles[pos]+'</div>\
<div class="sliderText">'+descripts[pos]+'</div>\
</div>\
<div class="sliderBlock">\
<div class="sliderIconHolder"><img class="sliderImg" src="../images/misc/'+icons[pos+1]+'"></div>\
<div class="sliderSubtitle">'+subtitles[pos+1]+'</div>\
<div class="sliderText">'+descripts[pos+1]+'</div>\
</div>\
<div class="sliderBlock">\
<div class="sliderIconHolder"><img class="sliderImg" src="../images/misc/'+icons[pos+2]+'"></div>\
<div class="sliderSubtitle">'+subtitles[pos+2]+'</div>\
<div class="sliderText">'+descripts[pos+2]+'</div>\
</div>\
').fadeIn(600);
}
function getLArrowString(cur){
var pos = getPos(cur);
var str = '../images/misc/lArrowGrey.png';
if (pos>0){
str = '../images/misc/lArrowGreen.png';
}
return str;
}
function getRArrowString(cur){
var pos = getPos(cur);
var max = getMaxLength(cur);
max = max - 3;
var str = '../images/misc/rArrowGrey.png';
if (pos<max){
str = '../images/misc/rArrowGreen.png';
}
return str;
}
function updateArrows(cur){
var lArrowId = '#'+cur+'Left';
var rArrowId = '#'+cur+'Right';
var lString = getLArrowString(cur);
var rString = getRArrowString(cur);
$(lArrowId).attr("src", lString);
$(rArrowId).attr("src", rString);
}
/******************** ON CLICKS ************************/
$(".sliderLeft").live("click", function() {
var cur = formatId(this.id);
if(canGoLeft(cur)){
if ( !isUpdating(cur) ){
//alert('is not moving');
setMoving(cur, true);
updateDisplay(cur, -1);
} else {
//alert('is moving');
}
} else {
//alert('cant go left');
}
});
$(".sliderRight").live("click", function() {
var cur = formatId(this.id);
if(canGoRight(cur)){
if ( !isUpdating(cur) ){
//alert('is not moving');
setMoving(cur, true);
updateDisplay(cur, 1);
} else {
//alert('is moving');
}
} else {
//alert('cant go right');
}
});