Name Mashup Program Improvement in Python I am learning python and trying to solve different problems i came across this program which is name mashup which is in Get Programming:Learn to code with Python.

problem statement: Write a program that automatically combines two names given by
the user. That’s an open-ended problem statement, so let’s add a few more details and restrictions:

Tell the user to give you two names in the format FIRST LAST. Show the user two possible new -names in the format FIRST LAST.
The new first name is a combination of the first names given by the user, and the new last name is a combination of the last names given by the user.

For example, if the user gives you Alice Cat and Bob Dog, a possible mashup is Bolice Dot.

I'd like to know is there any way to improve this program like using functions to make this program better.

    print('Welcome to the Name Mashup Game')

name1 = input('Enter one full name(FIRST LAST): ')
name2 = input('Enter second full name(FIRST LAST): ')

space = name1.find(" ")
name1_first = name1[0:space]
name1_last = name1[space+1:len(name1)]
space = name2.find(" ")
name2_first = name2[0:space]
name2_last = name2[space+1:len(name2)]
# find combinations
name1_first_1sthalf = name1_first[0:int(len(name1_first)/2)]
name1_first_2ndhalf = name1_first[int(len(name1_first)/2):len(name1_first)]
name1_last_1sthalf = name1_last[0:int(len(name1_last)/2)]
name1_last_2ndhalf = name1_last[int(len(name1_last)/2):len(name1_last)]

name2_first_1sthalf = name2_first[0:int(len(name2_first)/2)]
name2_first_2ndhalf = name2_first[int(len(name2_first)/2):len(name2_first)]
name2_last_1sthalf = name2_last[0:int(len(name2_last)/2)]
name2_last_2ndhalf = name2_last[int(len(name2_last)/2):len(name2_last)]

new_name1_first = name1_first_1sthalf + name2_first_2ndhalf
new_name1_last = name1_last_1sthalf + name2_last_2ndhalf
new_name2_first = name2_first_1sthalf + name1_first_2ndhalf
new_name2_last = name2_last_1sthalf + name1_last_2ndhalf

# print results
print(new_name1_first, new_name1_last)
print(new_name2_first, new_name2_last)

Any time you're doing essentially the same thing (even if it's in different orders) to different values, it's a strong clue that the values should be a collection (like a list or tuple) rather than independent named variables.  It'll usually make your code a lot shorter, and it makes it much much easier to extend -- what if you wanted to play the mashup game with more names?

from itertools import permutations
from typing import List

print('Welcome to the Name Mashup Game')

names = [
    input('Enter one full name(FIRST LAST): ').split(),
    input('Enter second full name(FIRST LAST): ').split(),
    # input('Enter third full name(FIRST LAST):' ).split(),  <- try me!
]

def mash(first: List[str], second: List[str]) -> List[str]:
    """
    Produce a list where each element has the first half of the corresponding
    element from the first list and the second half of the corresponding
    element from the second list.

    For example:
        mash(['AA', 'BB'], ['XX', 'YY']) -> ['AX', 'BY']
    """
    return [
        a[:len(a)//2] + b[len(b)//2:]
        for a, b in zip(first, second)
    ]

for first, second in permutations(names, 2):
    print(' '.join(mash(first, second)))


Implementing HTUnEscape I came across the following piece of code in a UI application I need to maintain.

int tool_unhex( char c )
{
    return( c >= '0' && c <= '9' ? c - '0'
        : c >= 'A' && c <= 'F' ? c - 'A' + 10
        : c - 'a' + 10 );
}

void unescape2QString(const char *sOrg, QString & str)
{
/*
 * Remove URL hex escapes from s... done in place.  The basic concept for
 * this routine is borrowed from the WWW library HTUnEscape() routine.
 */
    char* s = (char*)sOrg;
    unsigned short w = 0;

    str = "";

    for ( ; *s != '\0'; ++s ) {
        if ( *s == '%' ) {
            if(*(s+1) == 'u')
            {
                s++;
                if ( *++s != '\0' ) {
                    w = (wchar_t) (tool_unhex( *s ) << 12);
                }
                if ( *++s != '\0' ) {
                    w += (wchar_t) (tool_unhex( *s ) << 8);
                }
                if ( *++s != '\0' ) {
                    w += (wchar_t) (tool_unhex( *s ) << 4);
                }
                if ( *++s != '\0' ) {
                    w += (wchar_t) tool_unhex( *s );
                }

                str += QString::fromUtf16(&w, 1);
            }
        }
        else
        {
            str += QString::fromAscii(s, 1);
        }
    }
}

Isn't QString immutable?  The comment says it's changing in place, but isn't each str += creating a new QString?
Is this the most optimized way of doing this?

It looks like the function is trying to implement HTUnEscape() from w3.org, with UTF16 strings.

Firstly, to answer your first question, QString is definitely not immutable. Directly from the documentation:

  This operation is typically very fast (constant time), because QString preallocates extra space at the end of the string data so it can grow without reallocating the entire string each time.

I'm not a fan of the nested ternary statements in tool_unhex. Also, it does no error checking to make sure that whatever it is passed is actually a valid hex character. This is dangerous, as you use an unsigned short to store the shifted value of the character. If it's some character that's above F or f, then shifting and adding will potentially overflow and cause hard to track down bugs. For example, if there was a g that got in there somehow:

'g' - 'a' + 10;   // Equals 16
unsigned short w = (16 << 12) + (16 << 8) + (16 << 4) + 16;  // Overflow

I'd suggest changing it to the following:

#include <cctype>

// Converts the given (ASCII encoded) hex character to
// an integer value 0 - 15.
// throw std::out_of_range if the given character is not
// a valid hex character.
int tool_unhex(char c)
{
    if(std::isdigit(c))
        return c - '0';
    else if(c >= 'A' && c <= 'F')
        return c - 'A' + 10;
    else if(c >= 'a' && c <= 'f')
        return c - 'a' + 10;
    throw std::out_of_range("Not a hex character");
}

If you're sure that the character passed in will always be in range, this should be documented somewhere. Also, it should be documented that this will ONLY work for ASCII and ASCII-compatible encodings.

Your unescape method has some confusing parts. The comments and the code don't match.

char* s = (char*)sOrg;

Why cast away the constness of sOrg? You don't modify s anyway, so just work directly on the (const) sOrg.

str = "";

If you're just going to clear the QString passed in straight away, why not just create and return it instead of taking it as a reference?

if ( *s == '%' ) {
    if(*(s+1) == 'u')
    {
        s++;
        if ( *++s != '\0' ) {
            w = (wchar_t) (tool_unhex( *s ) << 12);
        }
        if ( *++s != '\0' ) {
            w += (wchar_t) (tool_unhex( *s ) << 8);
        }
        if ( *++s != '\0' ) {
            w += (wchar_t) (tool_unhex( *s ) << 4);
        }
        if ( *++s != '\0' ) {
            w += (wchar_t) tool_unhex( *s );
        }

        str += QString::fromUtf16(&w, 1);
    }
}

This can certainly be simplified with a loop.

In the end, I came up with something like this:

QString unescape2QString(const char *sOrg)
{
    QString str;

    for ( ; *sOrg != '\0'; ++sOrg ) {
        if (*sOrg == '%' && *(sOrg+1) == 'u') {
        {
           unsigned short w = 0;
           ++sOrg;
            // Left shift value
            int i = 12;
            while(*++sOrg != '\0' && i >= 0) {
                w += (wchar_t) (tool_unhex(*sOrg) << i);
                i -= 4;
            }
            str += QString::fromUtf16(&w, 1);
        }
        else
        {
            str += QString::fromAscii(s, 1);
        }
    }
    return str;
}

This definitely needs some documentation as to what exactly it's doing, as it is absolutely not clear currently.


Editing Surfaces Removing Color So I have recently tried to load an image and save it, but remove all of a certain color. I'm running the program and it is running really slow, is there a more efficient way to edit an image? My code is pasted below, or at least all that is relevant.

for x in range(50):
    im = pygame.image.load("horse%s.bmp") % (x)
    im.set_colorkey(im.get_at((0, 0)))
    surf = pygame.Surface((im.get_size(), pygame.SRCALPHA)
    surf.blit(im, (0, 0))
    pygame.image.save(surf, "horse%s.png") % (x)

You have a bug:

((im.get_size(), pygame.SRCALPHA)

You have one too many ('s.

Where did the number 50 come from in the range call?

Don't use magic numbers; set and use a variable, probably called picture_amount.

When you are using string formatting to put the number x into the image name string, you should be using %d or %i because those are for signed decimal (base 10) integers; %s is used for strings, which x is not.

After scrolling through some pygame documentation, your method for clearing all is a very good one for pygame. I am assuming that the excessive slowness is coming from having to paint the image to a surface every time.

A faster way to do this would probably be to use an image editing library so that you don't have to paint the image every time; you'd edit the image in the background.

I sifted through the PIL python imaging library and came across a convenient method call image.eval. This method will iterate through every pixel in an image, call a function (passed via argument) passing in the pixel information, and reset the pixel value based on the function output.

This may or may not be slower than what you are already doing.

Putting it all together:

from PIL import Image

for x in range(image_amount):
    im = Image.open("horse%d.bmp") % (x)
    Image.eval(im, lamda pix: 0 if p == im.getpixel((0,0))) # Sets the pixel value to 0... change this if you want.
    Image.save("horse%d", "PNG") % (x) # This could raise an IOException

I could not test this code at the time so if you see any errors, let me know.


Animated Score Amounts for Game This is a simple class for a label with a score that animates counting up or down.  When someone in the game scores points, the numbers count up or down to the new total.

Here is an example of what it looks like:

I ran into a few problems when building this class.  It was important to prevent the building of further animations while a current animation was playing, because otherwise the score would be incremented too many times.  It was also important to to set the correct values at the end of the sequence of animations in order to make sure that it was always accurate when it completed.

BZAnimatedScoreLabel.h

#import <SpriteKit/SpriteKit.h>

@interface BZAnimatedScoreLabel : SKLabelNode

+(BZAnimatedScoreLabel *) labelWithText:(NSString *)text score:(int)score size:(int)fontSize;

-(void) updateForScore:(int)newScore;

@end

BZAnimatedScoreLabel.m

#import "BZAnimatedScoreLabel.h"

@implementation BZAnimatedScoreLabel {
    int _score;
    SKLabelNode *_scoreLabel;
    NSMutableArray *_actionQueue;
    BOOL _isAnimationPlaying;
}

#pragma mark - Initialization
+(BZAnimatedScoreLabel *) labelWithText:(NSString *)text score:(int)score size:(int)fontSize {
    return [[BZAnimatedScoreLabel alloc]initWithText:text score:score size:fontSize];
}
-(instancetype) initWithText:(NSString *)text score:(int)score size:(int)fontSize {
    self = [super initWithFontNamed:@"Arial"];
    if (self) {
        self.text = text;
        self.fontSize = fontSize;
        self.fontColor = [SKColor whiteColor];
        _score = score;

        _scoreLabel = [[SKLabelNode alloc]initWithFontNamed:@"Arial"];
        _scoreLabel.fontSize = fontSize;
        _scoreLabel.fontColor = [SKColor whiteColor];
        _scoreLabel.text = [NSString stringWithFormat:@"%i", _score];
        _scoreLabel.position = CGPointMake((fontSize * 4), 0);
        [self addChild:_scoreLabel];

        _isAnimationPlaying = NO;
        _actionQueue = [[NSMutableArray alloc]init];
    }
    return self;
}

#pragma mark - Animation
-(void) updateForScore:(int)newScore {
    if (!_isAnimationPlaying) {
        if (newScore > _score) {
            [self updateForHigherScore:newScore];
        } else {
            [self updateForLowerScore:newScore];
        }
    }
}
-(void) updateForHigherScore:(int)newScore {
    for (int i = _score; i <= newScore; i+=10) {
        [self addAnimationToQueueForAmount:i];
    }
    [self playQueuedAnimationsFinalScore:newScore];
}
-(void) updateForLowerScore:(int)newScore {
    for (int i = _score; i >= newScore; i-=10) {
        [self addAnimationToQueueForAmount:i];
    }
    [self playQueuedAnimationsFinalScore:newScore];
}
-(void) addAnimationToQueueForAmount:(int)amount {
    [_actionQueue addObject:[SKAction runBlock:^(void){
        _scoreLabel.text = [NSString stringWithFormat:@"%i", amount];
    }]];
}
-(void) playQueuedAnimationsFinalScore:(int)finalScore {
    _isAnimationPlaying = YES;
    [self runAction:[SKAction sequence:_actionQueue] completion:^(void) {
        _score = finalScore;
        _scoreLabel.text = [NSString stringWithFormat:@"%i", _score];
        _isAnimationPlaying = NO;
        [_actionQueue removeAllObjects];
    }];
}

@end

Here is an example usage in the SKScene:

//object creation
_pointsLabel = [BZAnimatedScoreLabel labelWithText:@"Points = " score:0 size:20];
_pointsLabel.position = CGPointMake(_initialScreenSize.width/1.8, _initialScreenSize.height/28);
[self addChild:_pointsLabel];

//usage
[_pointsLabel updateForScore:_game.currentScore];

I thought about expanding the initialization method of the class to include the size of the scene (for proper spacing of the points from the text) and also the position of the label, but the initialization method is already pretty long and adding two further arguments feels like too much, but I am not sure.

If the score label is updated while the animation is still running, then
the update is simply ignored. For example, with

[_pointsLabel updateForScore:100];
[_pointsLabel updateForScore:200];

the label will animate to 100 and stay there, instead of animating to 200.

Instead of pre-computing all actions from the current score to the final value,
I would start only a single action that will display the next intermediate score, e.g. from 100 to 110. When that action has completed, start a new action.

This approach solves the problem of simultaneously running actions, and makes
both _isAnimationPlaying and the _actionQueue obsolete. Each time an action is created,
it can check whether the counter has to be incremented or decremented.
The animation is always in steps of 10, e.g. an update from 13 to 51 will
display 13, 23, 33, 43, 51. It would look nicer if multiples of 10 are displayed
where possible, in this case 13, 20, 30, 40, 50, 51.
The animation is too fast. At last on my Simulator it was running so fast that
not all intermediate steps can be recognized. I would add an small delay
between the actions.
The updateForScore: method is not really necessary. I would make score
a (public) property and override the setter method, so that

_pointsLabel.score = newValue;

updates the score and starts the animation.

Then your implementation could look like this:

BZAnimatedScoreLabel.h

@interface BZAnimatedScoreLabel : SKLabelNode
+(BZAnimatedScoreLabel *) labelWithText:(NSString *)text score:(int)score size:(int)fontSize;
@property (nonatomic) int score;
@end

BZAnimatedScoreLabel.m

#import "BZAnimatedScoreLabel.h"

@implementation BZAnimatedScoreLabel {
    SKLabelNode *_scoreLabel;
    int _currentScore; // The currently displayed score
}

#pragma mark - Constants

static NSString *kAnimationKey = @"BZLabelAnimationKey";
static const NSTimeInterval kAnimationDelay = 0.02;

#pragma mark - Initialization

+(BZAnimatedScoreLabel *) labelWithText:(NSString *)text score:(int)score size:(int)fontSize {
    return [[BZAnimatedScoreLabel alloc]initWithText:text score:score size:fontSize];
}

-(instancetype) initWithText:(NSString *)text score:(int)score size:(int)fontSize {
    self = [super initWithFontNamed:@"Arial"];
    if (self) {
        self.text = text;
        self.fontSize = fontSize;
        self.fontColor = [SKColor whiteColor];

        _currentScore = _score = score;
        _scoreLabel = [[SKLabelNode alloc] initWithFontNamed:@"Arial"];
        _scoreLabel.fontSize = fontSize;
        _scoreLabel.fontColor = [SKColor whiteColor];
        _scoreLabel.position = CGPointMake(fontSize * 4, 0);
        _scoreLabel.text = [NSString stringWithFormat:@"%i", score];
        [self addChild:_scoreLabel];
    }
    return self;
}

#pragma mark - Animation

-(void)setScore:(int)score {
    _score = score;
    [self updateDisplay];
}

// Compute next multiple of 10 from _currentScore in the direction of _score:
-(int)computeNextScore {
    int next;
    if (_score > _currentScore) {
        if (_currentScore >= 0) {
            next = ((_currentScore + 10)/ 10) * 10;
        } else {
            next = ((_currentScore + 1)/ 10) * 10;
        }
        if (next > _score) {
            next = _score;
        }
    } else if (_score < _currentScore) {
        if (_currentScore <= 0) {
            next = ((_currentScore - 10) / 10) * 10;
        } else {
            next = ((_currentScore - 1) / 10) * 10;
        }
        if (next < _score) {
            next = _score;
        }
    } else {
        next = _score;
    }
    return next;
}

-(void)updateDisplay {
    if (_score != _currentScore) {
        SKAction *wait = [SKAction waitForDuration:kAnimationDelay];
        SKAction *update = [SKAction runBlock:^() {
            _currentScore = [self computeNextScore];
            _scoreLabel.text = [NSString stringWithFormat:@"%i", _currentScore];
        }];
        SKAction *checkAgain = [SKAction performSelector:@selector(updateDisplay) onTarget:self];
        [self runAction:[SKAction sequence:@[wait, update, checkAgain]] withKey:kAnimationKey];
    } else {
        [self removeActionForKey:kAnimationKey];
    }
}

@end

_currentScore is the currently displayed score, and the computeNextScore
method computes the next value to be displayed. It looks a bit complicated,
but it works correctly for both positive and negative scores.

updateDisplay starts a sequence of three actions (if necessary): wait
is for the delay, update computes the next score to be displayed and
updates the label, and checkAgain causes the updateDisplay method to
be called again.

The sequence action is created with a key. This has two advantages:
The action can be removed, and starting a new action with the same key
will automatically stop the previous action.


Text representation for numbers with python Simple function which returns a text representation for numbers in range [1...1000].

N100 = 'hundred'
N1000 = 'one thousand'
N1_9 = [
    'one', 'two', 'three',
    'four', 'five', 'six',
    'seven', 'eight', 'nine',
]
N10_19 = [
    'ten', 'eleven', 'twelve',
    'thirteen', 'fourteen', 'fifteen',
    'sixteen', 'seventeen', 'eighteen',
    'nineteen',
]
Nx10 = [
    'twenty', 'thirty', 'forty',
    'fifty', 'sixty', 'seventy',
    'eighty', 'ninety',
]

def make_word(num):
    assert 0 < num <= 1000
    if num == 1000:
        return N1000
    lst = [0] * 3
    for i in xrange(3):
        div = 10 ** (2 - i)
        lst[i] = num / div
        num %= div
    res = []
    if lst[0] != 0:
        res.append(
            '{} {}'.format(N1_9[lst[0]-1], N100)
        )
    if lst[1] or lst[2]:
        if lst[1] == 0:
            res.append(
                N1_9[lst[2] - 1]
            )
        elif lst[1] == 1:
            res.append(
                N10_19[lst[2]]
            )
        else:
            if lst[2] == 0:
                res.append(
                    Nx10[lst[1] - 2]
                )
            else:
                res.append(
                    '{}-{}'.format(Nx10[lst[1] - 2], N1_9[lst[2] - 1])
                )
    return ' and '.join(res)

for x in xrange(1, 1000 + 1):
     print x, make_word(x)

Is it possible to rewrite the function a little bit cleaner? Maybe decrease a number of branches somehow

rewrite the function a little bit cleaner

A way to achieve it, would be to divide your code into simpler functions, for instance:

lst = [0] * 3
for i in xrange(3):
    div = 10 ** (2 - i)
    lst[i] = num / div
    num %= div

Could be a simple function call:

lst = split_decimals(num)

(btw the name lst is not a very explicit...)

  Maybe decrease a number of branches somehow

To clean your branching, I would advise to make a function that treats one part of the problem: numbers between 1 and 99, with a function under_hundred_as_word for instance.

Better variable names might also help the reader to understand your code.

After some further refactoring, this is what I came with:

units = [
    'zero', 'one', 'two', 'three',
    'four', 'five', 'six',
    'seven', 'eight', 'nine',
]
tens = [
    '', '', 'twenty', 'thirty', 'forty',
    'fifty', 'sixty', 'seventy',
    'eighty', 'ninety',
]

hundred_exceptions = {
        10: 'ten',
        11: 'eleven',
        12: 'twelve',
        13: 'thirteen',
        15: 'fifteen',
        18: 'eighteen',
}

def under_hundred_as_word(num):
    assert 0 < num < 100

    unit = num % 10
    ten = num / 10
    if ten == 0:
        return units[unit]
    elif ten == 1:
        if num in hundred_exceptions:
            return hundred_exceptions[num]
        else:
            return units[unit] + 'teen'
    else:
        if unit:
            return tens[ten] + '-' + units[unit]
        else:
            return tens[ten]

def make_word(num):
    assert 0 < num <= 1000
    if num == 1000:
        return 'one thousand'

    parts = []

    # two last digits
    if num % 100:
        parts.insert(0, under_hundred_as_word(num % 100))

    # hundred digit
    hundreds = (num / 100) % 10
    if hundreds:
        hundreds = units[hundreds] + ' hundred'
        parts.insert(0, hundreds)

    return ' and '.join(parts)

for x in xrange(1, 1000 + 1):
    print x, make_word(x)

It can be easily modified to print numbers greater than 1000.


Dragable infinite loop carousel, performance issues on dragging large number (25+) of slides I have two instances of this carousel to test with, one with 7 slides and the other with 25. They run the exact same code. The 7 slide instance drags smoothly forever as intended, but the 25 slide instance is choppy when dragging.

Something in the dragMove event is slowing down execution when the slide count is higher, and I'd like to find a way to improve it so that the slide count has no effect on performance.

I've already tried a few optimizations, like replacing:

$slider.children().last()

with

$Slider[0].lastChild

To bypass instantiating of all those unnecessary Jquery objects just to get the last one, but dragging is still choppy.

Here's the dragMove event:

function dragMove(event){
  var ev = event.originalEvent || event || window.event;
  var MousePosition = GetEventCoordinates(ev);
  var MouseMoveDistance = (MousePosition.x-locals.StartingMousePosition.x);
  var DistanceDifference = (locals.LastMouseMoveDistance-MouseMoveDistance);
  var MoveDirection = (DistanceDifference === 0 || isNaN(DistanceDifference)) ? locals.LastMoveDirection : (DistanceDifference>0) ? '<' : '>' ; // negative to the left
  var StepsMoved = Math.floor(MouseMoveDistance/base.protected.ItemWidth);
  var StepsDifference = (locals.LastStepsMoved-StepsMoved);

  if(StepsDifference !== 0 || locals.LastMoveDirection !== MoveDirection){
     // Steps has changed, recalculate!
     var oEdgeCounts = base.GetEdgeCounts();

     if(oEdgeCounts.left<1 && MoveDirection==='>'){
        // There are non to the left, but we need some!
        if(StepsDifference===0){StepsDifference=1;}
        for(var i=0; i<Math.abs(StepsDifference); i++){
           $Last = $(base.protected.$Slider[0].lastChild);
           base.protected.$Slider.prepend($Last);
        }
        locals.PopAdjustment = (locals.PopAdjustment+(Math.abs(StepsDifference)*base.protected.ItemWidth));
     }

     if(oEdgeCounts.right<1 && MoveDirection==='<'){
        //There are none to the right, but we need some!
        if(StepsDifference===0)StepsDifference=1;
        for(var i=0; i<Math.abs(StepsDifference); i++){
           $First = $(base.protected.$Slider[0].firstChild);
           base.protected.$Slider.append($First);
        }
        locals.PopAdjustment = (locals.PopAdjustment-(Math.abs(StepsDifference)*base.protected.ItemWidth));
     }
  }

  locals.NewSliderLeft = GetEventCoordinates(ev).x - locals.DragStartTouchOffsetX-locals.PopAdjustment;

  // Save values so we can compare on next event
  locals.LastSampleMousePosition = MousePosition;
  locals.LastMouseMoveDistance = MouseMoveDistance;
  locals.LastMoveDirection = MoveDirection;
  locals.LastStepsMoved = StepsMoved;

  base.protected.$Slider.css('left',locals.NewSliderLeft);
}

Here is the get Edge count function. It uses simple math to determin the number of slides off the left and off the right sides. I don't think it's speed is impacted by slide count.

GetEdgeCounts = function(){
   var base = this;
   var SliderLeft = parseInt(base.protected.$Slider.css('left'),10);
   var OffLeftCount = Math.abs(SliderLeft)/base.protected.ItemWidth;
   var OffLeftFloor = Math.floor(OffLeftCount);
   var OffRightCount = base.protected.TotalItems-base.protected.ItemsPerPage-OffLeftCount;
   var OffRightFloor = Math.floor(OffRightCount);

   OffRightFloor = (OffRightFloor >=0) ? OffRightFloor : 0;
   OffLeftFloor = (OffLeftFloor >=0) ? OffLeftFloor : 0;
   return{
      left:OffLeftFloor,
      right:OffRightFloor,
      center:base.protected.ItemsPerPage
   };
}

I'm looking for any ideas on how to make dragging with larger slide counts less choppy.

From a once over, some performance tips:

At least on some versions of Chrome, parseInt(string,10); is slower than a number of alternatives, my preference is +string
It is faster to call Math.abs(string) than Math.abs(parseInt(string,10))
Each object property lookup has a cost, so consider var base = this.protected; since all your object accesses are on `base.protected'
Be wary of ternary operators, a regular if tends to be faster
Your code has actually a number of little inefficiencies

You will capture var MousePosition = GetEventCoordinates(ev); but then later you still call locals.NewSliderLeft = GetEventCoordinates(ev).x - locals.DragStartTouchOffsetX-locals.PopAdjustment;
You store locals.NewSliderLeft but you use it only once in the assignment here: base.protected.$Slider.css('left',locals.NewSliderLeft);, I would skip storing this value in locals altogether.

From a style perspective, I would suggest that you use more spaces and fewer parentheses so var MouseMoveDistance = (MousePosition.x-locals.StartingMousePosition.x); becomes
var MouseMoveDistance = MousePosition.x - locals.StartingMousePosition.x;
Also prefixing in general is discouraged (oEdgeCounts)
From a DRY perspective, the block of if(oEdgeCounts.left<1 && MoveDirection==='>'){ and if(oEdgeCounts.right<1 && MoveDirection==='<'){ is pretty much copy pasted, you should refactor that

All in all, your code is quite readable and maintainable and I dont see immediately where the culprit is. I fully endorse the opion of Doug Gale to use the Chrome developer tools to find the culprit.


A faster way to combine two SQL table I would like to give it a try and ask about my codes on here from now on to see if there are improvement for'em

this code is supposed to combine two sql tables into one.

SELECT * FROM (SELECT * FROM posts WHERE id < $lastpost AND position = submitter order by id DESC LIMIT 5) t1 JOIN (SELECT username, firstname, lastname, avatar FROM users) t2 ON t1.submitter = t2.username

I was thinking that there might be a better way doing it using union or anything faster, if I'm even right about what I'm saying.

Assuming that each post has exactly one submitter, this is a more natural formulation of the query.

SELECT posts.*
     , users.username
     , users.firstname
     , users.lastname
     , users.avatar
    FROM posts
        JOIN users
            ON posts.submitter = users.username
    WHERE
        posts.id < $lastpost
        AND posts.position = posts.submitter
    ORDER BY posts.id DESC
    LIMIT 5

However, its performance is likely to be similar to your original query.  To investigate performance issues, run EXPLAIN SELECT to help you verify that the necessary indexes are in place.


Enhancing parser for 8 byte command string (expect problems with type conversions) for Arduino/Radio communication Because the throughput of my radio and device is limited, I wanted to compress my control string. I decided to compress the command into an 8 bytes (quasi) string (throttle high, throttle low, pitch, roll, yaw plus/minus, yaw value, checksum, stop byte) and after parsing in four variables:

Pitch (between -45° and 45°)
Roll (between -45° and 45°)
Yaw (between -180° and 180°)
Throttle (between 1000 and ~1900)

Because Some number ranges don't fit into a single byte (0 to 255 number range), I used two instead (throttle: one byte for hundreds (1-9) and one byte for tenners (1-99); Yaw: One byte for plus or minus and one byte for 0° - 180°). I send and read these bytes as characters. Then I convert them into numbers (0-255).

I noticed that sometimes a few reads are totally out of range (e.g. value: -31001) but checksum test is OK. I don't know exactly why (maybe radio, maybe I caused unexpected behavior because of type conversions here). This is why I made an additional range test. Now the remote control seems to be reliable.

However I don't feel very nice about it, because I do signed/unsigned and! number range conversions, the checksum function is also not very reliable. Maybe someone has suggestions how to make the code better.

Code (The command stored in buffer is already red correctly until the stop byte and valid in length):

bool Receiver::parse_radio(char *buffer) {
  int16_t throttle_high  = (uint8_t)buffer[0];       // 1. 1 - 9
  int16_t throttle_low   = (uint8_t)buffer[1];       // 2. 0 - 99
  int16_t pit            = (uint8_t)buffer[2] - 127; // 3. -45° - 45°
  int16_t rol            = (uint8_t)buffer[3] - 127; // 4. -45° - 45°
  int16_t yaw_pm         = (uint8_t)buffer[4] - 127; // 5. -1 || +1
  int16_t yaw_val        = (uint8_t)buffer[5];       // 6. yaw angle in degrees 180°

  int16_t chk            = (uint8_t)buffer[6];       // 7. checksum

  int16_t thr            = 1000 + (throttle_high * 100) + throttle_low;
  int16_t yaw            = yaw_val * yaw_pm;

  // Calculate checksum
  uint8_t checksum = 0;
  for(int i = 0; i < 6; i++) {
    checksum = (checksum + buffer[i]) << 1; // % 256 can be saved because of the uint8_t
  }

  // Validity check:
  // First checksum
  if(checksum != chk)
    return false;
  // Later other values
  if(yaw_pm < -1 || yaw_pm > 1)
    return false;
  if(rol > 45 || rol < -45)
    return false;
  if(pit > 45 || pit < -45)
    return false;
  if(yaw > 180 || yaw < -180)
    return false;
  if(thr > RC_THR_80P || thr < RC_THR_MIN)
    return false;

  // Do rest here
  return true;
}

I don't know exactly why (maybe radio, maybe I caused unexpected behavior because of type conversions here).

Can you keep statistics on how many checksum errors there are?

If there are many that implies that the communications link is noisy
If there are few or none then it's more probably something else (e.g. a software error)

I'm not sure that you're using a good checksum algorithm.; you might like to use a CRC instead.

You're only using 7 of your 8 bytes so you could have a 2-byte CRC (or checksum).

You could compress your data better:

'1 - 9'
'0 - 99' => 10 bits to store 0..999 throttle
'-45° - 45°' => 7 bits to store 90 angle values
'-45° - 45°' => 7 bits to store 90 angle values
'-1 || +1'
'yaw angle in degrees 180°' => 9 bits to store 360 angle values

10+7+7+9 => 37 bits = 5 bytes.

Alternatively 999 * 90 * 90 * 360 = 2913084000 which is less than 232. So you could fit your data into 4 bytes and use a 4-byte CRC.


Rock Paper Scissors in JavaScript In a few weeks I need to give a workshop about JavaScript to colleagues. They do not have much experience with JavaScript so I will explain the basics of JavaScript. At the end of the day I want them to make a small game with the aspects learned during the day.

I was thinking of Rock Paper Scissors. Below you can find the final code for this game:

var RockPaperScissors = (function () {
    var t = 'Tie',
        c = 'Computer wins',
        p = 'Player wins',
        winningMap = [
            [t, c, p],
            [p, t, c],
            [c, p, t]
        ],
        choices = ["Rock", "Paper", "Scissors"];

    var getComputerChoice = function () {
        return Math.floor(Math.random() * 3);
    };

    var getWinner = function (playerChoice, computerChoice) {
        return winningMap[playerChoice][computerChoice];
    };

    function RockPaperScissors(playerChoice) {
        this.playerChoice = playerChoice;
        this.computerChoice = getComputerChoice();
        this.winner = getWinner(this.playerChoice, this.computerChoice);
    }

    RockPaperScissors.prototype.toString = function () {
        return this.winner + " [Computer: " + choices[this.computerChoice] + ", Player: " + choices[this.playerChoice] + "]";
    };

    return RockPaperScissors;

})();

This code is executed everytime a select input box changes (see this as the user input):

$('#choice').on('change', function () {
    var $input = $(this);

    if ($input.val() === '-1') return;

    var rockPaperScissors = new RockPaperScissors($input.val());

    $('#message').html(rockPaperScissors.toString());
});

I've also set up a Fiddle.

Do you think this code is clear for beginners? Is this too advanced? I'm not sure if I should introduce them to prototyping or not.

The definition of the winningMap is very confusing.

I suggest a different form of input:

var winningMap = {Rock: "Scissors", Paper: "Rock", Scissors: "Paper"};

It defines which choice can be beaten by a specific selection.
If both inputs are the same, it's obvious a tie:

var getWinner = function (playerChoice, computerChoice) {
    if(playerChoice === computerChoice){
        return "Tie";
    }
    if(computerChoice === winningMap[playerChoice]){
        return "Player Wins";
    }
    return "Computer Wins";
};

Notice, that I didn't use the string-definitions. I don't think that it's necessary, and it only makes the code a bit more complicated.

In order to convert the computer's choice to an actual string (Rock, Scissors, Paper), you can use an Array:

var stringMapping = ["Rock", "Scissors", "Paper"];

var getComputerChoice = function () {
    return stringMapping[ Math.floor(Math.random() * 3) ];
};

Maybe a new-Operator would be understood easier

The closure might be confusing for a beginner, maybe you should consider to write the Object this way:

function RockPaperScissors (){
     var getComputerChoice = function(){ ... };    //= private variable
     this.getWinner = function(){ ... };           //= public variable
};

var actualObject = new RockPaperScissors();
actualObject.method();
actualObject['method']();

If your colleagues are used to object oriented programming, this style might be more familiar.

Always use brackets

if ($input.val() === '-1') { return; }

I personally don't mind conditions within one line, but dropping the brackets is bad practice - and can lead to strange behaviours, especially in JavaScript.

Furthermore, I wonder why u used the dollar-sign at the beginning of the $input-variable?

Edit:

I share @Barts's opinion from the comments - it's not neccessary to use jQuery in this example. You should show them jQuery once they know the basic principles of JavaScript.


Poor man's f-strings in Python 2.6 f-strings is a new feature in the upcoming python release which basically lets you do this:

>>> s = 'qwerty'
>>> f'{s} {1+2}'
'qwerty 3'

Not pleased at having to wait until f-strings are widely supported, I came up with the following function, which should work for any Python version starting with 2.6.

def f(s):
    return s.format(**inspect.currentframe().f_back.f_locals)

>>> s = 'qwerty'
>>> f('{s} abc')
'qwerty abc'

Switching to f-strings in the future should be as easy as removing the two parentheses.

Of course, my function doesn't support the more advanced features of f-strings (like evaluation of any expression).

My questions are:

If I start widespread use of this function in my code, is there a chance this bites me in the future? Is there anything objectionable in my current implementation?
How would you implement a more advanced f-function, supporting evaluation of arbitrary expressions? Preferably without having performance suffer too much - my benchmark shows the f-function is only slightly slower than directly calling format.

Python version 2.6.

The thing most likely, in my mind, to cause problems here is that using frames is not guaranteed to work in any other Python implementation besides CPython.

inspect.currentframe()
Return the frame object for the caller’s stack frame.
CPython implementation detail: This function relies on Python stack frame support in the interpreter, which isn’t guaranteed to exist in all implementations of Python. If running in an implementation without Python stack frame support this function returns None.

This will end up throwing an error if you try to unpack None because the implementation doesn't support your function. If you were dead-set on using stack inspection, then you should give a more useful error message than AttributeError: 'NoneType' object has no attribute 'f_back'. You should test for if you get a frame or not, and then use platform to print a more helpful error message.
def f(s):
    frame = inspect.currentframe()
    if frame is not None:
        return s.format(**frame.f_back.f_locals)
    else:
        raise NotImplementedError(
            "Python implementation {} does not support stack inspection.".format(
                platform.python_implementation()
            )
        )

At this point, then, you should make sure this is a well-documented limitation of your library, and I would probably write some unit tests where it asserts that is raised for certain implementations.
The real problem, however, isn't that you're using something that isn't guaranteed (although that's certainly a problem). The real problem is that stack inspection is almost never the right answer. We don't need it here either.  What we're going to want to do is something like this:
def _poor_fstring(namespace):
    def _():
        return namespace
    return _

get_namespace = _poor_fstring(locals())

def f(s):
    return s.format(**get_namespace())

This still isn't ideal - you'll need to redeclare get_namespace in any scope you want access to it - but you avoid stack inspection, which is worth it in my mind.
Really though, I don't see a huge benefit between using your function and just calling s.format(**namespace), where namespace is whatever lookup table you want.  The one benefit would be if, as you said, it supported arbitrary expressions. And the only way to get arbitrary expressions is to, well, evaluate them.  You'll basically have to parse the string and extract everything between curly braces. Then try a lookup in the namespace and replace that, otherwise compile it, and then eval what you find. Hopefully I don't need to point out that evaling random strings is... dangerous.
Going into actually parsing a format string is way beyond the scope of this question.
In short, my recommendation would be to just suffer through... or upgrade your Python version.


Animating rotations using jQuery I am writing a function for a rotating navigation with JS and jQuery. I am using a plugin called jQuery transit for animating rotation. The code works, but it is very long for such a simple function. Is there any way this can be improved?

var rotation = 0;
$('nav ul li a').bind('click', function() {
'use strict';
var item = $(this).parent();
var itemI = item.index();
var navAll = $(this).parents('ul');

var condition = rotation + '_' + itemI;

switch(condition)
{
   case '0_0':
        //do nothing
        break;
    case '0_1':
        navAll.transition({rotate: '-=90deg'});
        $('nav ul li a').transition({rotate:'90deg'});
        rotation = 270;
        break;
    case '0_2':
        navAll.transition({rotate: '-=180deg'});
        $('nav ul li a').transition({rotate:'180deg'});
        rotation = 180;
        break;
    case '0_3':
        navAll.transition({rotate: '+=90deg'});
        $('nav ul li a').transition({rotate:'-90deg'});
        rotation = 90;
        break;

    case '90_0':
        navAll.transition({rotate: '0deg'});
        $('nav ul li a').transition({rotate:'0deg'});
        rotation = 0;
        break;
    case '90_1':
        navAll.transition({rotate: '-90deg'});
        $('nav ul li a').transition({rotate:'90deg'});
        rotation = 270;
        break;
    case '90_2':
        navAll.transition({rotate: '+=90deg'});
        $('nav ul li a').transition({rotate:'-180deg'});
        rotation = 180;
        break;
    case '90_3':
        break;

    case '180_0':
        navAll.transition({rotate: '0deg'});
        $('nav ul li a').transition({rotate:'0deg'});
        rotation = 0;
        break;
    case '180_1':
        navAll.transition({rotate: '-90deg'});
        $('nav ul li a').transition({rotate:'90deg'});
        rotation = 270;
        break;
    case '180_2':
        //do nothing
        break;
    case '180_3':
        navAll.transition({rotate: '-=90deg'});
        $('nav ul li a').transition({rotate:'-90deg'});
        rotation = 90;
        break;

    case '270_0':
        navAll.transition({rotate: '0deg'});
        $('nav ul li a').transition({rotate:'0deg'});
        rotation = 0;
        break;
    case '270_1':
        //do nothing
        break;
    case '270_2':
        navAll.transition({rotate: '-=90deg'});
        $('nav ul li a').transition({rotate:'180deg'});
        rotation = 180;
        break;
    case '270_3':
        navAll.transition({rotate: '-=180deg'});
        $('nav ul li a').transition({rotate:'-90deg'});
        rotation = 90;
        break;

}

});

Instead of that complex switch statement, I'd just use an option object:

var allLinks = $('nav ul li a');
var rotation = 0;

$('nav').on('click', 'ul li a', function() {
    'use strict';
    var $this = $(this);
    var item = $this.parent();
    var navAll = $this.parents('ul');

    var option = {
        '0_1': {
            rotateAll: '-=90deg',
            rotateThis: '90deg',
            rotation: 270
        },
        '0_2': {
            rotateAll: '-=180deg',
            rotateThis: '180deg',
            rotation: 180
        }
        // Add them all here
    }[ rotation + '_' + item.index() ];

    if ( option ) {
        navAll.transition({rotate: option.rotateAll});
        allLinks.transition({rotate: option.rotateThis});
        rotation = option.rotation;
    }
}

There are 2 more things to consider:

Are you sure you really need that nav ul li a selector? Wouldn't nav a do the job just the same? While we're at it, why are you listening to the click on the a elements, and then find the li via .parent()? Can't you just listen to the click event on the lis themselves?
Is there any method to that configuration? I wasn't able to deduce any algorithm from what you've provided, but I'm sure there is. You'd be better off calculating it on the fly, if possible.


Calculate average from .txt file I have made an exercise where I need to calculate the average per row of a .txt file. (see below)

Toa Narumi gradeA 10 8 7 4 6,5
Jean-François Le Clerk gradeB 5 4 7
Joe GradeC 10 10

I only want the numbers behind the name and grade.
This is my code:

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;
using System.IO;
using System.Linq;

namespace RenoD_Oef_Strings
{
    class Program
    {
        static void Main(string[] args)
        {
            // Variablen
            StreamReader bestand = new StreamReader(@"F:\RenoD_Oef_Strings\Punten.txt");
            string strRow; // String used to read every row of .txt file
            double?[] dblPoints; // Double array used to store all values
            List<string> strInput = new List<string>(); // String List used to store all data from .txt

            // Code
            while ((strRow = bestand.ReadLine()) != null) // Read row per row in .txt
            {
                strInput = strRow.Split(' ').ToList(); // Puts every word in the row in to the list

                int intX = 0;
                foreach(string strX in strInput)    // Calculate how many strings are in the list
                {
                    intX++;
                }

                dblPoints = new double?[intX];  // Calculate the max number of elements the double array can have

                intX = 0;
                foreach(var x in strInput)  // Checks if elements in the list can be converted in to a double
                {
                    try
                    {
                        double dblR = Convert.ToDouble(x);  // If element can be converted in to a double, it will be stored in the double array

                        dblPoints[intX] = dblR;
                        intX++;
                    }
                    catch
                    {
                        intX++; // If element CAN NOT be converted in to a double, it will be still be stored in the double array but without any value
                    }
                }

                double dblAverage = 0;   // Double used to save the total average of one row
                intX = 0;
                foreach(var k in dblPoints)
                {
                    if (k.HasValue) // Elements without value will be ignored
                    {
                        dblAverage += Convert.ToDouble(k); // All double values will be added up
                        intX++; // Used to see how much double values there are to calculate average
                    }
                }

                dblAverage = Math.Round(dblAverage/intX, 2);  // Calculate average + round up to two decimals

                Console.WriteLine(dblAverage.ToString());
            }

            bestand.Close();

            Console.ReadKey();
        }
    }
}

What I have done is

Read row per row .txt file
Put all elements of that row in a list
Calculate how many elements that the double array will need to store (same number of elements the list has), which I later on use to store all doubles of the list
Check if elements in the list can be converted in to a double array; If the element can be converted in to a double, it will be stored in the double array.

If element CAN NOT be converted in to a double, it will be still be stored in the double array but without any value.

Only calculate the average of the elements in my double array that have values.

I have tested the code, and it works without any issue.

My question is, have I taken a good approach? What would you have done to make it more efficient?

I have tried searching in other threads for solutions. I have seen simular questions, but was unable to find any exactly the same as my exercise.

Enclose the file reading in a using statement
Use double.TryParse or regex to parse the numbers
Use Linq extention method Average() to calculate average
The size of strInput is strInput.Count, no need for a loop

Hear is a simpler code:

using (StreamReader bestand = new StreamReader(@"F:\RenoD_Oef_Strings\Punten.txt"))
{
    string strRow; // String used to read every row of .txt file
                    // Code
    while ((strRow = bestand.ReadLine()) != null) // Read row per row in .txt
    {
        List<string> strInput = strRow.Split(' ').ToList(); // String List used to store all data from .txt
        List<double> dblPoints = new List<double>(); // Double list used to store all values

        foreach (var x in strInput)  // Checks if elements in the list can be converted in to a double
        {
            if (double.TryParse(x, out double result))
            {
                dblPoints.Add(result);
            }
        }
        double dblAverage = dblPoints.Average();

        Console.WriteLine(dblAverage.ToString());
    }
}

Console.ReadKey();


REST API to list books, where null means "everything" My company has a REST API, and one design issue that has come up a few times is that we use null to indicate "get everything".

For example /api/books?genre=romance will get all romance books, but /api/books will return all books from all genres.

The current technique used in the code is something like this

public ResponseEntity getBooks(@RequestParam(required = false) String genre) {

    List<BookDto> bookDtos = new ArrayList<>();
    List<Book> allBooks = getAllBooks();

    for (Book book : allBooks) {
       // This if condition is what I take issue with
       if (genre == null || book.genre.equals(genre)) {
           bookDtos.add(book.convertToDto());
       }
    }
    return bookDtos;

}

I think it is bad practice to use null here to convey the meaning of get everything. Is this correct?

My current solution is to distinguish between these two cases at the very top level like so:

public ResponseEntity getBooks(@RequestParam(required = false) String genre) {

    if (genre == null) {
        return getAllBooks();
    } else {
        return getBooks(genre);
    }
}

And then a few supporting functions that funnel all of the logic into the same place by massaging the inputs a little bit.

// This is the core code, that takes a list of genres and returns all books from all those genres.
public Collection<BookDto> getBooks(List<String> genre) {
    // And here is the code that will actually get the books from the database.
    // ...
    // ...
}

// This is a supporting function, that finds all genres and funnels that into the above function.
public Collection<BookDto> getAllBooks() {
    List<String> allGenres = getAllGenres()
    return getBooks(allGenres);
}

// This is a supporting function that builds a list from a single genre, to again funnel into the 1st function.
public Collection<BookDto> getBooks(String genre) {
    List<String> genres = new ArrayList<>();
    genres.add(yearCode);
    return getBooks(genres);
}

The idea here is to have a single place where the work actually takes place and the database is accessed, but to have multiple ways to access it. And more importantly to remove the idea of null == all in the codebase, aside from the very top level where the execution starts after the user requests /api/books.

It's always felt natural to me but I've not seen it used by anyone else (I've only worked at a few places, so this could just be inexperience on my part). Is the proposed design good? Is there a standard way of solving this problem?

I personally think it's fine, but it helps to turn your thinking around from a "selector" to a "filter". Then, the default is to return all books, and you selectively filter down the set you're going to eventually respond with.

(Note I haven't written Java in a long time, so grain of salt here,) I might specify a BookFilter object which can incrementally have fields set from the parameters, and then have BookDTO.filterBooks. Then, your request method becomes:

public ResponseEntity getBooks(@RequestParam(required = false) String genre) {
    BookFilter filter = new BookFilter();
    if (genre != null) {
      filter.setGenre(genre);
    }

    return filterBooks(filter);
}

I know it's not directly applicable (your DTO probably doesn't work this way, based on what you described above), but I think what you're doing with the null check in your controller is pretty much the same thing with the tools you've got.

You might consider trying to refactor your DTO so it can work with filters however, because eventually you may want ?genre=scifi&published=1970+&publisher=penguin etc. etc., at which point it'll be a lot easier to manage a single filter object than a bunch of explicit queries.


Excel VBA used to fetch Access data I am using the following VBA to fetch data from an Access database. The query works but there is a lot of information in the database and the response time is very slow, about 30 seconds.

Is there any part of the code that I could optimise or write differently?

Sub test()

Dim strsql1, tr As String
Dim rst As ADODB.Recordset

strsql1 = "SELECT * FROM FP WHERE SUBMISSION_ID = 5683;"

Set cn = New ADODB.Connection
cn.Open "Provider=Microsoft.ace.OLEDB.12.0; Data Source=\\REDACTED.accdb;"

Set rst = New ADODB.Recordset

rst.Open strsql1, cn, adOpenStatic

End Sub

There's nothing to optimise here.

Your steps are:

Open connection

Execute about the simplest SQL query possible

End

There's nothing we can change there that would make any difference.

Now, if you want to post a separate question, detailing your Database Structure, your SQL query, and asking how to optimise it, we could help there.

That aside, thoughts on your code:

You never use tr.

This:

Dim strsql1, tr As String

Does not actually Dim both as strings. When you declare variables, each variable must have a datatype or it will default to Variant.

This is what your code is actually doing, implicitly:

Dim strsql1 [As Variant], tr As String

Your database connection string should be a constant

If you're targeting a database, and you don't need to build the connection string dynamically, you should just have a project constant like so:

Public const FP_DATABASE_CONNECTION_STRING As String = "Provider=Microsoft.ace.OLEDB.12.0; Data Source=\\REDACTED.accdb;"

Now it's in one place, and you know where it is, and if it ever changes, you know where to go to change it.

You should name your query

strsql1 tells me absolutely nothing useful.

selectFpId5683 would be a much better name. Sure, it's a bit verbose, but it actually lets us know what the variable is. And now, when we use it later, we can know that we're using the right thing.

selectFpId5683 = "SELECT * FROM FP WHERE SUBMISSION_ID = 5683;"

And now:

Option Explicit

Public Const FP_DATABASE_CONNECTION_STRING As String = "Provider=Microsoft.ace.OLEDB.12.0; Data Source=\\REDACTED.accdb;"

Function GetAllFpRecordsWithId5683() As ADODB.Recordset

    Dim fpConnection As ADODB.Connection
    Set fpConnection = New ADODB.Connection
    fpConnection.Open FP_DATABASE_CONNECTION_STRING

    Dim selectFpId5683 As String
    selectFpId5683 = "SELECT * FROM FP WHERE SUBMISSION_ID = 5683;"

    Dim recordsWhereId5683 As ADODB.Recordset
    Set recordsWhereId5683 = New ADODB.Recordset
    recordsWhereId5683.Open selectFpId5683, fpConnection, adOpenStatic

    Set GetAllFpRecordsWithId5683 = recordsWhereId5683

End Function

Much, much easier to understand, work with, debug etc.

And then make it even better:

You'll probably want to target more than 1 ID in the future, so why not make your function a general one that can target any ID?

Option Explicit

Public Const FP_DATABASE_CONNECTION_STRING As String = "Provider=Microsoft.ace.OLEDB.12.0; Data Source=\\REDACTED.accdb;"

Function GetAllFpRecordsWithId(ByVal targetID As String) As ADODB.Recordset

    Dim fpConnection As ADODB.Connection
    Set fpConnection = New ADODB.Connection
    fpConnection.Open FP_DATABASE_CONNECTION_STRING

    Dim selectFpId As String
    selectFpId = "SELECT * FROM FP WHERE SUBMISSION_ID = " & targetID & ";"

    Dim recordsWhereId As ADODB.Recordset
    Set recordsWhereId = New ADODB.Recordset
    recordsWhereId.Open selectFpId, fpConnection, adOpenStatic

    Set GetAllFpRecordsWithId = recordsWhereId

End Function


Conditionally selecting random element from array I'm building a clone of 2048 in C with SDL2, and I'm implementing the function that adds a number randomly to an empty cell on the board. The board is representd by a 2-Dim array board[BOARD_SIZE][BOARD_SIZE], and stores the exponent for the square value(10 for 1024 and so on). There must be a 90% chance of the cell being worth 1 (i.e. 2^1) against 10% of it being 2. Here's how I implemented this

void randCell(int _board[BOARD_SIZE][BOARD_SIZE]) {
    int row;
    int col;
    bool has_space = false;
    // Check if there's any empty spaces
    for (int i = 0; i < BOARD_SIZE; ++i) {
        for (int j = 0; j < BOARD_SIZE; ++j) {
            if (_board[i][j] == EMPTY) has_space = true;
        }
    }
    // Randomly pick a cell until we find an empty one
    // TODO: Better way to find empty cells.
    if (has_space) {
        do {
            row = randRange(0, BOARD_SIZE - 1);
            col = randRange(0, BOARD_SIZE - 1);
        } while (_board[col][row]);
        // Fill the cell
        _board[col][row] = (randRange(1, 100) < 90) ? 1 : 2;
    }
}

How can I improve this? Could I somehow decrease the number of loops my while performs by only picking a random cell from the EMPTY ones?

Confusing explanation

According to your explanation, storing 10 in the board means displaying \$2^{10}\$ or 1024 on the screen.  If so, then storing a 1 in the board would display \$2^1\$ or 2, and storing a 2 would display \$2^2\$ or 4.  But this contradicts your program which chooses a new display value of 1 and 2 using board values 1 and 2.

Either you should be generating board values 0 and 1, or your explanation was wrong.  I'm guessing that your explanation was wrong and that storing x in your board represents \$2^{x-1}\$ and not \$2^x\$.

Random number generation off by one

You said you wanted to generate 1 with 90% frequency, but you actually are generating it with 89% frequency.  You need to change your < to a <= to fix that.

One easy way to see that you are off by one is if you change the 90 to a 100.  Then you will see that you still have a 1% chance of generating a 2 instead of a 0% chance.

Deterministic way of choosing empty cell

Instead of randomly picking cells until you get an empty one, you can count the number of empty cells, then pick among them.  For example, if you count 5 empty cells, then you can pick a number between 0 and 4, and then go back through the cells and find the nth empty cell, where n is the random number you picked.

Sample rewrite

Here is an example of how you could implement the above suggestion:

void randCell(int _board[BOARD_SIZE][BOARD_SIZE]) {
    int num_spaces = 0;

    // Count the number of empty spaces.
    for (int i = 0; i < BOARD_SIZE; ++i) {
        for (int j = 0; j < BOARD_SIZE; ++j) {
            if (_board[i][j] == EMPTY)
                num_spaces++;
        }
    }

    if (num_spaces == 0)
        return;

    int rand_space = randRange(0, num_spaces - 1);
    for (int i = 0; i < BOARD_SIZE; ++i) {
        for (int j = 0; j < BOARD_SIZE; ++j) {
            if (rand_space-- == 0) {
                _board[i][i] = (randRange(1, 100) <= 90) ? 1 : 2;
                return;
            }
        }
    }
}


Queue Notifier Extension Script I am trying to keep the progression of the Review Queue Notifier moving forward.  My next big move is to turn it into an extension for Chrome and FireFox and eventually Edge.

I would like the Javascript for the code reviewed again to get me back into the groove of coding this extension/script.

Here are the guts:

$( document ).ready(function() {
    //Public Key
    var publicKey = '?key=hyEwZ8*W*OF7tQ3KYgNjzg((';
    var sites;
    var ACTIVESITES;// = chrome.storage.sync.get(activeSites);

    GetSelectedSites();
    getAllTehSitez();

    function getAllTehSitez() {
        console.log("getAllTehSitez has been called");
        $.getJSON('http://api.stackexchange.com/2.2/sites' + publicKey + '&pagesize=100', function(data) {
            sites = data.items;
            isActiveSite();
        });
    }

    function isActiveSite() {
        console.log("isActiveSite has been called");
        chrome.runtime.sendMessage("getUrl", function(response) {
            var tabUrl = response.url;
            for (var site in sites) {
                if ((tabUrl == sites[site].site_url + '/review')
                    && (ACTIVESITES.indexOf(sites[site].name.toLowerCase()) > -1)) {
                        runRQN();
                        return;
                }
            }
        });
    }

    function GetSelectedSites () {
        console.log("GetSelectedSites has been called");
        chrome.storage.sync.get({
            activeSites: "Code Review"
        }, function(item) {
            ACTIVESITES = item.activeSites;
        });
    }

    function runRQN () {
        console.log("runRQN has been called")
        Notification.requestPermission();
        var DELAY =  300 * 1000; //120,000 milliseconds = 2 Minutes
        function getDelayAmount() {
            chrome.storage.sync.get({
                refreshRate: 300000
            }, function(item){
                DELAY = item.refreshRate;
            });
        }
        getDelayAmount();

        setTimeout(function(){
            window.location.reload();
        }, DELAY);

        console.log(DELAY);
        var notificationTitle = (document.title.split(' - ')[1] + ' Review Queue').replace(' Stack Exchange', '.SE');
        var reviewCount = 0;
        var reviewItems = document.getElementsByClassName('dashboard-num');

        for (var i = 0; i < reviewItems.length; i++){
            if (reviewItems[i].parentNode.className != 'dashboard-count dashboard-faded'){
                reviewCount += parseInt((reviewItems[i].getAttribute("title")).replace(',', ''), 10);
                console.log(reviewItems[i]);
            }
        }
        console.log(reviewCount);
        var image = chrome.extension.getURL('Icon2.jpg');
        if (reviewCount > 0) {
            var details = {
                body: reviewCount + ' Review Items',
                icon: image
            }
            var n = new Notification(notificationTitle, details );
            setTimeout(n.close.bind(n), 100000); // Magic number is time to notification disappear
        }
    }
});

Stack Apps Question

GitHub Repository

This is a follow-up to the following questions, kind of...

First Question

Second Question

Foreword

I know that this was posted nearly 1.5 years ago and the code appears to have changed dramatically and perhaps you've already learned about what I will mention below, but I feel like this question needs an answer...

Feedback

I like the good use of the Notification interface, as well as the radix passed to parseInt().

Suggestions

The code to check if the site is in the list of active sites, i.e.

if ((tabUrl == sites[site].site_url + '/review')
            && (ACTIVESITES.indexOf(sites[site].name.toLowerCase()) > -1)) {

can be simplified using Array.prototype.includes(), which has been supported since FF 43 (released 12/15/2015) and Chrome 47 (released 12/01/2015):

if ((tabUrl == sites[site].site_url + '/review')
            && (ACTIVESITES.includes(sites[site].name.toLowerCase()))) {

The reload registration could be simplified from:

setTimeout(function(){
    window.location.reload();
}, DELAY);

To the following, which makes a partially applied function using Function.bind():

setTimeout(window.location.reload.bind(), DELAY);

that way there is no excess lambda/anonymous function, and two fewer lines (I get the impression you might like one-liners)!
The selection of dashboard elements could be simplified, using a more selective CSS selector:

var reviewItems = document.getElementsByClassName('dashboard-num');
for (var i = 0; i < reviewItems.length; i++){
    if (reviewItems[i].parentNode.className != 'dashboard-count dashboard-faded'){

Instead, one could utilize the :not() selector and document.querySelectorAll() (or jQuery selector function, though that would return a jQuery collection instead of a NodeList):

var reviewItems = document.querySelectorAll('.dashboard-num:not(.dashboard-count):not(.dashboard-faded)');

One could utilize promises for the asynchronous callbacks...


Hacker Rank - Poisonous Plants I have the solution for the problem, but it is taking me 7 seconds to run on a large dataset. I am trying figure out a better way of doing this. It has to run in under 3 seconds.

Problem Statement:

There are N plants in a garden. Each of these plants has been added with some amount of pesticide. After each day, if any plant has more pesticide than the plant at its left, being weaker than the left one, it dies. You are given the initial values of the pesticide in each plant. Print the number of days after which no plant dies, i.e. the time after which there are no plants with more pesticide content than the plant to their left.

Explanation:

Initially all plants are alive.

Plants = \$\{(6,1), (5,2), (8,3), (4,4), (7,5), (10,6), (9,7)\}\$
Plant = \$(i,j)\$ \$\Rightarrow\$ \$j\$th plant has pesticide amount \$i\$.
After the 1st day, 4 plants remain as plants 3, 5, and 6 die.
Plants = \$\{(6,1), (5,2), (4,4), (9,7)\}\$
After the 2nd day, 3 plants survive as plant 7 dies.
Plants = \$\{(6,1), (5,2), (4,4)\}\$
After the 3rd day, 3 plants survive and no more plants die.
Plants = \$\{(6,1), (5,2), (4,4)\}\$

I added the test case to this Pastebin.

static void Main(string[] args)
{
    Stopwatch timer = new Stopwatch();
    timer.Start();
    string data = "";// Console.ReadLine(); //"6 5 8 4 7 10 9";

    foreach (var line in File.ReadLines(@"FilePath"))
    {
        data = line;
    }

    bool happyState = false;
    bool plantDied = false;
    int numberOfDays = 0;

    LinkedList<int> lstPlants = new LinkedList<int>(data.Split(new char[] { ' ', '\t' }).Select(s => int.Parse(s)).ToList());

    LinkedListNode<int> lastNode = lstPlants.Last;
    LinkedListNode<int> currentNode = lstPlants.First;
    int cValue = currentNode.Value;

    while (!happyState)
    {
        numberOfDays++;
        plantDied = false;
        happyState = false;
        lastNode = lstPlants.Last;
        currentNode = lstPlants.First;
        cValue = currentNode.Value; //bug fix
        while (currentNode != lstPlants.Last)
        {
            if (currentNode.Next.Value > cValue)
            {
                plantDied = true;

                cValue = currentNode.Next.Value;
                lstPlants.Remove(currentNode.Next);
                //currentNode = currentNode.Next;
            }
            else
            {
                currentNode = currentNode.Next;
                cValue = currentNode.Value;
            }
        }

        if (!plantDied)
        {
            numberOfDays--;
            happyState = true;
        }

    }
    timer.Stop();
    Console.WriteLine("Time taken to calculate is {0} seconds and the answer is {1}.", timer.Elapsed.TotalSeconds, numberOfDays);
    Console.ReadLine();
}

Let's start by analysing the current algorithm.

Each day that plants are dying, we iterate through all living plants. So the worst case would be if only one plant dies each day. In this case, we can see that the algorithm will take \$O(N^2)\$ time.

For example, the input sequence \$ 1, 2, 2, 2, 2, \ldots, 2\$ will exhibit this quadratic behaviour.

From the problem statement, \$1 \leq N \leq 10^5\$, so we will have to do better than \$O(N^2)\$.

We start with the easy case, when the current plant has more pesticide in it than the one to its immediate left. Then we know that the current plant dies after one day.

Otherwise, we look for the right-most plant to the left of us that has less pesticide than the current plant.

If there is no such plant then we know that the current plant will never die.

So suppose there is such a plant, and let's say it's at index \$i\$. I claim that the current plant (at index \$k\$) will die on day
$$\max \left\{ d[j] \,\middle|\, i < j < k \right\} + 1,$$
where \$d[j]\$ is the day on which plant \$j\$ dies.

What this means is that for plant \$k\$ to die, I have to wait for all the plants between plant \$i\$ and plant \$k\$ to die, at which point \$i\$ will be to the immediate left of \$k\$, and then I have to wait one more day.

So far, we still have an \$O(N^2)\$ algorithm. If we think about the input \$ 1, 2, 2, 2, 2, \ldots, 2\$, for every plant we have to iterate all the way back to the start to find the plant with less pesticide in it.

Let's think about the following input:

$$ 2, 5, 1, 10, 3, 11 $$

The first plant will never die, and the second plant will die on day one.

For the third plant, we look at the previous two values and see that they both have more pesticide than the third, so the third plant will never die. The key here is that we can now forget that the first and second plants ever existed.

Why? Because any plant waiting for the first or second plant to die will have to wait for the third plant to die first.

The fourth plant dies on day one.

For the fifth plant, we look at the previous two values, 1 and 10. The fifth plant will die a day after the 10, and again we can forget the 10 ever existed, because any plant waiting for the 10 to die will have to wait for the 3 to die first.

Spoilers

How would this look in code? Suppose we have a stack containing pairs of the amount of pesticide in the plant and the day on which the plant dies.

If we are in the interesting case, where the plant doesn't die on day one, then we keep popping elements off the stack while they have at least as much pesticide as the current plant. While we're popping elements, we keep track of the maximum number of days it takes for these plants to die.

If we have popped all elements off the stack, the current plant will never die. Otherwise, the plant will die on the maximum number of days that we've seen, plus one.

One we have calculated the number of days it takes for the plant to die, we push it onto the stack.

Since exactly \$N\$ elements are pushed onto the stack, we cannot have more than \$N\$ pops. So we can see that this algorithm is \$O(N)\$.


Find and copy .tiff files My code works as expected but I am not sure how to optimize the code. The purpose of the code is to find files with given parameters and copy them to a separate folder.

import java.io.File;
import java.io.IOException;
import org.apache.commons.io.FileUtils;
import javax.swing.JOptionPane;
import java.io.File;
import java.io.IOException;
import java.awt.Frame;
import java.awt.image.RenderedImage;
import javax.media.jai.widget.ScrollingImagePanel;
import javax.media.jai.NullOpImage;
import javax.media.jai.OpImage;
import com.sun.media.jai.codec.SeekableStream;
import com.sun.media.jai.codec.FileSeekableStream;
import com.sun.media.jai.codec.TIFFDecodeParam;
import com.sun.media.jai.codec.ImageDecoder;
import com.sun.media.jai.codec.ImageCodec;

public class FileSearch {

    int sizeOfFile = Integer.parseInt((JOptionPane.showInputDialog(null,"Enter Size: ")));

    public void findFiles(File root) throws IOException {

        File[] listOfFiles = root.listFiles();
        for (int i = 0; i < listOfFiles.length; i++) {
            String iName = listOfFiles[i].getName();
            if (listOfFiles[i].isFile() && iName.endsWith(".tif")) {

                RenderedImage renderedImage[], page;
                File tiffFile = new File(listOfFiles[i].getAbsolutePath());

                SeekableStream seekableStream = new FileSeekableStream(tiffFile);
                ImageDecoder imageDecoder = ImageCodec.createImageDecoder("tiff",seekableStream, null);
                renderedImage = new RenderedImage[imageDecoder.getNumPages()];

                int tifPageCount = 0;
                for (int k = 0; k < imageDecoder.getNumPages(); k++) {
                    renderedImage[k] = imageDecoder.decodeAsRenderedImage(k);
                    tifPageCount++;
                }

                long fileSize = listOfFiles[i].length();

                if ((fileSize <= (sizeOfFile*1000)) && (tifPageCount <=15)) {
                    File file = new File("\\\\server\\filename\\TestFiles\\" + listOfFiles[i].getName());

                    if (file.exists()) {
                        System.out.println(file.getName() + " already exists");
                    } else {
                        File folder = new File("\\\\server\\filename\\TestFiles\\Files by Size\\Files of size " + ((sizeOfFile*1000) / 1000) + " KB");
                        if (!folder.exists()) {
                            if (folder.mkdir()) {
                                System.out.println("Folder : " + folder.getAbsolutePath());
                            }
                        }

                        try{
                            System.out.println(listOfFiles[i].getName() + " | " + FileUtils.byteCountToDisplaySize(fileSize));
                            FileUtils.copyFileToDirectory(listOfFiles[i], folder);
                        }
                        catch(IOException e){
                            System.out.println("Same files");
                            return;
                        }
                    }
                }
            }

            else if (listOfFiles[i].isDirectory()) {
                findFiles(listOfFiles[i]);
            }
        }
    }
}

Simplifying the notation

        File[] listOfFiles = root.listFiles();
        for (int i = 0; i < listOfFiles.length; i++) {
            String iName = listOfFiles[i].getName();
            if (listOfFiles[i].isFile() && iName.endsWith(".tif")) {

Consider

        for (File source : root.listFiles()) {
            if (source.isFile() && source.getName().endsWith(".tif")) {

This way we don't create an index variable that we don't need.  Instead, we iterate over the list directly.  This saves dereferencing.

I also got rid of the iName variable.  This is more of a judgment call, but it was only used once.  I find that unnecessary.

I prefer the name source, as it is clearer about the purpose of the file.

Direct answers

                RenderedImage renderedImage[], page;
                File tiffFile = new File(listOfFiles[i].getAbsolutePath());

                SeekableStream seekableStream = new FileSeekableStream(tiffFile);
                ImageDecoder imageDecoder = ImageCodec.createImageDecoder("tiff",seekableStream, null);
                renderedImage = new RenderedImage[imageDecoder.getNumPages()];

                int tifPageCount = 0;
                for (int k = 0; k < imageDecoder.getNumPages(); k++) {
                    renderedImage[k] = imageDecoder.decodeAsRenderedImage(k);
                    tifPageCount++;
                }

Consider

                File tiffFile = new File(source.getAbsolutePath());
                SeekableStream seekableStream = new FileSeekableStream(tiffFile);
                ImageDecoder imageDecoder = ImageCodec.createImageDecoder("tiff", seekableStream, null);

                int tifPageCount = imageDecoder.getNumPages();

This saves declaring the never used page.

It saves declaring and populating renderedImage when you only want to generate a count.

Separation of concerns

The real problem here is that the method does a bunch of different things.  It takes a directory and traverses it.  At each file, it checks if the file matches a characteristic.  If the file matches, it takes an action.  It produces output as it goes.  Consider

public class FileFinder {

    private List<File> files = new ArrayList<>();
    private final FileMatcher matcher;
    private final File root;

    public FileFinder(FileMatcher matcher, File root) {
        this.matcher = matcher;
        this.root = root;
    }

    public List<File> findMatchingFiles() throws IOException {
        files = new ArrayList<>();
        findMatchingFiles(root);
        return files;
    }

    private void findMatchingFiles(File root) throws IOException {
        if (root.isFile()) {
            if (matcher.matches(root)) {
                files.add(root);
            }

            return;
        }

        if (!root.isDirectory()) {
            return;
        }

        for (File file : root.listFiles()) {
            findMatchingFiles(file);
        }
    }

}

This class and method has a single purpose, to traverse a file system from a particular root and find all the matching files.  Rather than processing them in place, it returns the files to be processed by the caller.  It doesn't even know how the files match, only that they do.

It's not clear to me that this needs the throws IOException.  I didn't try it, as I didn't want to set up a test environment.  If needed, this is how to do it.  If not, you can just remove them.

public interface FileMatcher {

    boolean matches(File file);

}

Not much to the interface.

public class SuffixMatcher implements FileMatcher {

    private final String suffix;

    public SuffixMatcher(String suffix) {
        this.suffix = suffix;
    }

    public boolean matches(File file) {
        return file.getName().endsWith(suffix);
    }

}

Not much to the implementation either.  The important part is that this abstracts the matching from the traversal.  We could reuse this code for not just different suffixes but entirely different criteria.

public class TiffCopier {

    public static final int MAXIMUM_COUNT = 15;
    public final int FILE_SIZE;
    public final int MAXIMUM_SIZE;
    public final File FOLDER;
    public final String TARGET_PATH;

    public TiffCopier(int fileSize, String targetPath) throws IOException {
        FILE_SIZE = fileSize;
        MAXIMUM_SIZE = fileSize * 1000;
        TARGET_PATH = targetPath;

        FOLDER = new File(targetPath + "Files by Size\\Files of size " + fileSize + " KB");
    }

    public static int countPages(File source) throws IOException {
        File tiffFile = new File(source.getAbsolutePath());
        SeekableStream seekableStream = new FileSeekableStream(tiffFile);
        ImageDecoder imageDecoder = ImageCodec.createImageDecoder("tiff", seekableStream, null);

        return imageDecoder.getNumPages();
    }

    public void copy(File source) throws IOException, SameFilesException {
        long fileSize = source.length();
        if (fileSize > MAXIMUM_SIZE || countPages(source) > MAXIMUM_COUNT) {
            return;
        }

        File file = new File(TARGET_PATH + source.getName());
        if (file.exists()) {
            System.out.println(file.getName() + " already exists");
            return;
        }

        if (!FOLDER.exists()) {
            if (FOLDER.mkdir()) {
                System.out.println("Folder : " + FOLDER.getAbsolutePath());
            }
        }

        try {
            String sizeDisplay = FileUtils.byteCountToDisplaySize(fileSize);
            System.out.println(source.getName() + " | " + sizeDisplay);
            FileUtils.copyFileToDirectory(source, FOLDER);
        } catch(IOException e) {
            System.out.println("Same files");
            throw new SameFilesException();
        }
    }

    public void copyAll(File root) throws IOException {
        FileMatcher matcher = new SuffixMatcher(".tif");
        FileFinder finder = new FileFinder(matcher, root);

        for (File source : finder.findMatchingFiles()) {
            try {
                copy(source);
            } catch (SameFilesException e) {
                return;
            }
        }
    }

}

This breaks your single long method into smaller pieces.

There are still some separation of concerns issues.  This still has output scattered throughout the execution of the program.  I did localize it to just one of the smaller methods.  But that method still does more things than it should.  I'm not entirely sure of the point of the output, so I didn't try to overengineer a more elegant solution.

This program still halts execution on most IOException occurrences.  It might be better to handle these and continue.

I still include throws IOException in places that may not need it.  I would encourage you to remove the extras.

I did break out several utilities.  For example, the countPages is now its own method.  I left it here, although it could be moved into its own class.  Suffix matching and file system traversal get their own reusable classes.

I did not implement SameFilesException.  Its implementation should be relatively trivial, as it basically just rebrands an IOException occurring at a particular point.

I declared several constants, including two that are just operations performed repeatedly on the same datum.  I find it easier to follow this more abstracted version.

I standardized on putting control statements between ending and beginning braces when that fit.  E.g. } else { on the same line.  You sometimes did that and sometimes put the control statement on a new line after a closing brace.  The normal Java standard is all on the same line, but the most important thing is consistency.  Please pick one style and stick to it throughout each program.

As I said, I didn't try to compile this much less test it.  Beware of silly errors introduced in editing.


File Parsing in Java Using Collection I have the following requirements:

  /* Collection

  Write a program that counts the number of unique words in a large text file.

  a] Store the words in a collection and report the number of unique words.

  b] Once you have created the collection, allow the user to search it to see whether various words appear in the text file and to get the count if present

  */

I have implemented the following code to satisfy the above requirements. Please review the code and give suggestions to make this code more clear,  exactly correct and optimized.

import java.util.*;
import java.io.*;
public class test5
{
    public static void main(String[]args) throws FileNotFoundException
    {

        Scanner s = new Scanner(new File("test.txt"));
        ArrayList<String> wordList = new ArrayList<String>();
        Map<String, Integer> wordCount = new HashMap<String, Integer>();

        while (s.hasNext())
        {
            wordList.add(s.next());
        }

        HashSet<String> hset = new HashSet<String>(wordList);

        System.out.println("Count of Unique words from file is: "+hset.size());

        for(String word: wordList)
        {
            Integer count = wordCount.get(word);
            wordCount.put(word, (count==null) ? 1 : count+1);
        }
        System.out.println(wordCount);
    }
}

Your code looks overly complex to me, using three different collections when one, the hash map, would be enough. Use your code from the wordList loop when emptying the scanner, and update the word count there:

while (scanner.hasNext()) {
    String word = scanner.next()
    Integer count = wordCount.get(word)
    wordCount.put(word, count == null ? 1 : ++count)
}


Catch all checked exceptions and turn into unchecked exception I am currently trying to learn Lambdas and following the book Java SE8 for the really Impatient and just finished Chapter 1 of it. This is the question, I am trying to get done with:

  Didn’t you always hate it that you had to deal with checked exceptions
  in a Runnable? Write a method uncheck that catches all checked
  exceptions and turns them into unchecked exceptions. For example,

new Thread(uncheck( () -> { System.out.println("Zzz"); Thread.sleep(1000); })).start(); // Look, no catch (InterruptedException)!

  Hint: Define an
  interface RunnableEx whose run method may throw any exceptions. Then
  implement public static Runnable uncheck(RunnableEx runner). Use a
  lambda expression inside the uncheck function. Why can’t you just use
  Callable<Void> instead of RunnableEx?

This is my solution:

@FunctionalInterface
public interface RunnableEx extends Runnable {
    @Override
    default void run () {
        try {
            doRun();
        } catch (Error | RuntimeException e) {
            throw e;
        } catch (Throwable throwable) {
            throw new IndexOutOfBoundsException("IOBE");
        }
    }

    void doRun ()
        throws Throwable;
}

and

public class Question6 {
    public static void main (String... args) {
        uncheck(() -> {
            System.out.println("Question6");
        });
    }

    public static Runnable uncheck (final RunnableEx runner) {
        final RunnableEx runnable = () -> {
            try {
                runner.doRun();
            } catch (Error | RuntimeException e) {
                throw e;
            } catch (Throwable ignored) {
                runner.doRun();
            }
        };

        return runnable;
    }

    // We can not use Callable<Void> because it throws Checked Exception.
}

I request two things:

How can this be improved, if it can be?
As per my understanding, this implementation seems to be correct, but is it really?

default void run () {

There should be no blank after a method name (neither in a declaration not in a call). There are only blanks after if and while to distinguish them visually from method calls.

    try {
        doRun();
    } catch (Error | RuntimeException e) {
        throw e;
    } catch (Throwable throwable) {
        throw new IndexOutOfBoundsException("IOBE");
    }

Really IndexOutOfBoundsException??? What index, what bounds? It should be

    try {
        doRun();
    } catch (Error | RuntimeException e) {
        throw e;
    } catch (Throwable e) {
        throw new RuntimeException(e);
    }

It's called exception wrapping. You can't throw e, so you wrap it into a RuntimeException and it can be obtained via getCause. Printing the stack does it, so you lose no information.

    final RunnableEx runnable = () -> {
        try {
            runner.doRun();
        } catch (Error | RuntimeException e) {
            throw e;
        } catch (Throwable ignored) {
            runner.doRun();
        }
    };

The line runner.doRun() makes no sense to me. It failed, so you do it again. Why?

Moreover, it may throw a Throwable.

Moreover, you're repeating what you did above.

You've been already given a workable example in the question:

new Thread(uncheck( () -> { System.out.println("Zzz"); Thread.sleep(1000); })).start(); // Look, no catch (InterruptedException)!


Count chars difference between two strings Given two strings with the same length, we check how many chars on the same positions are different. For example: ABCD and ABBD should return 1. Here is a link to a gist with tests, to have a better understanding of what I'm building.
Here is my solution:

object Hamming {

  def compute(str1: String, str2: String):Int = {
    if (str1.length != str2.length)
      throw new IllegalArgumentException("Strings have different length")
    (0 to str1.length - 1).filter(i => str1(i) != str2(i)).size
  }

}

How about this variant?

object Hamming {

    def apply(a: String, b: String): Int = if (a.length != b.length) {
        throw new IllegalArgumentException(s"Strings a=$a and b=$b have different length.")
    } else a.zip(b).count {
        case (charA, charB) => charA != charB
    }

}

The zip-function does create a new collection of tuples with two corresponding chars side by side in every tuple. (Char, Char)
The count-function counts the number of elements for which a boolean evaluation function, passed as an argument, returns true. This should work not only for characters, because it uses pattern matching. Also if there are two arguments of the same type (e.g. for comparison) and I cannot further specify what they are, then I like to name them a and b.
I cannot tell if this solution is faster than yours, but I don't think there will be a significant difference.

P.S.:

I see the function is named compute. Thats really general and abstract. I named it apply. It can be called like this: Hamming("Alice", "Bob")
Look at what I did with the brackets.
If you would like to use the function for other sequences than characters you can change the snippet to this:

def apply(a: Seq[_], b: Seq[_]): Int = if (a.size != b.size) {
    throw new IllegalArgumentException("Parameters have different length.")
} else a.zip(b).count {
    case (charA, charB) => charA != charB
}


Using a registry with static container I am currently working on a class to make objects and vars available to child classes while attempting to avoid a singleton. Each child with dependencies extend this class and pass params (var names) of required dependencies. If parent construct is called and params is empty full registry is set.

Example of Registry:

class Registry {
public static $registry = [];

public function __construct($DI = '') {
    if (self::$registry) {

        /**
         *  $DI List Missing Pass Entire Registry
         */
        if (!$DI) {
            foreach (self::$registry as $var => $value) {
                $this->$var = $value;
            }
        }

        /**
         *  $DI List Set, Pass Specific Dependencies
         */
        else {
            foreach ($DI as $dependency) {

                    if (isset(self::$registry[$dependency])) {
                        $this->$dependency = self::$registry[$dependency];
                    }
                    else {
                        if (ENVIRONMENT == 'development') {
                            echo get_called_class().' Needs Dependency '.$dependency.'

';
                        }
                    }

            }
        }
    }
}

/**
 *  Add to Registry
 */
public static function add($array, $type = 'class', $namespace = '') {
    foreach ($array as $var => $value) {
        if (!isset(self::$registry[$var])) {

            switch ($type) {
                case 'class':
                    $class                  = $namespace.$value;
                    self::$registry[$var]   = new $class;
                    break;

                case 'var':
                    self::$registry[$var]   = $value;
                    break;
            }

        }
    }
}

private function __clone() {}
private function __wakeup() {}
}

Adding and using vars/objects from registry is easily done through static calling, but the main point of this class is to pass dependencies to child classes. I am wondering if there are drawbacks that someone with more experience would know of/find.

I think the proposed solution is bad. There is no room for lazy loading. All dependencies need to be defined for this 'registry' class. The registry class thus becomes all knowing. All other classes extend Registry, thus are all knowing as well. If you think that is no problem? then stop reading and continue

Another problem: objects are passed around by reference, vars are copied. Is this desired?

What if the $class needs different parameters set depending on where it is used?

Lets look at the following situation:
Both the database and the mailer need a LoggerInterface. So they have a dependency of 'LoggerInterface'. The database however logs to a file, and the Mailer logs to the database. How do you solve this? Have the Mailer depend on a DatabaseLogger?

So to be able to use your Registry, we can no longer hint for generic interfaces, but we hint for concrete classes.


C++ beginner exercises: grading program I finished the grading exercise from this list of beginner c++ exercises. I have a (probably unfounded) dislike of if elses to dispatch a grade to a note so contrarily to this similar question I implemented a vector of tuples to which I compare the received note. After completion, I realized that it probably amounts to mostly the same thing, if not worse (I am still comparing the value with an if).

I looking for any feedback, but possibly something about:

Is the vector of tuple an unnecessary overhead over the simple if/else?
Is it actually slower, or does compilation optimizes execution?
In this case it probably doesn't matter because the grade set is small, but is this approach considered a good/correct practice if we were to have a bigger set?

#include <iostream>
#include <vector>
#include <tuple>

using namespace std;

char dispatchGrade(int grade) {
    vector<tuple<int, char>> fork {
            tuple<int, char> {90, 'A'},
            tuple<int, char> {80, 'B'},
            tuple<int, char> {70, 'C'},
            tuple<int, char> {60, 'D'},
            tuple<int, char> {0, 'E'},
    };
    for (vector<tuple<int, char>>::size_type i = 0; i != fork.size(); i++) {
        if (grade >= get<0>(fork[i])) {
            return get<1>(fork[i]);
        }
    }
}

int main() {
    int grade;
    char note;
    cout << "Hello, World! enter your grade:" << endl;
    cin >> grade;
    if (!cin.fail() && grade > -1 && grade < 101) {
        cout << "That is a valid grade. ";
        note = dispatchGrade(grade);
        cout << "That grade is good for a " << note << endl;
    } else {
        cout << "That is not a valid grade." << endl;
    }
}

cmake_minimum_required(VERSION 3.8)
project(grades)

set(CMAKE_CXX_STANDARD 17)

set(SOURCE_FILES main.cpp)
add_executable(grades ${SOURCE_FILES})

Code Review:
grade > -1 && grade < 101

That's fine for integers. But you have to consider that somebody can come along behind you and modify your types sometime in the future. If I changed the type of grade to float (not an unreasonable change). Then a grade of -.05 becomes valid. I would change this to be:
grade >= 0 && grade <= 100
!cin.fail()

Checking the state of the file manually is basically never correct (its an anti-pattern). Also this case is also wrong as fail() does not check for end of file, so you could have hit end of file not read anything into grade.
A better check is:
cin.good();

But the more usual check is
cin  // If used in a boolean context (like an if) it will be converted
     // to a boolean based on the call to good.

if (cin && grade >= 0 && grade <= 100)

But even this is not usual. Usually you check the result of the read operation by placing the read into the if statement.
if (cin >> grade && grade >= 0 && grade <= 100)

Note: the operator>> returns a a reference to the stream, so this is functionally the same as the line above. But it imparts more information.
What you are saying here is: if the read works and the grade is in the range [0..100] we succeeded.
Looking at your dispatch. You have a structure that is the same every time you call.
vector<tuple<int, char>> fork {
        tuple<int, char> {90, 'A'},
        tuple<int, char> {80, 'B'},
        tuple<int, char> {70, 'C'},
        tuple<int, char> {60, 'D'},
        tuple<int, char> {0, 'E'},
};

Since this is never modified it should be marked const. Also since you really only want to build the structure once just mark it static that way you only build the structure on the first call. Also there is a convenience function called make_tuple
static const vector<tuple<int, char>> fork
{
        make_tuple(90, 'A'),
        make_tuple(80, 'B'),
        make_tuple(70, 'C'),
        make_tuple(60, 'D'),
        make_tuple(0,  'E')
};

This loop is a bit C-Like (though it works).
for (vector<tuple<int, char>>::size_type i = 0; i != fork.size(); i++)

Though I would have used some typedefs to make it more readable:
typedef std::tuple<int, char>  MarkRange;
typedef std::vector<MarkRange> MarkSet;
for (MarkSet::size_type i = 0; i != fork.size(); ++i)  // prefer pre-increment

If you were using C++03 then I would be using iterators:
for (MarkSet::const_iterator i = fork.begin(); i != fork.end(); ++i)

But since C++11 we have had range based for
for (auto const& mark : fork)

Note:
// Since C++11 prefer `using` over typedef
using MarkRange = std::tuple<int, char>;
using MarkSet   = std::vector<MarkRange>;

Using std::endl is not a good idea. Prefer "\n". The difference is that endl flushes the stream.  Flushing is usually always wrong. The streams will flush themselves at appropriate times let the code do its work.
Note: The most common complaint about C++ stream is that they are slower than C stream. This is usually down to inappropriate calls to flush that significantly degrades performance when done incorrectly (and manually is usually always incorrect).
Questions:

Is the vector of tuple an unnecessary overhead over the simple if/else?

I like it. Its called Data Driven programing (where the code uses data to define its structure). This is usually considered best practice.

Is it actually slower, or does compilation optimizes execution?

I doubt you could measure the difference.
But if it is important then you should measure it.
People will always have opinions on this; they are not relevant. The only thing that matters is data. BUT it only matters if this is important. Normally I would consider more expressive readable code more important. This code is readable and easy to update (I could even put the data into a config file that is modified seprately).

In this case it probably doesn't matter because the grade set is small, but is this approach considered a good/correct practice if we were to have a bigger set?

Even in bigger list this seems like a better idea.


Find the largest odd number (follow-up code) Ask user to input 10 integers and then print the largest odd number that was entered. If no odd number was entered, print a message to that effect.

This is a follow-up code on feedback from Find the largest odd number

This time I've taken care of a bug related to the number of integers entered and tried to handle invalid input if any. Please let me know if the code still can be tidied up in any way. I'm a beginner programmer.

print "You'll be asked to enter 10 integers."
print 'The program will look for odd numbers, if any.'
print 'Then, it will print the largest odd number.'
print 'Press Enter to start.'
raw_input('-> ')

numbers_entered = []

while True:
    try:
        number = int(raw_input('Enter an integer: '))
        numbers_entered.append(number)
    except ValueError:
        print 'That was not an integer.'

    if len(numbers_entered) == 10:
        break

odd_numbers = [item for item in numbers_entered if item % 2 == 1]

if odd_numbers:
    print 'The largest odd number entered was {}.'.format(max(odd_numbers))
else:
    print 'No odd number was entered.'

Be careful of changing back and forth from ' and ". I can see that you probably used the double quotes just so you could include an apostrophe, but I think it's best to stay consistent and use them throughout since you never need to print any "'s later in the program.

print "You'll be asked to enter 10 integers."
print "The program will look for odd numbers, if any."

Also, you don't need to keep the list of odd_numbers if all you want is the highest one. So you could just directly get the max value:

max_value = max([item for item in numbers_entered if item % 2 == 1])

And this way, you don't even need to build a list. You can instead pass something called a generator expression directly to max. It's like a list comprehension, but it doesn't create a full list. It can be used in both for loops and functions that lake lists, like max does.

However, this would raise a ValueError if you had no odd numbers in the list, as max can't take an empty sequence. But you could just use this as your test instead of if odd_numbers.

try:
    max_value = max(item for item in numbers_entered if item % 2 == 1)
    print "The largest odd number entered was {}.".format(max_value)
except ValueError:
    print "No odd number was entered."