Living off of chocolate This is the second project for my CS1 class, this time I'm actually getting it reviewed before I submit it ;)

  The Harris-Benedict equation estimates the number of calories your
  body needs to maintain your weight if you do no exercise.  This is
  called your basal metabolic rate, or BMR.

  The formula for the calories needed for a woman to maintain her weight
  is

  $$ \text{BMR} = 655 + \left( 4.3 \cdot \text{weight in pounds} \right) + \left( 4.7 \cdot \text{height in inches} \right) - \left( 4.7 \cdot \text{age in years}\right)$$

  The formula for the calories needed for a man to maintain his weight
  is

  $$ \text{BMR} = 66 + \left( 6.3 \cdot \text{weight in pounds} \right)
 + \left( 12.9 \cdot \text{height in inches} \right) - \left( 6.8 \cdot \text{age in years}\right)$$

  A typical chocolate bar will contain around 230 calories.  Write a
  program that allows the user to input his or her weight in pounds,
  height in inches, age in years, and the character 'M' for male and 'F'
  for female.  The program should then output the number of chocolate
  bars that should be consumed to maintain one's weight for the
  appropriate sex of the specified weight, height, and age.

calorie.cpp:

/**
 * @file calorie.cpp
 * @brief Determines the number of chocolate bars you need to eat in order to live
 * @author syb0rg
 * @date 9/12/14
 */

#include <iostream>
#include <cctype>
#include <limits>

int main()
{
    double weight = 0;
    double height = 0;
    double age = 0;
    double bmr = 0;
    char gender = 'm';

    // get input for weight, re-read input if not a number
    std::cout << "Enter the weight in pounds: ";
    while(!(std::cin >> weight))
    {
        std::cin.clear();
        std::cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n');
        std::cout << "Invalid input.  Please enter a number: ";
    }
    // get input for height, re-read input if not a number
    std::cout << "Enter the height in inches: ";
    while(!(std::cin >> height))
    {
        std::cin.clear();
        std::cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n');
        std::cout << "Invalid input.  Please enter a number: ";
    }
    // get input for age, re-read input if not a number
    std::cout << "Enter your age: ";
    while(!(std::cin >> age))
    {
        std::cin.clear();
        std::cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n');
        std::cout << "Invalid input.  Please enter a number: ";
    }
    // get input for weight
    std::cout << "Enter your gender (M/f): ";
    std::cin >> gender;

    if ('m' == gender)
    {
        bmr = 66 + (6.3 * weight) + (12.9 * height) - (6.8 * age);
    }
    else if ('f' == gender)
    {
        bmr = 655 + (4.3 * weight) + (4.7 * height) - (4.7 * age);
    }
    else std::cout << "You aren't human." << std::endl;

    const double numBars = bmr / 230; // divide by the number of calories in a chocolate bar
    std::cout << "The person with the entered metrics is allowed to consume " << numBars << " bars of chocolate." << std::endl;
}

Just a quick glance.
But:

std::cout << XXXX
while(!(std::cin >> age))
{
    std::cin.clear();
    std::cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n');
    std::cout << "Invalid input.  Please enter a number: ";
}

That could be a function.
This would reduce code and make the application easier to read.

Remember code should be DRY.

The formula is the same here:

if ('m' == gender)
{
    bmr = 66 + (6.3 * weight) + (12.9 * height) - (6.8 * age);
}
else if ('f' == gender)
{
    bmr = 655 + (4.3 * weight) + (4.7 * height) - (4.7 * age);
}

So why not pick the constants from some indexed value:

bmr = bmrconst[gender].base
      + (bmrconst[gender].weight * weight)
      + (bmrconst[gender].height * height)
      - (bmrconst[gender].age    * age);

but pick better names than I did.

Don't like magic numbers (230).

const double numBars = bmr / 230;

Use a constant that has a name that makes this have meaning.

Don't declare all your variables at the top:

double weight = 0;
double height = 0;
double age = 0;
double bmr = 0;
char gender = 'm';

Declare them as you need them as close to the point of first use as possible.


iOS UITableView; saving cell checkmarks to disk As per suggestion on SO, I am posting my code here to be reviewed. It was said I should not be saving and reading from disk so many times when a user selects and deselects a cell in my tableView.

@property (nonatomic, strong) IBOutlet UITableView *tableView;
@property (nonatomic, strong) NSArray *list;

- (void)viewDidLoad
{
    [super viewDidLoad];

    self.list = [[NSArray alloc] initWithObjects:@"Alabama", @"Alaska", @"Arizona",
    @"Arkansas", @"California", @"Colorado", @"Connecticut", @"Delaware", @"District of Columbia",
    @"Florida", @"Georgia", @"Hawaii", @"Idaho", @"Illinois", @"Indiana", @"Iowa", @"Kansas",
    @"Kentucky", @"Louisiana", @"Maine", @"Maryland", @"Massachusetts", @"Michigan", @"Minnesota",
    @"Mississippi", @"Missouri", @"Montana", @"Nebraska", @"Nevada", @"New Hampshire",
    @"New Jersey", @"New Mexico", @"New York", @"North Carolina", @"North Dakota", @"Ohio",     @"Oklahoma",
    @"Oregon", @"Pennsylvania", @"Rhode Island", @"South Carolina", @"South Dakota",
    @"Tennessee", @"Texas", @"Utah", @"Vermont", @"Virginia", @"Washington", @"West Virginia",
    @"Wisconsin", @"Wyoming", nil];
}

- (NSString *)SettingsPlist
{
    NSString *paths = [NSSearchPathForDirectoriesInDomains(NSDocumentDirectory, NSUserDomainMask, YES) objectAtIndex:0];
    NSString *PlistPath = [paths stringByAppendingPathComponent:@"Settings.plist"];
    return PlistPath;
}

- (NSInteger)tableView:(UITableView *)tableView
 numberOfRowsInSection:(NSInteger)section
{
    return [[self list] count];
}

- (UITableViewCell *)tableView:(UITableView *)tableView cellForRowAtIndexPath:(NSIndexPath *)indexPath
{
    NSString *contentForThisRow = [[self list] objectAtIndex:[indexPath row]];
    UITableViewCell *cell = [tableView dequeueReusableCellWithIdentifier:@"Cell"];

    if(cell == nil)
    {
        cell = [[UITableViewCell alloc] initWithStyle:UITableViewCellStyleDefault reuseIdentifier:@"Cell"];
    }

    NSDictionary *dict = [[NSDictionary alloc] initWithContentsOfFile:[self SettingsPlist]];

    NSString *row = [NSString stringWithFormat:@"%d",indexPath.row];

    if([[dict objectForKey:row]isEqualToString:@"0"])
    {
        cell.accessoryType = UITableViewCellAccessoryNone;
    }
    else
    {
        cell.accessoryType = UITableViewCellAccessoryCheckmark;
    }

    [[cell textLabel] setText:contentForThisRow];

    return cell;
}

- (void)tableView:(UITableView *)tableView didSelectRowAtIndexPath:(NSIndexPath *)indexPath
{
    NSMutableDictionary *plist = [NSMutableDictionary dictionaryWithContentsOfFile:[self SettingsPlist]];

    UITableViewCell *cell = [tableView cellForRowAtIndexPath:indexPath];

    NSString *row = [NSString stringWithFormat:@"%d",indexPath.row];

    if(cell.accessoryType == UITableViewCellAccessoryNone)
    {
        cell.accessoryType = UITableViewCellAccessoryCheckmark;
        NSString *on = @"1";
        [plist setObject:on forKey:row];
        [plist writeToFile:[self SettingsPlist] atomically:YES];
    }
    else if(cell.accessoryType == UITableViewCellAccessoryCheckmark)
    {
        cell.accessoryType = UITableViewCellAccessoryNone;
        NSString *off = @"0";
        [plist setObject:off forKey:row];
        [plist writeToFile:[self SettingsPlist] atomically:YES];
    }
    [tableView deselectRowAtIndexPath:indexPath animated:YES];
}

you should read the settings plist once in -viewDidLoad: and save the contents as a dictionary to a property.

As you are doing, it is a huge performance hit as the disk will be accessed every time a cell appears on screen during scrolling.

In -tableView:cellForRowAtIndexPath: and -tableView:didSelectRowAtIndexPath: you then should only work on that dictionary.

Further more you could introduce a singleton-like shared object, that holds the settings and can be accessed in any class.

see my example code for how to do multiple selection and keeping track of the selected contents.


Find a best fuzzy match for a string I am trying to find a best match for a name within a list of names.

I came up with the below code, which works, but I feel that the creation of the intermediate ratios list is not the best solution performance-wise.

Could it be somehow replaced? Would reduce be a good fit here (I have tried it, but could not get the index of best match and its value at the same time).

from fuzzywuzzy import fuzz

name_to_match = 'john'
names = ['mike', 'james', 'jon', 'jhn', 'jimmy', 'john']
ratios = [fuzz.ratio(name_to_match, name) for name in names]
best_match = names[ratios.index(max(ratios))]
print(best_match) # output is 'john'

You should take advantage of the key argument to max():

  The key argument specifies a one-argument ordering function like that used for list.sort().

best_match = max(names, key=lambda name: fuzz.ratio(name_to_match, name))


Button click logic with SFML I wrote some functionality that works as intended in order to create functional clickable rectangles. I inherit from RectangleShape in SFML and I have several objects (not just buttons) that inherit from this. The intended logic is to check if a downpress and release pair both happen inside of the same object.

clickableRect.h

#ifndef CLICKABLERECT
#define CLICKABLERECT

#include <SFML\Graphics.hpp>

class clickableRect : public sf::RectangleShape
{
public:
    clickableRect();

    bool click(const sf::RenderWindow & window, const sf::Event & ev);

    void reset();
private:
    bool m_mouseHeld;
    bool m_pressed;
    bool m_active;

    bool isInside(const sf::RenderWindow & window) const;
};
#endif // !CLICKABLERECT

and

clickableRect.cpp

#include "clickableRect.h"

clickableRect::clickableRect() :
    m_pressed(false),
    m_mouseHeld(false),
    m_active(false)
{
     //do nothing
}

bool clickableRect::click(const sf::RenderWindow & window, const sf::Event & ev)
{
    if (ev.type == sf::Event::MouseButtonPressed && isInside(window) && !m_mouseHeld)
    {
        m_pressed = true;
        m_mouseHeld = true;
    }
    else if (ev.type == sf::Event::MouseButtonPressed)
    {
        m_mouseHeld = true;
    }

    if (ev.type == sf::Event::MouseButtonReleased)
    {
        if (isInside(window) && m_pressed)
        {
            m_active = true;
        }
        m_pressed = false;
        m_mouseHeld = false;
    }

    return m_active;
}

void clickableRect::reset()
{
    m_active = false;
}

bool clickableRect::isInside(const sf::RenderWindow & window) const
{
    return sf::Mouse::getPosition(window).x > getPosition().x
        && sf::Mouse::getPosition(window).x < getPosition().x + getSize().x
        && sf::Mouse::getPosition(window).y > getPosition().y
        && sf::Mouse::getPosition(window).y < getPosition().y + getSize().y;
}

I would love any and all feedback on all aspects of the code provided but request specifically to know about:

Is this a good use of inheritance and am I doing it right? Should I use a component pattern instead?
Can I clean up the logic itself inside the click function. It seems a bit messy to me and also requires me to call reset on the objects immediately after returning a true value.

Compile with all warnings

If you compile this code with all warnings enabled (e.g. -Wall -Wextra -pedantic) the compiler will tell you that your initialization is out of order.
This simply means you initialize your members in a different order than they have been declared in. In your example this is not bad but if you have members that depend on others being initialized before them this can lead to problems.

& is part of the type in C++

While this is not an error it can look like bitwise AND at a glance and be a potential source of confusion. Prefer to write foo& bar instead of foo & bar.

User defined types

Also just a minor nitpick but usually user defined types start with an uppercase letter i.e. ClickableRect.
Since you inherit from Shape I think it would make more sense to name your class ClickableShape but this is just personal preference.

Design

You expose window to your class only to get access to the mouse coordinates. Unless you plan to use window in a different matter in the future it'd be better to hide this info from your class.
A simple approach to testing for a click within your shape could be:

class ClickableShape : public sf::RectangleShape {
public:
    bool click(const sf::Vector2i& mouse_pos) const {
        return getGlobalBounds().contains(mouse_pos.x, mouse_pos.y);
    }
};

Which can then be called as:

ClickableShape cs{};
cs.setSize(sf::Vector2f(50, 50));
cs.setFillColor(sf::Color(0xffffffff));

And finally in your event loop:

case sf::Event::MouseButtonPressed:
    cs.click(sf::Mouse::getPosition(window));
    break;

This way you avoid exposing unnecessary data to your class and can still check if the shape was clicked.

Approach #2:

I'm not sure what you're planning to do with this in the future but another approach to this would be to keep the inheritance short and have the rectangle as a member.

class MyButton : public sf::Drawable {
public:
    MyButton() {
        shape.setSize(sf::Vector2f(50, 50));
        shape.setFillColor(sf::Color(0xffffffff));
    }

    bool click(const sf::Event& event) const {
        return shape.getGlobalBounds().contains(event.mouseButton.x, event.mouseButton.y);
    }

    void draw(sf::RenderTarget& target, sf::RenderStates states) const {
        target.draw(shape);
    }

private:
    sf::RectangleShape shape;
};


Implementing a Steepest Descent Algorithm I am teaching myself some coding, and as my first "big" project I tried implementing a Steepest Descent algorithm to minimize the Rosenbrock function:

$$f(x, y) = 100 (y - x^2)^2 + (1 - x)^2$$

The algorithm goes like this: We start with an initial guess \$x_0\$ (vector). We update the guess using the formula

$$x_{k+1} = x_k - alpha (\nabla f(x_k) \cdot \nabla f(x_k))$$

where alpha is to be chosen so that is satisfies the Armijo condition. We keep repeating until we reach a point where the gradient is less than 0.1 in both components.

Could you please tell me any ways in which I could improve my algorithm? In particular, I'm looking to increase its speed. From the current starting point, it takes about 30 seconds to run on my computer (16GM ram, i7 processor).

Remark: The reason I keep using np.array([[1, 2, 3]]) for vectors is so that I can transpose and matrix multiply them at will. I'm not sure if this is a good practice.

# This program uses the Steepest Descent Method to
# minimize the Rosenbrock function
import numpy as np

# Define the Rosenbrock Function
def f(x_k):
    x, y = x_k[0, 0], x_k[0, 1]
    return 100 * (y - x**2)**2 + (1 - x)**2

# Gradient of f
def gradient(x_k):
    x, y = x_k[0, 0], x_k[0, 1]
    return  np.array([[-400*x*(y-x**2)-2*(1-x), 200*(y-x**2)]])

def main():
    # Define the starting guess
    x_k = np.array([[10, 5]])
    # Define counter for number of steps
    numSteps = 0

    # Keep iterating until both components of the gradient are less than 0.1 in absolute value
    while abs((gradient(x_k)[0, 0])) > 0.1 or abs((gradient(x_k))[0, 1]) > 0.1:
        numSteps = numSteps + 1

        # Step direction
        p_k = - gradient(x_k)
        gradTrans = - p_k.T

        # Now we use a backtracking algorithm to find a step length
        alpha = 1.0
        ratio = 0.8
        c = 0.01 # This is just a constant that is used in the algorithm

        # This loop selects an alpha which satisfies the Armijo condition
        while f(x_k + alpha * p_k) > f(x_k) + (alpha * c * (gradTrans  @ p_k))[0, 0]:
            alpha = ratio * alpha

        x_k = x_k + alpha * p_k

    print("The number of steps is: ", numSteps)
    print("The final step is:", x_k)
    print("The gradient is: ", gradient(x_k))

main()

You biggest time waster appears to be this loop:

    while f(x_k + alpha * p_k) > f(x_k) + (alpha * c * (gradTrans  @ p_k))[0, 0]:
        alpha = ratio * alpha

f(x_k), c, gradTrans, and p_k are all constant in the loop, so you can compute f(x_k) and c * (gradTrans @ p_k) before the loop and use these computed values in the test expression, instead of recomputing the same values over and over.

    fxk = f(x_k)
    offset = c * (gradTrans @ p_k)
    while f(x_k + alpha * p_k) > fxk + (alpha * offset)[0, 0]:
        alpha = ratio * alpha

Doing so cuts the time roughly in half.

Similarly, gradient(x_k) is computed 3 times here:

while abs((gradient(x_k)[0, 0])) > 0.1 or abs((gradient(x_k))[0, 1]) > 0.1:
    ...
    p_k = - gradient(x_k)

Again, compute once and store the result.

You should probably use vectors instead of matrices:

    x_k = np.array([10., 5.])

Which can be unpacked using tuple assignment:

def f(x_k):
    x, y = x_k
    return 100 * (y - x**2)**2 + (1 - x)**2

And using 10. and 5. in the above x_k initialization makes the arrays float64 instead of int32, which allows you to use in-place addition operators:

        x_k += alpha * p_k

instead of:

        x_k = x_k + alpha * p_k


UIAlertController and input validation I'm refactoring some "old" Swift code using deprecated UIAlertView and delegates.

I have a UIAlertController with an input textfield inside (and two buttons, OK and CANCEL). The user should insert his email address, then I should validate it anytime he enters a character: when the mail address is valid, the UIAlertController button is enabled, else is disabled.

I got this code perfectly working but for some reason I'm not very happy with this solution. Is there something you would change?

var alertTextField: UITextField!

func textFieldDidChange(){

    if let e = alertTextField.text {
        let alertButton = alertController.actions[0]
        alertButton.isEnabled = !Utils.isValidEmail(testStr: e) ? false : true
    }
}

var alertController = UIAlertController(title: "Insert your email", message: "message", preferredStyle: .alert)

@IBAction func passwordForgot(_ sender: Any) {

    let ok = UIAlertAction(title: "OK", style: .default, handler: { (action) -> Void in
        print("Ok Button was Pressed")
    })
    ok.isEnabled = false

    let cancel = UIAlertAction(title: "Cancel", style: .cancel) { (action) -> Void in
        print("Cancel Button was Pressed")
    }

    alertController.addAction(ok)
    alertController.addAction(cancel)

    alertController.addTextField { (textField) -> Void in
        self.alertTextField = textField
        self.alertTextField?.placeholder = "mail"
        textField.addTarget(self, action: #selector(self.textFieldDidChange), for: UIControlEvents.editingChanged)
    }

    self.present(alertController, animated: true, completion: nil)

}

I would change this:

alertButton.isEnabled = !Utils.isValidEmail(testStr: e) ? false : true

to this one:

alertButton.isEnabled = Utils.isValidEmail(testStr: e)


"Three pitchers with water" problem I don't know what this problem is named, so I can't Google for a solution about it. Here's the image:

There are three pitchers with capacities of 10, 7 and 3 quarts.

We need to move the water from a pitcher (or jar) to another pitcher so it will result in 5, 5 and 0.

I have to print all the steps needed to make the pitchers contains 5, 5 and 0 quarts of water.

Here's my working code. Please review it and tell me how to make it better, faster, shorter, more readable.

class Jar
  attr_accessor :max, :current, :target

  def initialize max, current, target
    @max = max
    @current = current
    @target = target
    raise 'max should not be 0' if max == 0
  end

  def full?
    return @current == @max
  end

  def empty?
    return @current == 0
  end

  def overflow
    return 0 if @current < @max
    over = @current - @max;
    @current = max
    return over
  end

  def move_to o
    o.current += @current
    @current = o.overflow
  end

end

class Quartz
  MAX_STEPS = 12

  def initialize max, init, target
    raise 'max.length must equal init.length' if max.length != init.length
    raise 'max.length must equal target.length' if max.length != target.length
    @ite = (0..(max.length-1))
    @memoize = {}
    @targets = []
    @steps = []
    @all_solutions = []
    @jars = @ite.collect { |x| Jar.new(max[x],init[x],target[x]) }
    @targets = current(:target)
  end

  def recurse step=1
    orig = current
    @steps << orig
    if orig == @targets
      p @steps
      return @steps.length
    end
    return false if step>=MAX_STEPS

    @memoize[orig] = step

    @jars.each do |x|
      next if x.empty?
      @jars.each do |y|
        next if x == y
        next if y.full?
        x.move_to y
        done = @memoize[current]
        if done
          restore orig
          next
        end
        ok = recurse(step+1)
        #return ok if ok # return only first solution
        @steps.pop
        restore orig
      end
    end

    return false
  end

  def current attr=:current
    @jars.collect{ |x| x.send attr }
  end

  alias_method :to_array, :current

  def restore a
    @ite.each { |i| @jars[i].current = a[i] }
  end

end

q = Quartz.new [10,7,3], [10,0,0], [5,5,0]
p q.recurse

This is the program's output:

[[10, 0, 0], [3, 7, 0], [3, 4, 3], [6, 4, 0], [6, 1, 3],
 [9, 1, 0], [9, 0, 1], [2, 7, 1], [2, 5, 3], [5, 5, 0]]

Some comments on your code:

def initialize max, current, target. The community consensus seems to be to put always parenthesis on method definitions. I'd definitely do it.
return @current == @max. It's not idiomatic to put explicits return, the last expression of a block/method is returned.
MAX_STEPS = 12. Isn't this kind of cheating?
raise 'max.length must equal init.length' if max.length != init.length. I wouldn't lose a second validating inputs. If the input data is wrong it's the caller's problem.
[Lots of imperative code which, frankly, I don't understand] When you are dealing with mathematics-related problems (some would argue all computation is a mathematics-related problem) you'll get better algorithms with functional programming.

I'd write it with a depth-first algorithm in functional style. Note that I've created two generic abstractions, Enumerable#map_detect (Facets has it) and Array#merge, so the main algorithm remains as clean and declarative as possible. Granted, it may be difficult to grasp at first if you have only experience with imperative programming, ask if you don't understand something (also, reviews of the review welcomed):

require 'facets/enumerable/map_detect'

class Array
  # [:a, :b, :c, :d].merge_from_hash(1 => :bb, 3 => :dd) #=> [:a, :bb, :c, :dd]
  def merge_from_hash(hash)
    map.with_index { |x, idx| hash.fetch(idx, x) }
  end
end

class PitchersProblem < Struct.new(:total)
  def solve(current, final, solution = [])
    if current == final
      solution + [current]
    else
      (candidates(current) - solution).map_detect do |candidate|
        solve(candidate, final, solution + [current])
      end
    end
  end

  def candidates(current)
    (0...total.size).to_a.permutation(2).map do |i1, i2|
      free2 = total[i2] - current[i2]
      if current[i1] > 0 && free2 > 0
        moved = [current[i1], free2].min
        current.merge_from_hash(i1 => current[i1] - moved, i2 => current[i2] + moved)
      end
    end.compact
  end
end

p PitchersProblem.new([10, 7, 3]).solve([10, 0, 0], [5, 5, 0])
#=> [[10, 0, 0], [3, 7, 0], [0, 7, 3], [7, 0, 3], [7, 3, 0],
#    [4, 3, 3],  [4, 6, 0], [1, 6, 3], [1, 7, 2], [8, 0, 2],
#    [8, 2, 0],  [5, 2, 3], [5, 5, 0]]

Notes on my code:

solve can be read as: "If the current state is the final, add it to the path solution and we are done. Otherwise, for each permutation of the pitchers, move water from one to the other if possible, and try to find a solution recursively (unless that state is already in our solution path)".
This algorith returns the first solution it gets (size 13), which is not the optimal (size 10?), that's because it's a depth-first algorithm, you can transform it into a breadth-first algorithm to get the shortest solution first.
Ideally solution should be an insertion-ordered data-structure with O(1) lookup (because of solution.include?(new_current)), something like an OrderedSet. An array has O(n) lookup, but anyway, let's leave it just to keep it simple.


Locale language reducer I am learning redux by doing a small project. Everything is working fine, but I need to know how I can refactor such code. For this situation, it might be good to use if conditions like I have done, but what if I need to handle 7-8 languages? It won't be viable to use else if for 7 times. What is the proper way to handle such situation?

import { FRENCH } from '../../public/messages/fr';
import { ENGLISH } from '../../public/messages/en';

const initialState = {
  lang: FRENCH.lang,
  messages: FRENCH.messages
};
export const localeReducer = (state = initialState, action) => {
  switch (action.type) {
    case 'LOCALE_SELECTED':
    if (action.locale === 'fr') {
      return { ...initialState, lang: FRENCH.lang, messages: FRENCH.messages };
    } else if (action.locale === 'en') {
      return { ...initialState, lang: ENGLISH.lang, messages: ENGLISH.messages };
    } break;
    default:
      return state;
  }
};

I don't know redux at all, so I might have missed something important so that my suggestion may be naive and doesn't work.
If so, my apologies, and don't shoot me too hard :)

That said, here is how I'd work:

first add a registry for the currently added langs
then merely use the right one depending on action.locale if it exists in registry, or keep initialState if not

Actually it results in something like this:

import { FRENCH } from '../../public/messages/fr';
import { ENGLISH } from '../../public/messages/en';

const initialState = {
  lang: FRENCH.lang,
  messages: FRENCH.messages
};
const langs = {
  fr: FRENCH,
  en: ENGLISH
}

export const localeReducer = (state = initialState, action) => {
  switch (action.type) {
    case 'LOCALE_SELECTED':
      if (langs[action.locale]) {
        return {
          ...initialState,
          lang: langs[action.locale].lang,
          messages: langs[action.locale].messages
        };
      }
    default:
      return state;
  }
};


Tower of Hanoi with helper function Here is my solution to the Tower of Hanoi problem using Python

def helper(number_rings: int, origin: int, target: int, spare: int, list_result=[])->list:
    """Helper function works out which ring to move"""
    if number_rings == 1:
        list_result.append([origin, target])
    else:
        #Move all but 1 ring from ORIGIN tower to SPARE tower
        helper(number_rings - 1, origin=origin, target=spare, spare=target)
        #MOVE the last ring from ORIGIN tower to TARGET tower
        helper(1, origin=origin, target=target, spare=spare)
        #MOVE the remaining rings from the SPARE tower to TARGET tower
        helper(number_rings - 1, origin=spare, target=target, spare=origin)
        return list_result

def move_rings(number_rings: int)->int:
    """Function prints the ring move required"""
    dict_rings = {0: "A", 1: "B", 2: "C"}
    if number_rings == 1:
        print("Move from A to C")
        return 1
    list_result = helper(number_rings, 0, 2, 1)
    for item in list_result:
        print("Move from", dict_rings[item[0]], "to", dict_rings[item[1]])
    return len(list_result)

if __name__ == "__main__":
    counter = move_rings(1)
    print(f"Solution possible in {counter} moves")

Overall your implementation is fine, I would just suggest a few things:

Naming - move_rings and helper are not very descriptive. Give them better names, and give them good docstrings (I like NumpyDoc, but personal preference is fine)
move_rings shouldn't both do calculations and print to stdout. Instead, move_rings should return the information, and the caller should decide what to do with it
move_rings doesn't need the number_rings == 1 special case; your API handles it just fine
Instead of the hard-coded number approach, you can make a little Peg class that makes things a bit cleaner - subclassing namedtuple is a nice way to do this
Using f-strings makes printing easier
You can yield from instead of appending to the default argument list, which is easier to read and doesn't waste time reallocating space for the list
I added a (honestly pretty bad) visualization function to see the movements as well.

This is what I came up with (didn't do all of the things I mentioned above)

from math import ceil
from collections import defaultdict, namedtuple
from typing import Iterable, Generator, List, Tuple

class Peg(namedtuple("Peg", "id name")):
    pass

SOURCE_PEG = Peg(0, "A")
TARGET_PEG = Peg(2, "C")
WORKER_PEG = Peg(1, "B")
PEGS = [SOURCE_PEG, TARGET_PEG, WORKER_PEG]

def helper(
    number_rings: int, origin: Peg, target: Peg, spare: Peg
) -> Generator[Tuple[Peg, Peg], None, None]:
    """
    Helper function for performing the recursive call when solving the tower puzzle.

    Parameters
    ----------
    number_rings : int
        Number of rings in the puzzle
    origin : Peg
        The peg that we're moving from
    target : Peg
        The peg that we want to move to
    spare : Peg
        The other peg, that we can use as a helper

    Yields
    ------
    move : tuple
        The movement(s) (from-Peg, to-Peg) we determined were ncessary
    """

    if number_rings == 1:
        yield (origin, target)
    else:
        # Move all but 1 ring from ORIGIN tower to SPARE tower
        yield from helper(
            number_rings - 1,
            origin=origin,
            target=spare,
            spare=target,
        )
        # MOVE the last ring from ORIGIN tower to TARGET tower
        yield from helper(
            1, origin=origin, target=target, spare=spare
        )
        # MOVE the remaining rings from the SPARE tower to TARGET tower
        yield from helper(
            number_rings - 1,
            origin=spare,
            target=target,
            spare=origin,
        )

def move_rings(
    number_rings: int
) -> Iterable[Tuple[Peg, Peg]]:
    """
    Finds the optimal number of moves when solving a Tower of Hanoi puzzle
    with `number_rings` rings, starting on the first ring.

    Parameters
    ----------
    number_rings : int
        The number of rings in the Tower of Hanoi puzzle

    Returns
    -------
    move_list : iterator
        An iterable of the Peg movements
    """

    return helper(
        number_rings, SOURCE_PEG, TARGET_PEG, WORKER_PEG
    )

def draw_tower_of_hanoi(ring_map: List[int]):
    """
    Pretty dumb drawing tool that visualizes current peg state.

    Parameters
    ----------
    ring_map : list
        List of rings and their pegs. Format for a 3-ring puzzle start would be
        [0, 0, 0]. The index represents ring size (size = i+1), and the value at
        that index represents the peg (0, 1, or 2).
    """

    largest_ring = len(ring_map)
    peg_to_ring = defaultdict(list)
    for ring, peg in enumerate(ring_map, 1):
        peg_to_ring[peg].append(ring)

    peg_to_ring = {
        peg: sorted(ring_list, reverse=True)
        for peg, ring_list in peg_to_ring.items()
    }
    height_per_ring = {
        peg: len(ring_list)
        for peg, ring_list in peg_to_ring.items()
    }

    total_width = largest_ring * 6 + 7
    total_height = len(ring_map) + 2

    peg_centers = [
        1 + largest_ring,
        3 + largest_ring * 3,
        5 + largest_ring * 5,
    ]

    # fill it out
    drawing = [
        [" " for _ in range(total_width)]
        for _ in range(total_height)
    ]

    # Draw the pegs
    for i, row in enumerate(drawing):
        for center in peg_centers:
            drawing[i][center] = "P"

    # Draw the base
    drawing[-1][:] = ["G"] * total_width

    # Draw the rings
    for peg in peg_to_ring:
        for i, ring in enumerate(peg_to_ring[peg], 2):
            index = -i
            center = peg_centers[peg]
            drawing[index][center - ring : center] = [
                "R"
            ] * ring
            drawing[index][
                center + 1 : center + ring + 1
            ] = ["R"] * ring

    print("\r\n".join(["".join(row) for row in drawing]))

def visualize_tower_movements(
    number_rings: int, moves: Iterable[Tuple[Peg, Peg]]
):
    """
    Visualization tool for drawing the movement of rings in a Tower of Hanoi puzzle.

    Parameters
    ----------
    number_rings : int
        The number of rings in the puzzle
    moves : iterable
        The set of moves performed while working on the puzzle
    """

    visualizer = [0] * number_rings
    draw_tower_of_hanoi(visualizer)
    for move_from, move_to in moves:
        # Get the smallest ring on the peg, as that is the only one that can move
        move_from_index = min(
            ring
            for ring, value in enumerate(visualizer)
            if value == move_from.id
        )
        visualizer[move_from_index] = move_to.id
        draw_tower_of_hanoi(visualizer)

if __name__ == "__main__":
    moves = list(move_rings(5))
    print(f"Took {len(moves)} moves")
    for move_from, move_to in moves:
        print(
            f"Move from {move_from.name} to {move_to.name}"
        )
    visualize_tower_movements(5, moves)


Finding the next higher digital permutation of a number This is my solution for CodeEval challenge 44.

  You are writing out a list of numbers.Your list contains all numbers with exactly Di digits in its decimal representation which are equal to i, for each i between 1 and 9, inclusive. You are writing them out in ascending order. For example, you might be writing every number with two '1's and one '5'. Your list would begin 115, 151, 511, 1015, 1051. Given N, the last number you wrote, compute what the next number in the list will be. The number of 1s, 2s, ..., 9s is fixed but the number of 0s is arbitrary.

  INPUT SAMPLE:

  Your program should accept as its first argument a path to a filename. Each line in this file is one test case. Each test case will contain an integer n < 10^6. E.g.

115
842
8000

  OUTPUT SAMPLE:

  For each line of input, generate a line of output which is the next integer in the list. E.g.

151
2048
80000

The function followingInteger takes a number in its decimal representation and returns the next higher permutation of the digits of the number. If there isn't a larger permutation of the digits, a '0' may be added to the digits:

ghci> followingInteger "123"
"132"
ghci> followingInteger "10"
"100"

The entire program can be used like this:

./ch044_solution ch044_input

where ch044_input is a file containing one integer on each line.

The code:

import Data.List (partition, sort)
import System.Environment (getArgs)

followingInteger :: String -> String
followingInteger digits
  | isSortedDesc digits = lowestPermutation ('0' : digits)
  | otherwise           = nextHigherPermutation digits

isSortedDesc :: (Ord a) => [a] -> Bool
isSortedDesc xs = all (uncurry (>=)) $ zip xs (tail xs)

lowestPermutation :: String -> String
lowestPermutation digits = let (zeros, nonZeros) = partition (== '0') digits
                               (lowestNonZeroDigit:otherDigits) = sort nonZeros
                           in  lowestNonZeroDigit : zeros ++ otherDigits

nextHigherPermutation :: String -> String
nextHigherPermutation = snd . foldr foldFun (False, "")
  where foldFun d (True, digits)  = (True, d:digits)
        foldFun d (False, digits)
          | null biggerDigits = (False, d:digits)
          | otherwise         = (True, newTail)
          where (biggerDigits, smallerDigits) = partition (> d) digits
                (swapDigit:otherDigits) = sort biggerDigits
                newTail = swapDigit : sort smallerDigits ++ (d : otherDigits)

mapFileLines :: (String -> String) -> IO ()
mapFileLines f = do
    (fileName:_) <- getArgs
    contents <- readFile fileName
    mapM_ (putStrLn . f) $ lines contents

main = mapFileLines followingInteger

I'm pretty new to Haskell, so please criticize my code in every way possible!

I'm especially interested in ways to make the code more readable!

First of all: your code is very readable and you seem to know your way around standard library functions, which is very important is Haskell. Good job!

A couple of suggestions:

A simpler isSortedDesc:

import Data.List.Ordered (isSortedBy)

isSortedDesc :: (Ord a) => [a] -> Bool
isSortedDesc = isSortedBy (>=)

Edit: Sorry, isSortedBy is not a standard function - it belongs to data-ordlist-0.2. I'd implement it in a straightforward recursive manner:

isSortedDesc :: (Ord a) => [a] -> Bool
isSortedDesc (x:y:xs') = x>=y && isSortedDesc (y:xs')
isSortedDesc _ = True

foldFun:

In foldFun, try to make the arguments more understandable. You can add a type signature, name the arguments or add a comment. For example, what is the purpose of the boolean argument?

Another thing: nextHigherPermutation has two execution steps - find a digit we can swap, and then just copy the rest of the string. Personally I'm not a fun of the idea of passing extra information to fold (the boolean argument) and then discarding it. Instead, consider splitting it to two seperate functions - for example, one that finds the swap digit and another that performs the swap.


Validating sign up pages using Selenium Python I have multiple sign up pages that I am currently testing. Each of the form have seprate fields. However, some share common fields(as you can see from signup page.py).

This is what I have so far(I am using Page Object method), I would like to know how I can improve my code to make it more robust.

Here is the signUp page.py:

class signupPage(Page):

    def __init__(self, driver):
        super(signupPage, self).__init__(driver)

    def SignupPage1139_CompanyOne(self):
        self.driver.get("https://abc.com/s_csignup.jsp?token=XVtQHUUGYFFaRhNaXxJaQVBRZEl2EnBo")
        self.driver.find_element_by_id("cname").clear()
        self.driver.find_element_by_id("cname").send_keys("QA EXTERNAL")
        self.driver.find_element_by_id("cf_2727675").clear()
        self.driver.find_element_by_id("cf_2727675").send_keys("Test")
        self.driver.find_element_by_id("cf_2697953").clear()
        self.driver.find_element_by_id("cf_2697953").send_keys("Test")
        self.driver.find_element_by_id("sbbut").click()
        self.assertEqual("Could not create organization, an organization with the entered criteria already exists.", self.close_alert_and_get_its_text())

    def SignupPage1139_CompanyTwo(self):
        self.driver.get("https://abc.com/s_csignup.jsp?token=XVtQHUUGYFFaRhNaXxJaQVBRZElzGnJt")
        self.driver.find_element_by_id("cname").clear()
        self.driver.find_element_by_id("cname").send_keys("QA EXTERNAL")
        self.driver.find_element_by_id("city").clear()
        self.driver.find_element_by_id("city").send_keys("Toronto")
        Select(self.driver.find_element_by_id("state")).select_by_visible_text("Ontario")
        self.driver.find_element_by_id("sbbut").click()
        self.assertEqual("Cannot update using new scripts as there is already a matching company", self.close_alert_and_get_its_text())

    def SignupPage1139_CompanyThree(self):
        self.driver.get("https://abc.com/s_csignup.jsp?token=XVtQHUUGYFFcRhZdXxJaQVBRZEl9HnRr")
        self.driver.find_element_by_id("cname").clear()
        self.driver.find_element_by_id("cname").send_keys("DO NOT DELETE - Pandora External")
        self.driver.find_element_by_id("cf_2698238").clear()
        self.driver.find_element_by_id("cf_2698238").send_keys("000000124")
        self.driver.find_element_by_id("sbbut").click()

Here is my test.py:

class MatchingContactCompany(BaseTestCase, unittest.TestCase):

    def setUp(self):
        super(MatchingContactCompany, self).setUp()

    def test_MatchingCompany(self):
        # form 1
        signup.obj = signupPage(self.driver)
        signup.obj.SignupPage1139_CompanyOne()
        # form 2
        signup.obj.SignupPage1139_CompanyTwo()
        # form 3 create the account and verify if the account was created
        signup.obj.SignupPage1139_CompanyThree()

   def tearDown(self):
       super(MatchingContactCompany, self).tearDown()

if __name__ == "__main__":
    unittest.main()

This is not how the Page Object pattern should be implemented:

page object needs to have abstractions over the visual elements on a page
you should not be asserting things in the page object methods

There are, of course, many different Python+Selenium Page Object pattern implementations and I am not completely sure, which one is the One, here are some(1) of(2) them(3).

We can, at least, start with defining locators and then reusing them inside the methods, something along these lines:

class SignupPage(Page):
    cname_input = (By.ID, "cname")

    username_input = (By.ID, "cf_2727675")
    password_input = (By.ID, "cf_2697953")  # assuming this is password

    submit_button = (By.ID, "sbbut")

    def __init__(self, driver):
        super(SignupPage, self).__init__(driver)

    def submit(self):
        self.driver.find_element(*self.submit_button).click()

    def sign_up(self):
        self.driver.get("https://abc.com/s_csignup.jsp?token=XVtQHUUGYFFaRhNaXxJaQVBRZEl2EnBo")

        cname = self.driver.find_element(*self.cname_input)
        cname.send_keys("QA EXTERNAL")

        username = self.driver.find_element(*self.username_input)
        username.clear()
        username.send_keys("Test")

        password = self.driver.find_element(*self.password_input)
        password.clear()
        password.send_keys("Test")

        self.submit()

Or, even better, we can create a class for every Page Element, like suggested in the documentation.

Note that the assertion itself should be moved to the test case method, and, hence, your Page class should not be derived from the unittest.TestCase (or have the assertEqual method).

Also, here are some notes about the code style of the code in the question:

if your BaseTestCase class inherits unittest.TestCase (it should), there is no need for multiple inheritance. And, you would not need to override the setUp and tearDown methods:

class BaseTestCase(unittest.TestCase):
    # ...

class MatchingContactCompany(BaseTestCase):
    def test_matching_company(self):
        # ...

according to PEP8, a class name should start with a capital letter and be in camel case - call it SignupPage instead of signupPage
the test_* methods should be named according to the PEP8 method name convention - it should be test_matching_company instead of test_MatchingCompany
same note about your page object methods - use a lower case and separate words with underscore


Optimizing execution time of merging images I am using the below code to merge two images, but it take avg of 6-7 sec for creating 16 images. Is there any way I can speed up my process?

$image1=imagecreatefrompng('img/'.$value);
$image2=imagecreatefrompng('img/'.$value1);
$w=imagesx($image1);
$h=imagesy($image1);
imagecopy($image1,$image2,0,0,0,0,$w,$h);
imagejpeg($image1,'final/'.$key.$key1.'.jpg');
imagedestroy($image1);

I also get a black backgroud. I do not want to add an extra layer as it will increase execution time.

Below is basic detail

1: images of size: 1000 * 1000
2: png + png = jpg image
3: different image will combine i had already coded in way to avoid dupliacy

below i what i am trying to do like:

http://www.gemvara.com/jewelry/emerald-isle-ring/emerald-cut-diamond-14k-white-gold-ring-with-diamond/by53

here you can see new product is combination of stone and ring base.
and has 4 different view i am trying to do like that

The best way to gain speed is to keep the resulting images, and use them again if the same images are needed. This will eliminate the merging on the second run altogther.

Since you don't give any clue as to what end you're doing this, I cannot start to guess how useful my suggestion is...

Your question would be better if we knew:

What your images look like, how big are they? (You've added this in, thanks)
Where do they come from?
What do you use the images for?

If you describe the situation in which you use your code, instead of just an isolated bit of code, then people can be much more creative in their answers. It is obvious there's not much we can improve in the plain and simple PHP code you show, so any solution would probably be found in the way you use the code.

Now that you've added some extra information to your question, it's easier to help you.

I assume, from what you say, that you're using the code only once, just after you've uploaded a new picture in your CMS, and you need to merge something into it.

The answer can be pretty short: Computers don't have unlimited processing power, so complex operations, like this, will take a long time.

There are some tips I can give:

Don't merge 16 times when you receive the image, but merge (once) when one on the 16 images is requested. This reduces the merger time to less than half a second per image.
Reduce the size of the images before merging. Images of 500 x 500 pixels will merge four times as fast as images of 1000 x 1000 pixels.
Use a background process to do the merging: https://florian.ec/articles/running-background-processes-in-php (use with care and only if there's really no other solution, which almost always does exists).


Python - Knight's tour recursive function I realize that there're many implementations of Knight's tour problem suggested across webs, without jumping to those suggested solutions (most in Java, I'm not proficient in Java), I managed to implement one myself using Python. It works and returns some results.

Description

  A knight's tour is a sequence of moves of a knight on a chessboard such that the knight visits every square only once. If the knight ends on a square that is one knight's move from the beginning square (so that it could tour the board again immediately, following the same path), the tour is closed, otherwise it is open.

For example:

board = Board(4,4)
a = list(dfs_search([0,0], tuple()))

Outputs:

# a[0]

([0, 0], [2, 1], [3, 3], [0, 2], [1, 0], [3, 1], [1, 2], [2, 0], [3, 2], [1, 1], [3, 0], [2, 3], [0, 3], [2, 2], [0, 1], [1, 3])

Most of the outputs are correct, which seems that the recursive function is actually working. The last few vertex, transit from [1,1] to [3,0] is problematic. I've spent hours debugging trying to resolve it, but failed. As the last resort, I bring the question here, hoping anyone could shed some light to my implementation.

This is my implemntaion of Knight's tour problem using recusive functions.

def dfs_search(start, path):
    if not path:
        path = (start,)
    if len(path) == board.goal:
        yield path

    for next_cell in board.avaiable_moves(start):
        if next_cell not in path:
            path += (next_cell,)
            yield from dfs_search(next_cell, path)

class Board(object):
    def __init__(self, width, length):
        self.width = width
        self.length = length
        self.goal = width * length

    def is_legit_move(self, cell):
        row, col = cell
        return (row >= 0 and row <= self.width-1) and (col >=0 and col <= self.length-1)

    def avaiable_moves(self, cell):
        ret = []
        r, c = cell
        tmp = [[r+1, c-2], [r+2, c-1], [r+2, c+1], [r+1, c+2],\
               [r-1, c-2], [r-2, c-1], [r-2, c+1], [r-1, c+2]]
        for elem in tmp:
            if self.is_legit_move(elem):
                ret.append(elem)
        return ret

I don't have much to say about this, only a few small nitpicks.

self.height might be a better name, (width and height) make more sense to me. But this might be subjective.
At is_ligit_move() you could use, elem in range(), because range is special. See this excellent description by @Graipher for more details.
At available_moves you could use list comprehension instead of the res variable

So your res would be this:

return [elem for elem in possible_moves if self.is_legit_move(elem)]

It does the same but looks a little cleaner. See PEP202 for more information.

Overall I think you have crafted a nice piece of code!


Creating the buttons on a calculator using a for-loop How can I make the for-loop more compact by using functions for actionlistener?

package räknare;
import javax.swing.JButton;
import javax.swing.JFrame;
import javax.swing.JTextField;
import javax.swing.SwingConstants;
import java.awt.event.ActionListener;
import java.awt.Font;
import java.awt.event.ActionEvent;

public class miniräknare {

    static JFrame frame;
    static JTextField textField;
    static JButton[] nummerButton;
    static String Inmatning= null;
    static double talOne;
    static double talTwo;
    static double resultat;
    static String räknesätt;
    static String svar;
    public static void main(String[] args) {
    Inmatning = new String();
    nummerButton=new JButton [10];
    frame=new JFrame("Kalkylatorn");
    frame.setBounds(100,100,270,450);
    frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
    frame.getContentPane().setLayout(null);
    textField= new JTextField();
    textField.setHorizontalAlignment(SwingConstants.RIGHT);
    textField.setBounds(10,22,234,62);
    frame.getContentPane().add(textField);
    textField.setColumns(10);

    for (int i=1 ; i<10 ; i++) {
        nummerButton[i]= new JButton(String.valueOf(i));
        int kolumn =((i-1)%3);
        int horisont=((i-1)/3);
        nummerButton[i].setBounds((10+(kolumn*60)),(220-(horisont*60)),50,50);
        nummerButton[i].setFont(new Font("Tahoma", Font.BOLD, 20));
        frame.getContentPane().add(nummerButton[i]);
        nummerButton[i].addActionListener(new ActionListener() {
            public void actionPerformed(ActionEvent e) {
                    JButton myButton = (JButton)e.getSource();
                    String Inmatning= textField.getText() + myButton.getText();
                                textField.setText(Inmatning);                                   }
        });
    }

Basics
Classes in Java naming conventions should start with an upper-case letter: miniräknare should be Miniräknare.
Variables in Java naming conventions should start with a lower-case letter: Inmatning should be inmatning.
Refactoring
There are a few things in your code that can be refactored to improve the readability. Functions are a part of that. Consider a function that adds a digits to the display:
private void addDigit(String digit) {
    textField.setText(textField.getText() + digit);
}

That function removes the display logic from the action listener, and puts it in the miniräknare class itself.
Another trick in Java is that interfaces with a single method, like an ActionListener are implicitly a functional interface. This allows instances of them to be declared in functional shorthand....
nummerButton[i].addActionListener(e -> addDigit(((JButton)e.getSource()).getText())

One more thing is that the font can be extracted to a constant...
private static final Font keyFont = new Font("Tahoma", Font.BOLD, 20);

Code order
I know this one is pedantic, but when you have "setup code" and "use code", you should put all the setup code first before you use the instance you have set up.
In your case, you have the line of code:

frame.getContentPane().add(nummerButton[i]);

before you add the ActionListener to nummerButton[i].
Also, I would avoid the nummerButton array entirely until the button is complete....
Result
    int kolumn =((i-1)%3);
    int horisont=((i-1)/3);
    JButton key = new JButton(String.valueOf(i));
    key.setBounds((10+(kolumn*60)),(220-(horisont*60)),50,50);
    key.setFont(keyFont);
    key.addActionListener(e -> addDigit(((JButton)e.getSource()).getText());

    nummerButton[i] = key;
    frame.getContentPane().add(key);

If you want, you can put all the JButton creation code in a new function too, but that may be too much refactoring in your case depending on how much other code is in your method....
private JButton createKey(int value) {
    int kolumn =((value-1)%3);
    int horisont=((value-1)/3);
    JButton key = new JButton(String.valueOf(value));
    key.setBounds((10+(kolumn*60)),(220-(horisont*60)),50,50);
    key.setFont(keyFont);
    key.addActionListener(e -> addDigit(((JButton)e.getSource()).getText());
    return key;
}

and then your calling code becomes:
    nummerButton[i] = createKey(i);
    frame.getContentPane().add(nummerButton[i]);


String 'expanding' - reinserting repeating characters The challenge: Given a String, for each digit in that original string, replace that digit with that many occurrences of the character following it. So a3tx2z yields attttxzzz.

My Solution:

import java.util.Scanner;

public class Standford1 {
    public static void main(String[] args) {
        Scanner input = new Scanner(System.in);

        System.out.print("Enter string to blowup: ");
        String userInput = input.nextLine();
        System.out.println(blowup(userInput));
    }

    public static String blowup(String str) {
        StringBuilder sb = new StringBuilder();
        for (int i = 0; i < str.length(); i++) {
            if (!isNumeric(str.substring(i, i + 1))) {
                sb.append(str.charAt(i));
            }
            else {
                sb.append(lengthen(str.charAt(i + 1),
                    Integer.parseInt(str.substring(i, i + 1))));
            }
        }
        return sb.toString();
    }

    public static boolean isNumeric(String str) {
        try {
            int num = Integer.parseInt(str);
        }
        catch(NumberFormatException nfe) {
            return false;
        }
        return true;
    }

    public static String lengthen(char c, int i){
        if (i == 0 || c == ' ') { return ""; }
        int count = 0;
        StringBuilder sb = new StringBuilder(c);
        while (count != i) {
            sb.append(c);
            count++;
        }
        return sb.toString();
    }
}

Could this be more readable? I alternate a lot between char and string (it seems optimal, am I wrong?), but should I just stick with one? Any and all feedback/suggested revisions/general optimizations welcomed with gratitude.

Edit: Stay within the specifications of the challenge. In the case of a number following a number this code is WAI i.e. the correct yield of blowup("a231") results in "a33111."

Edit 2: End digits just get ignored, since they have nothing to repeat, i.e. blowup("a2x3") would yield "axxx" - I realize I overlooked this case due to a question below, I have now amended this, but left the original question and code intact.

Edit 3: These should all be true:

/* Two plain cases */
System.out.println("Test 1: " + "xxaaaabb".equals(blowup("xx3abb")));
System.out.println("Test 2: " + "xxxZZZZ".equals(blowup("2x3Z")));
/* digit at the end */
System.out.println("Test 3: " + "axxx".equals(blowup("a2x3")));
/* Adjacent digits */
System.out.println("Test 4: " + "a33111".equals(blowup("a231")));
/* Digit include 0 */
System.out.println("Test 5: " + "aabb".equals(blowup("aa0bb")));
/* Unusual Characters */
System.out.println("Test 6: " + "AB&&,- ab".equals(blowup("AB&&,- ab")));
/* Empty String */
System.out.println("Test 7: " + "".equals(blowup("")));
/* Only digits */
System.out.println("Test 8: " + "".equals(blowup("2")));
System.out.println("Test 9: " + "33".equals(blowup("23")));

Note: So this isn't incorrectly perceived as a 'do this for me' question, as stated in edit 2,  I've already accounted for all cases.

Your instincts are good here, in almost all respects. Your basic algorithm is good, and your suspicions that you are changing too often between String an char is also good.

Using the StringBuilder is the right thing to do, well done. Breaking the String in to chars is also good, but the way you do that is inconvenient.... In part that's because there is a convenient function in the standard Java Character class that you will find useful: Character.isDigit(char)

With that function, you can throw out your isNumber() function, and simplify the whole thing down to chars, strings, and ints.... something like:

public static String blowup(String str) {
    StringBuilder sb = new StringBuilder();
    int repeat = 0;
    for (char c : str.toCharArray()) {
        if (Character.isDigit(c)) {
            repeat = repeat * 10 + Character.getNumericValue(c);
        } else {
            while (repeat > 0) {
                sb.append(c);
                repeat--;
            }
            sb.append(c);
        }
    }
    return sb.toString();
}

(I put this in an Ideone here)

Note that it also keeps a counter outside the loop. This removes the need for the 'ugly' i + 1 offset on a repeating character count.

The code still assumes valid input (where the last character cannot be a digit, or it is lost). Additionally, I tweaked it to handle multiple digit input.... like 20z ... for fun.

Update: In my mind, I immediately 'threw out' your isNumeric() method, because it was not needed... but I have only now just read it.... Eeugh! Yuck!

You should never use exception handling (try/catch) to handle normal program flow. The try/parse/catch logic you have is very slow to run. Throwing an exception for every non-digit character will be a lot of overhead. Even though Java has improved in terms of performance, when processing exceptions, it is still much slower than the alternatives, like a regular expression, or some other mechanism...

Update 2: working with the original specs (thus ignoring the multi-digit repeat), the code would be (here in a different Ideone):

public static String blowup(String str) {
    StringBuilder sb = new StringBuilder();
    int repeat = -1;
    for (char c : str.toCharArray()) {
        if (repeat < 0 && Character.isDigit(c)) {
            repeat = Character.getNumericValue(c);
        } else {
            do {
                sb.append(c);
            } while (--repeat >= 0);
        }
    }
    return sb.toString();
}

Notes for this version:

Use the 'repeat' as a flag set to negative for nothing. This allows for input like ab01 which will output ab1 (which would be impossible without the 0-repeat option).
Note that the repeat is now also used as a flag for the isDigit check.
a do-while loop instead of a while-do loop (as per Josay's comment).


Minimum window substring using python containing substr in any order Given an array of unique characters arr and a string str, implement a function getShortestUniqueSubstring that finds the smallest substring of str containing all the characters in arr. Return "" (empty string) if such a substring doesn’t exist.

This algorithm is used.

import collections
import sys

def get_shortest_unique_substring(substring, string):
  def trim_left(substring, window, count):
    ''' find how much left pointer can be moved in a window where all substring is present'''
    for left, c in enumerate(window):
      if c in count and count[c] > 1:
        count[c] -= 1
      elif c not in count:
        continue
      else:
        return left
    return 0

  substring = set(substring)
  ''' validating input '''
  if not substring or not string:
    return ""
  if len(substring) == len(string) and substring == set(string):
    return string

  window, min_window, min_window_len = [], [], sys.maxint
  left, right, count = 0, 0, collections.Counter()
  # to check if the initial minimum window has been found which has all the substring
  substr_found = set(substring)

  for right, c in enumerate(string):
    if c in substring:
      count[c] += 1
      if c in substr_found:
        substr_found.remove(c)
      # window has been found and now trim left pointer as much as possible maintaining the invariant that substr is still present between left and right pointers
      if not substr_found:
        left += trim_left(substring, string[left:right+1], count)
        # update the minimum window
        if right - left + 1 < min_window_len:
          min_window_len, min_window = right - left + 1, string[left:right+1]
        # we can't find better window than the length of substr
        if len(min_window) == len(substring):
          return "".join(min_window)
  return "".join(min_window)

Overview

Interesting algorithm.  I looked over it and tried to find a bit simpler of a way.    Basically, I implemented the modified version with a dictionary.  The dictionary stores the data in the formation char:index.  Every time we run into a relevant character we update the dictionary.  This eliminates all need for back tracking via trim_left. Every time we updated our dictionary of character locations, we would get a string by finding the min and max of the dictionary. If that string is smaller, then we assign it to the smallest variable.

Performance

Typically the newer version performs a lot faster.  This can be explained by referring to big O notation.  Yours, because of back tracking, performs at O(n^2) or worse. However, the newer version doesn't back track so much, and it sticks to only modifying one dictionary so the new version worst case scenario is O(n) efficiency. (Or, to be more specific, it is O(nm) where n is the length of your string and m is the length of the substring; however there is a very logical limit to the length of substring m, so it will probably never be worse than O(64n) [note: we convert the string to a set to remove duplicates]).

New Code

def newShortest(subString, string):
  myChars = set(subString)
  smallest = ""
  current={}
  for i, char in enumerate(string):
    if char in myChars:
      current[char] = i
      if len(current) == len(myChars):
        temp = string[current[min(current,key=current.get)]:current[max(current,key=current.get)]+1]
        if len(temp) < len(smallest) or len(smallest) == 0:
          smallest = temp
  return smallest

Code used to time your version vs modified version

I switched out the args for smaller and larger sizes:

import time

args = ["hello","I would like to all over you hehhhhhkkkkkkkkkklo"]

def timeIt(func,args):
  then = time.time()
  for _ in range(10000):
    func(*args)
  print("{} timing: {:.2f}".format(func.__name__, time.time()-then))

timeIt(newShortest,args)
timeIt(get_shortest_unique_substring,args)


The Bucketizer Script This code is written in a SSIS Script Component that basically accomplishes what I previously had as a T-SQL script, that was reviewed here.

I need to split a 80 character string that contains 20 buckets of 4 characters each, without any delimiter, and generate a new row for each bucket. I'm using a script component to achieve this in SSIS - here's the code:

using System;
using System.Linq;
using System.Text.RegularExpressions;
using System.Data;
using Microsoft.SqlServer.Dts.Pipeline;
using Microsoft.SqlServer.Dts.Pipeline.Wrapper;
using Microsoft.SqlServer.Dts.Runtime.Wrapper;

[SSISScriptComponentEntryPoint]
public class ScriptMain : UserComponent
{
    /// <summary>
    /// This code splits a row's [Sizes] column into buckets of 4 characters,
    /// and adds a new row to the output buffer for each non-empty bucket found.
    /// </summary>
    /// <param name="Row">The row that is currently passing through the component</param>
    public override void Input0_ProcessInputRow(Input0Buffer Row)
    {
        var buckets = Regex.Match(Row.Sizes, "^(?<bucket>.{4})+$")
                           .Groups["bucket"].Captures
                           .Cast<Capture>()
                           .Select(capture => capture.Value)
                           .ToArray();
        try
        {
            for (int index = 0; index < buckets.Length; index++)
            {
                var bucket = buckets[index].Trim();
                if (bucket.Contains(' '))
                {
                    // size codes don't contain whitespaces; something went really wrong if that's the case:
                    throw new FormatException("Size code '" + bucket +"' contains a whitespace. This is interpreted as a parse error.");
                }

                if (string.IsNullOrEmpty(bucket))
                {
                    // don't output a row for empty buckets
                    continue;
                }

                OutputSizesBuffer.AddRow();
                OutputSizesBuffer.ETLStart = Row.ETLStart;
                OutputSizesBuffer.SourceId = Row.SourceId;
                OutputSizesBuffer.SourceTable = Row.SourceTable;
                OutputSizesBuffer.SizeRangeId = Row.SizeRangeId;
                OutputSizesBuffer.SizeRangeIndex = index + 1; // make it 1-based.
                OutputSizesBuffer.Code = bucket;
            }
        }
        catch (Exception exception)
        {
            var cancel = false;
            ComponentMetaData.FireError(0, "Script Component", exception.ToString(), string.Empty, 0, out cancel);
        }
    }

}

I would like this code to be reviewed from a performance perspective, because I will be needing very similar script components soon, to process hundreds of thousands of rows; this specific instance works well, but only processes the "master data", which is only a dozen rows or so: if there's a performance issue with this code, it's only later that I'm going to find out - I'd rather get it peer reviewed now and make adjustments before I use similar code to process many, many rows.

Is the error/exception handling overkill? Is the regex pattern self-documenting? (wait, is that even possible?)

The regex silently fails on inputs that don't have length a multiple of 4; i.e. no results are returned. Things that fail silently are incredibly hard to debug. It's easy to imagine trying to figure out why no rows are being inserted for the input

jvqbjqbxvvlxlcwkuivasoeljaxamrthcecbudnxvqdboxroudfvrwoureqisaldcduhauoqtxdclcw

until you realise that it only has 79 characters.

You would want to measure the performance, but I would suggest not using a regex and writing a simple method, like so

private static IEnumerable<string> ToBuckets(string input)
{
    const int Length = 4;
    for (var i = 0; i < input.Length; i += Length)
    {
        yield return input.Substring(i, Length);
    }
}

We should add input validation to make it easier to find out exactly what went wrong.

if (input == null)
{
    throw new ArgumentNullException("input");
}

if (input.Length % Length != 0)
{
    throw new ArgumentException(string.Format("Argument length must be a multiple of {0}", Length));
}

I made a microbenchmark comparing the two methods that you can find here. For 100,000 calls, the regex solution takes ~2.5s compared to ~0.2s for this version.

It looks like you're using exceptions for control flow with the FormatException. I would suggest making the try-catch more specific, both in the code that is executed within the try, and in the exceptions that you're catching.

If we split out the OutputSizesBuffer parts to its own method, we end up with something like this, which I find a bit cleaner (illustrative code only)

var buckets = ToBuckets(Row.Sizes).Select(bucket => bucket.Trim()).ToArray();
for (int index = 0; index < buckets.Length; index++)
{
    // Don't output a row for empty buckets.
    if (bucket.Length == 0)
    {
        continue;
    }

    if (bucket.Contains(' '))
    {
        // Size codes don't contain whitespace; something went really wrong if that's the case.
        FireError(string.Format(Resources.BucketContainsWhiteSpace, bucket));
        return;
    }

    try
    {
        AddRow(index + 1, bucket);
    }
    catch (SomeSpecificException e)
    {
        FireError(e.Message);
        return;
    }
}


Is this validation sufficient to ensure that the vals are not empty? I'm working on a small winforms app, the goal of which is to capture data and write to xml. I'm still a child where programming is concerned so could you guys please take a look and suggest changes as I'm pretty sure that this is not a good approach.
I'm using a masked-textbox for the invoice number which should only contain numbers. I'm trying not to use a numupdown for obvious reasons.

InputVars is a public class with empty string properties (string value {get;set;})

    private bool TestVal(string testedVal)
    {
        if (!String.IsNullOrEmpty(testedVal))
        {
            return true;
        }
        else
        {
            return false;
        }
    }

    private InputVars SetSemVars()
    {
        InputVars inVars = new InputVars();
        if (TestVal(txtCapt.Text))
        {
            inVars.captured = txtCapt.Text;
        }
        else
        {
            MessageBox.Show("Please insert your name here");
            txtCapt.Focus();
        }
        if (TestVal(mtxtInvoiceNumber.Text))
        {
            inVars.invoice = mtxtInvoiceNumber.Text;
        }
        else
        {
            MessageBox.Show("Please insert the invoice number here");
            mtxtInvoiceNumber.Focus();
        }
        if(TestVal(cmbNetwork.SelectedIndex.ToString()))
        {
            inVars.network = cmbNetwork.SelectedIndex.ToString();
        }
        else
        {
            MessageBox.Show("Please select the network ");
            cmbNetwork.Focus();
        }
        if(TestVal(cmbRegion.SelectedIndex.ToString()))
        {
            inVars.region = cmbRegion.SelectedIndex.ToString();
        }
        else
        {
            MessageBox.Show("Please select your office code here");
            cmbRegion.Focus();
        }
        if(TestVal(cmbSupplier.SelectedIndex.ToString()))
        {
            inVars.supplier = cmbRegion.SelectedIndex.ToString();
        }
        else
        {
            MessageBox.Show("Please select the supplier here");
            cmbSupplier.Focus();
        }
        return inVars;
    }

I was playing around earlier and got this to work , just thought I'd share

        foreach (TextBox box in this.panel1.Controls.OfType<TextBox>())
        {
            if (String.IsNullOrWhiteSpace(box.Text))
            {
                box.Text = "Enter value here";
                //or do a messagebox etc.
                box.Focus();
                return;
            }

        }

that`s loads and loads shorter and easier to read and makes sure that I don't forget a value... me gusta

Don't use String.IsNullOrEmpty to validate required values.
You can create a method

private bool IsEmpty(string value, string errorMessage, Control controlToValidate)
{
    if ((value ?? string.Empty).Trim().Length == 0)
    {
        MessageBox.Show(errorMessage);
        controlToValidate.Focus();
        return true;
    }
    else
    {
        return false;
    }
}

instead of TestVal.
Consider using Validating event for validating your controls and the Tag  property to associate error messages with the controls.
Consider using ErrorProviders.


Factorials and Powers calculator I just started learning Haskell, and I made a simple program which can calculate factorials and powers:

Input:

What do you want to do (0=quit, 1=factorial, 2=power) 1
Enter a number: 10
The factorial of 10 is: 3628800
What do you want to do (0=quit, 1=factorial, 2=power) 2
Enter the base number: 2
Enter the exponent: 10
2^10 = 1024.0
What do you want to do (0=quit, 1=factorial, 2=power) 0

The code is:

import System.IO
import System.Exit
import Control.Monad

factorial :: Integer -> Integer
factorial n = product [1..n]

powerInternal :: Double -> Integer -> Double
powerInternal b 0 = 1
powerInternal b e = b * powerInternal b (e - 1)

power :: Double -> Integer -> Double
power b e = if e >= 0
  then powerInternal b e
  else 1 / powerInternal b (-e)

main :: IO ()
main = do
  hSetBuffering stdout NoBuffering

  putStr "What do you want to do (0=quit, 1=factorial, 2=power) "
  input <- getLine

  when (read input == 0) exitSuccess

  if read input == 1
    then do
    putStr "Enter a number: "
    hFlush stdout

    num <- getLine
    putStrLn ("The factorial of " ++ num ++ " is: " ++ show (factorial (read num)))
  else do
    putStr "Enter the base number: "

    base <- getLine
    putStr "Enter the exponent: "
    expe <- getLine

    putStrLn (base ++ "^" ++ expe ++ " = " ++ show (power (read base) (read expe)))

  main

I am looking for suggestions on how to improve the code, make it shorter, ...

The rise for power

Let's start with powerInternal. First of all, there's a better algorithm for \$b^e\$, which takes \$\mathcal O(\log e)\$ instead of \$\mathcal O(e)\$:

$$b^e = \begin{cases}\left(b^2\right)^\frac{e}{2},&e~\text{even}\\
b\left(b^2\right)^\frac{e-1}{2},&e~\text{odd}\\
\end{cases}
$$

Implementing this is left as an exercise. But it seems like you want to use powerInternal only in power, since it doesn't work with negative exponents. Therefore, you should reduce its scope:

power :: Double -> Integer -> Double
power b e = if e >= 0
    then powerInternal b e
    else 1 / powerInternal b (-e)
  where
    powerInternal :: Double -> Integer -> Double
    powerInternal b 0 = 1
    powerInternal b e = b * powerInternal b (e - 1)

However, we're reinventing the wheel. There's already a function of type

(Fractional a, Integral b) => a -> b -> a

that supports negative exponents. It's called (^^). So we can change your power function:

power :: Double -> Integer -> Double
power b e = b ^^ e
-- or, shorter, but less readable
-- power = (^^)

This is probably the shortest variant you can get. Onwards

Don't trust the user

What do you want to do (0=quit, 1=factorial, 2=power) abc
Run.hs: Prelude.read: no parse

Users will provide wrong input. Either because they intended to do so, or because their finger slipped and they accidentally put 12 instead of 1. If you don't want your program to crash, you probably want to ask again instead. Therefore, we should probably use a function:

askUser :: IO Int
askUser = do
  putStr "What do you want to do? (0: quit; 1: factorial; 2: power) "
  input <- getLine

We have now several ways to handle this. We could use case

  case input of
    "0" -> return 0
    "1" -> return 1
    "2" -> return 2
    _   -> putStrLn "Wrong input, please try again" >> askUser

but that's error prone. Instead, we use readMaybe from Text.Read:

  let input' = readMaybe input

  if 0 <= input' && input' <= 2
     then return input'
     else putStrLn "Wrong input, please try again" >> askUser

We can shorten it if we use fmap readMaybe getLine:

askUser :: IO Int
askUser = do
  putStr "What do you want to do? (0: quit; 1: factorial; 2: power) "
  input <- fmap readMaybe getLine

  case input of
     Just x | 0 <= x && x <= 2 -> return x
     _                         -> putStrLn "Wrong input, please try again" >> askUser

Now we know that askUser will always return a number in the range 0-2.

Foul magic

However, what's 0? What is 1? All three numbers are magic numbers, which is usually a code smell. You, as a developer, have to remember what a certain number stands for. Save them somewhere, like

userQuit, userFactorial, userPower :: Int
userQuit      = 0
userFactorial = 1
userPower     = 2

Now you can write

if answer == userQuit

instead of

if answer == 0

But aren't the choices of the user rather limited in comparison to Int? How about

data UserChoice = Quit | Factorial | Power deriving (Enum, Bounded, Show)

instead? Now we can use

askUserChoice :: IO UserChoice
askUserChoice = fmap toEnum askUser

to convert 0-2 to our own enumeration.

Now, with askUserChoice, we get the following main:

main :: IO ()
main = do
  hSetBuffering stdout NoBuffering

  input <- askUserChoice

  case input of
    Quit      -> exitSuccess
    Factorial -> userFactorial >> main
    Power     -> userPower     >> main

I will come to the yet unknown functions in a second. What did we gain so far? Well, input is going to be a valid answer when we try to inspect it, and we can pattern match on it. Even better, the compiler can tell us that we forgot a pattern if we decide to add Addition or something similar later.

Splitting hairs and concerns

Now to the user* functions. They are pretty much the same as yours, with a small twist:

userFactorial :: IO ()
userFactorial = do
    num <- getInteger "a number"
    putStrLn ("The factorial of " ++ show num ++ " is: " ++ show (factorial num))

userPower :: IO ()
userPower = do
    base <- getInteger "the base number"
    expe <- getInteger "the exponent"
    putStrLn (show base ++ "^" ++ show expe ++ " = " ++ show (power base expe))

The twist is, that getInteger will ask the user till they actually provide an integer:

getInteger :: String -> IO Integer
getInteger what = do
    putStrLn ("Enter " ++ what ++ ": ")
    hFlush stdout

    input <- fmap readMaybe getLine

    case input of
      Nothing -> putStrLn "Not a number, please try again." >> getInteger what
      Just x  -> return x

Summary

So, what has been improved?

The power function uses (^^), a standard library function
We check user input for validity
We don't read values multiple times
We show our intentions for the user's action with a data type
We provide a succinct main and let other functions handle the actual work
We share common problems (getting an integer from the user) and don't repeat ourselfs.

From a beginners perspective, your code is fine, by the way. Some will tell you to use ($) instead of parentheses in your putStrLn lines, but that's a personal preference. The only two real problems where the suboptimal power, and that you don't actually check user input.

Bonus: Short alternative main

Well, the data type and so on is an overkill. We can check the initial decision a lot easier if we simply don't convert the result of getLine into a number (or another type):

main = do
  putStr "What do you want to do (0=quit, 1=factorial, 2=power) "
  input <- getLine

  case input of
    "0" -> exitSuccess
    "1" -> userFactorial >> main
    "2" -> userPower     >> main
    _   -> putStrLn "Wrong input, try again!" >> main

The key to getting main succinct here is still to split the code into another function. You can provide the code inline, but that's hardly readable anymore.


Bash script for reviewing .pdf articles I wrote a bash script for reviewing scientific articles. It adds a blank page for notes and creates a new landscape file. I'm sure it can be made better code wise. Any suggestions?

#!/bin/bash

if [ $# -ne 1 ]
then
  echo "Usage example: ./bashscript src.pdf"
  exit $E_BADARGS
else
  NUM=$(pdftk $1 dump_data | grep 'NumberOfPages' | awk '{split($0,a,": "); print a[2]}')
  COMMSTR=''

  for i in $(seq 1 $NUM);
  do
    COMMSTR="$COMMSTR A$i B1 "
  done
  $(echo "" | ps2pdf -sPAPERSIZE=a4 - pageblanche.pdf)
  $(pdftk A=$1 B=pageblanche.pdf cat $COMMSTR output 'mod_'$1)
  (pdfnup 'mod_'$1 --nup 2x1 --landscape --outfile 'print_'$1)
  $(rm pageblanche.pdf && rm 'mod_'$1)

fi

#for f in *.pdf; do ./bashscript.sh $f; done 2> /dev/null

First of all "bashscript" is a very poor name:

It doesn't tell what the script does. "prepare-articles-for-review.sh" would be better. Or just "prepare-articles.sh"
It is customer to use .sh extension for Bash scripts

If your typical use case is this:

for f in *.pdf; do ./bashscript.sh $f; done 2> /dev/null

Then it would be better to make the script handle multiple file parameters.

  exit $E_BADARGS

I'm not familiar with such variable. I don't think it's a standard, and it's not mentioned in my man bash. If the variable is undefined (and I think it is), then this will have the same effect as exit, which is equivalent to exit $?, where $? is the exit code of the last statement, in this case the echo on the previous line, which is most probably 0. I think you intended to exit with failure, with non-zero exit code.

  NUM=$(pdftk $1 dump_data | grep 'NumberOfPages' | awk '{split($0,a,": "); print a[2]}')

Many problems here:

You should double-quote "$1", to protect from spaces in the filename
The quoting in grep 'NumberOfPages' is redundant
Are you sure there won't be more than one lines matching "NumberOfPages"? Just to be safe, I would add exit in the Awk command
The Awk command can be written simpler as awk -v FS=": " '{print $2}'

  COMMSTR=''

You can simplify to:

  COMMSTR=

  for i in $(seq 1 $NUM);

seq is not recommended because it's not portable. You can achieve the same using native Bash functionality:

for ((i=1; i<=$NUM; i++));

  $(echo "" | ps2pdf -sPAPERSIZE=a4 - pageblanche.pdf)

The $(...) wrapping is really pointless, and awful.
No need for echo "", simply echo is exactly the same.

  $(pdftk A=$1 B=pageblanche.pdf cat $COMMSTR output 'mod_'$1)

Does this work at all? Unfortunately I don't have pdftk so cannot try.
Maybe it's fine, but it looks suspicious.

Btw, you don't need the quotes in 'mod_'$1.
And again, the $(...) wrapping is pointless.

  (pdfnup 'mod_'$1 --nup 2x1 --landscape --outfile 'print_'$1)

As earlier, you don't need those quotes,
and the (...) wrapping is pointless.

The same goes for the rest of the code.

Suggested implementation

I cannot fully test this because I don't have pdftk, but it should work:

#!/bin/bash

if [ $# = 0 ]
then
  echo "Usage: $0 file1.pdf file2.pdf ..."
  exit 1
fi

for file; do
    NUM=$(pdftk "$file" dump_data | awk -v FS=": " '/NumberOfPages/ { print $2; exit }')
    COMMSTR=

    for ((i=1; i<=$NUM; i++)); do COMMSTR="$COMMSTR A$i B1 "; done

    blank=blank.pdf
    ps2pdf -sPAPERSIZE=a4 - $blank < /dev/null
    pdftk A="$file" B=$blank cat $COMMSTR output mod_"$file"
    pdfnup mod_"$file" --nup 2x1 --landscape --outfile print_"$file"
    rm $blank && rm mod_"$file"
done


Qt foreach-like alternative to iterate over value AND key of an associative container The Qt documentation recommend an iterator-based solution to iterate over an associative container like QMap and QHash, and I always wondered if there really isn't a (nice) solution using a foreach loop like in PHP:

foreach($container as $key => $value)

I looked into the source code of the foreach macro and extended it by a key variable. The following code is only the GCC version of the loop (it doesn't work with all compilers, see Q_FOREACH macro definition in qglobal.h for other versions).

#define foreachkv(keyvar, variable, container)                        \
for (QForeachContainer<__typeof__(container)> _container_(container); \
     !_container_.brk && _container_.i != _container_.e;              \
     __extension__  ({ ++_container_.brk; ++_container_.i; }))        \
    for (keyvar = _container_.i.key();; __extension__ ({break;}))     \
        for (variable = *_container_.i;; __extension__ ({--_container_.brk; break;}))

Now you can do the following:

QVariantHash m;
m.insert("test", 42);
m.insert("foo", true);
foreachkv(QString k, QVariant v, m)
    qDebug() << k << "=>" << v;

The output will be:

"foo" => QVariant(bool, true)
"test" => QVariant(int, 42)

Tests I've done so far:

examples from above
foreachkv works as expected with break and continue statements within the loop.

What did I do? I just added another for loop to introduce the key variable (called keyvar in the macro). Maybe I don't need this loop?

You can probably do something simpler to avoid making this a special case macro, e.g. using std::tie or boost::tie if that's not available:

#include <map>
#include <functional>
#include <boost/foreach.hpp>
#include <iostream>

int main() {
  std::map<int, std::string> map = {std::make_pair(1,"one"), std::make_pair(2,"two")};
  int k;
  std::string v;
  BOOST_FOREACH(std::tie(k, v), map) {
    std::cout << "k=" << k << " - " << v << std::endl;
  }
}


Improving an itoa function I wrote an itoa function yesterday, which based on feedback I am now working on improving. How does the below look? Are there any ways in which I can improve it?
#include <stdio.h>
#include <string.h>
#include <limits.h>

char* itoa(int number, size_t sz, char buffer[sz])
{
    // do error handling and edge cases
    if (buffer == NULL || sz == 0)
        return NULL;
    else if (number == 0)
        return strcpy(buffer, "0");

    char tmp[sz];
    int idx = 0;
    bool is_negative = (number < 0);
    if (!is_negative) number = -number; // so we can handle INT_MIN

    for (; number != 0; number/=10, idx++) {
        if (sz-- == 0) return NULL;
        tmp[idx] = '0' - (number % 10); // negative version of number + '0'
    }
    if (is_negative) {
        if (sz-- == 0) return NULL;
        tmp[idx++] = '-';
    }

    // reverse
    for (int i=0; i<idx; i++)
        buffer[idx-i-1] = tmp[i];
    buffer[idx] = '\0';
    return buffer;
}

int main(void)
{
    char buffer[100];
    int test[] = {123, 234, 0, 17, -4, -7, -17, -27, INT_MAX, INT_MIN};
    size_t len = sizeof(test) / sizeof(*test);

    // give it a small buffer and see how it works
    for (int i=0, num; i < len; i++) {
        num = test[i];
        char* str = itoa(num, 2, buffer);
        printf("%-10d <%s>\n", num, str);
    }

    // now give it the correctly-sized buffer
    for (int i=0, num; i < len; i++) {
        num = test[i];
        char* str = itoa(num, sizeof buffer / sizeof *buffer, buffer);
        printf("%-10d <%s>\n", num, str);
    }
}

Working code here: https://onlinegdb.com/H1xKCemNu. A few questions about this:

It seems like there are just so many edge cases to consider for the most trivial of functions to write in C. For example, the above function took me about two hours to write (and was after some very helpful feedback from @chux). Is that a common issue in writing C code? Or is this more a beginner issue that will go away with experience?
With the for loop I often find myself being bitten by an off-by-one error, for example, not realizing the idx is incremented after the previous loop but before the next loop is checked. Are there particular ways to look out for that?
Any other areas to improve here?

Incorrect size check
itoa(num, 2, buffer); forms "17", which needs 3 char, even though only 2 provided.
Perhaps OP meant 2 to be the string length allowed and not the string size needed.  In which case sizeof buffer / sizeof *buffer is not the length allowed.
Missing header
Add #include <stdbool.h>
Missing size check
Corner case: strcpy(buffer, "0"); occurs even if sz == 1.

A simple alternative to buffer overflow concerns is to form the initial characters in a local buffer that is right-sized for worst case (INT_MIN) like char local_buf[12].  Check idx against sz.  Then reverse into the true destination.
// Re-write end-of-code.
// ....
// Prior code does not check `sz` nor change sz as characters are saved in ample sized local_buf

if (idx + 1 > sz) {
  if (sz > 0) buffer[0] = 0;  // Set buffer[] to "" if possible
  return NULL;
}

for (int i=0; i<idx; i++)
    buffer[idx-i-1] = local_buf[i];
buffer[idx] = '\0';
return buffer;

Rather than hard-code 12 use code to scale to the size needed.  E.g.
// Scale by log10(2)
#define INT_STR_SZ (sizeof(int)*CHAR_BIT*301030/1000000 + 3)
char local_buf[INT_STR_SZ];


Converts numbers in range 0 through 15 into ASCII-coded character '0' through 'F' This is an assignment and i have implemented the function hexasc from the description above. To run this code I'm using MARS-simulator.
For the sake of understanding it better i would appreciate to see different approach.

  Write a small subroutine, that converts numbers in the range 0 through 15 into a printable ASCII-coded character: '0' through '9', or 'A' through 'F', depending on the number. For numbers not in the range 0 through 15, some bits will be ignored.

  The function must be called hexasc.
  Parameter: One, in register $a0.
  The 4 least significant bits specify a number, from 0 through 15. All other bits in register $a0 can have any value and must be ignored. Return value: The 7 least significant bits in register $v0 must be an ASCII code as described below. All other bits must be zero when your function returns.

  Required action: The function must convert input values 0 through 9 into the ASCII codes for digits '0' through '9', respectively. Input values 10 through 15 must be converted to the ASCII codes for letters 'A' through 'F', respectively.

.text
main:
li      $a0,10      # change this to test different values

jal     hexasc      # call hexasc
nop             # delay slot filler (just in case)

move    $a0,$v0     # copy return value to argument register

li      $v0,11      # syscall with v0 = 11 will print out
syscall         # one byte from a0 to the Run I/O window

stop:   j   stop            # stop after one run
nop             # delay slot filler (just in case)

hexasc:
andi    $a0,$a0,0xf     #only 4 least significant bits ignore other bits
addi    $v0,$zero,0x30  #$v0 = 0x30 ('0')
addi    $t0,$zero,0x9   #t0 = 0x9

ble     $a0,$t0,L1      #branch if a0 <= 0x9
nop
addi    $v0,$v0,0x7     #v0 = v0 +0x7

L1:
add     $v0,$a0,$v0     #v0 = V0 +a0
jr      $ra
nop

I see a few things that may help you improve your code.

Don't alter registers unless specified

The current code alters the $a0 register, which is appropriate to the problem, but the specification doesn't say that the register may be altered.  I'd suggest not altering passed in registers unless the specification says to.

Know your instruction set

The ble instruction can also work with a constant, so there's no real need to load a register with the value 9.  Also, it would be better to simply use a move instruction than the curious construct addi $t0,$zero,0x9.

Avoid nop instructions

I don't know of a reason that would require nop instructions where this code has them, so I'd recommend eliminating them.

Minimize register use

One of the most important things to keep track of in assembly language programming is register usage.  In this particular case, I'd suggest that there is no need to use more than a single register.  Here's how I'd do it:

hexasc:
andi $v0,$a0,0xf        # save only low 4 bits
addi $v0,$v0,0x30   # turn into ASCII
ble $v0,0x39,L2     # if it's > ASCII '9'
addi $v0,$v0,7      #   add additional 7 for 'A'-'F'
L2:
jr $ra


Recursive implementation of merge sort Information about my code:

I am following this MIT OCW algorithms course. The first lecture described insertion sort and merge sort. I implemented merge sort in C.

The algorithm is structured as a function called from the main function. The array to be sorted is allocated dynamically in the main function but can also be statically allocated.

What I am looking for:

I am looking whether the 2 functions can be optimized without changing the algorithm(merge sort), whether it follows the best-practices of programming in C and does it have proper readability factor?

#include<stdio.h>
#include<stdlib.h>

#define SORTING_ALGO_CALL merge_sort

void merge_parts(int arr[], int length)
{
    /*
    Sorts into increasing order
    For decreasing order change the comparison in for-loop
    */
    int ans[length];
    //This for and next if-else puts the merged array into temporary array ans
    //in a sorted manner
    int i, k;
    int temp, j = temp = length/2;
    for (i = k = 0; (i < temp && j < length); k++){
        ans[k] = (arr[i] < arr[j]) ? arr[i++] : arr[j++];
    }

    if(i >= temp){
        while(j < length){
            ans[k++] = arr[j++];
        }
    }
    else{
        while(i < temp){
            ans[k++] = arr[i++];
        }
    }

    //This for-loop puts array ans into original array arr
    for(i = 0; i < length; i++){
        arr[i] = ans[i];
    }
}

void merge_sort(int arr[], int length)
{
    if(length > 1)
    {
        merge_sort(&arr[0], (length/2));
        merge_sort(&arr[length/2], (length - length/2));
        merge_parts(arr, length);
    }
}

int main()
{
    int length;
    scanf("%d", &length);
    while (length < 1)
    {
        printf("\nYou entered length = %d\n", length);
        printf("\nEnter a positive length: ");
        scanf("%d", &length);
    }

    int *arr;
    if ((arr = malloc(sizeof(int) * length)) == NULL)
    {
        perror("The following error occurred");
        exit(-1);
    }

    for (int i = 0; i < length; i++){
        scanf("%d", &arr[i]);
    }

    SORTING_ALGO_CALL(arr, length);

    for (int i = 0; i < length; i++){
        printf("%d ", arr[i]);
    }

    free(arr);
    return 0;
}

The book I am reading is Introduction to Algorithms 3rd Edition by Cormen. It mentions a sentinel value be placed at the end of two subarrays. That, if implemented, it can result in better efficiency. But I am not sure what to use as a sentinel value.

I've slightly reorganised your code to make it easier to follow.

void merge_parts(int arr[], int length)
{
    /*
    Sorts into increasing order
    For decreasing order change the comparison in for-loop
    */
    int ans[length];
    //This for and next if-else puts the merged array into temporary array ans
    //in a sorted manner
    int mid = length/2;
    int i = 0, k = 0, j = mid;
    while (i < mid && j < length){
        ans[k++] = (arr[i] < arr[j]) ? arr[i++] : arr[j++];
    }

    // Only one of the two loops will apply
    while(j < length){
        ans[k++] = arr[j++];
    }
    while(i < mid){
        ans[k++] = arr[i++];
    }

    //This for-loop puts array ans into original array arr
    for(int i = 0; i < length; i++){
        arr[i] = ans[i];
    }
}

void merge_sort(int arr[], int length)
{
    if(length > 1)
    {
        int mid = length/2;
        merge_sort(arr,     mid);
        merge_sort(arr+mid, length - mid);
        merge_parts(arr, length);
    }
}

int main()
{
    int length;
    for (;;)
    {
        printf("\nEnter a positive length: ");
        scanf("%d", &length);
        printf("\nYou entered length = %d\n", length);
        if (length>=1) break;
    }

    int *arr = malloc(sizeof(int) * length);
    if (!arr)
    {
        perror("The following error occurred");
        exit(-1);
    }

    for (int i = 0; i < length; i++){
        scanf("%d", &arr[i]);
    }

    merge_sort(arr, length);

    for (int i = 0; i < length; i++){
        printf("%d ", arr[i]);
    }

    free(arr);
    return 0;
}

Then for the only optimisation I can think of, you could avoid some copying : when j < length after the main loop, it corresponds to a situation where the end of arr is already sorted in the right place. Thus, you don't need to copy it from arr to ans and then from ans to arr.

int mid = length/2;
int i = 0, k = 0, j = mid;
while (i < mid && j < length){
    ans[k++] = (arr[i] < arr[j]) ? arr[i++] : arr[j++];
}

while(i < mid){
    ans[k++] = arr[i++];
}

//This for-loop puts array ans into original array arr
for(i = 0; i < j; i++){
    arr[i] = ans[i];
}


Calculate grades based on pass/fail percentage Our school grading scale is from 1..10 with one decimal. If you do nothing you still get a grade of 1.0. A passing grade equals 5.5. A 'cesuur' percentage defines at what percentage of correct answers the 5.5 will be given to a student.

Examples:

List item
Grade(0,100,x) should always result in 1.0
Grade(100,100,x) should always results in 10.0
Grade(50,100,0.5) should result in 5.5

Questions: how can I simplify the code? How can I make it more robust?

Public Function Grade(Points As Integer, MaxPoints As Integer, Cesuur As Double) As Double
    Dim passPoints As Integer
    Dim maxGrade As Integer
    Dim minGrade As Integer
    Dim passGrade As Double
    Dim base As Double
    Dim restPoints As Integer
    Dim restPass As Double

    passPoints = Cesuur * MaxPoints
    maxGrade = 10
    minGrade = 1
    passGrade = (maxGrade + minGrade) / 2
    base = maxGrade - passGrade
    If Points < passPoints Then
        Grade = 1 + (passGrade - minGrade) * Points / passPoints
    Else
        restPoints = MaxPoints - Points
        restPass = MaxPoints * (1 - Cesuur)
        Grade = maxGrade - restPoints * base / restPass
    End If
    Grade = Round(Grade, 1)
End Function

The function's parameters are implicitly passed by reference, which probably isn't the intent since none of the parameters are assigned/returned to the caller.

Signature should pass its parameters by value, like this:

Public Function Grade(ByVal Points As Integer, ByVal MaxByVal As Integer, ByVal Cesuur As Double) As Double

maxGrade and minGrade are only ever assigned once - they're essentially constants and could be declared as such:

Const MAXGRADE As Integer = 10
Const MINGRADE As Integer = 1

I would suggest to declare variables closer to their usage, and perhaps only assign the function's return value in one place.

Variables restPoints and restPass are only ever used with a passing grade, in that Else block. VBA doesn't scope variables at anything tighter than procedure scope, so you could extract a method to calculate a passing grade, but that's borderline overkill - here's what it would look like, with parameter casing to camelCase:

Option Explicit
Private Const MAXGRADE As Integer = 10
Private Const MINGRADE As Integer = 1

Public Function Grade(ByVal points As Integer, ByVal maxPoints As Integer, ByVal cesuur As Double) As Double

    Dim passPoints As Integer
    passPoints = cesuur * maxPoints

    Dim passGrade As Double
    passGrade = (MAXGRADE + MINGRADE) / 2

    Dim base As Double
    base = MAXGRADE - passGrade

    Dim result As Double

    If points < passPoints Then
        result = 1 + (passGrade - MINGRADE) * points / passPoints
    Else
        result = CalculatePassingGrade(MAXGRADE, base, points, maxPoints, cesuur)
    End If

    Grade = Round(result, 1)

End Function

Private Function CalculatePassingGrade(ByVal base As Double, ByVal points As Integer, ByVal maxPoints As Integer, ByVal cesuur As Double) As Double

    Dim restPoints As Integer
    restPoints = maxPoints - points

    Dim restPass As Double
    restPass = mxPoints * (1 - cesuur)

    CalculatePassingGrade = MAXGRADE - restPoints * base / restPass

End Function


Error handling for parsing a date Is this return style acceptable? Would the code be more readable if I assigned the MonthDay.parse() return value to a variable and returned that variable on the last line?

Also, is it a bad idea to both log and rethrow inside a catch block?

/**
 * Parses showing date from the header text.
 *
 * @throws PageStructureException if the date string does not conform to the expected format
 */
public MonthDay parseShowingDate( String date ) throws PageStructureException {
    Preconditions.checkNotNull( date );

    DateTimeFormatter matDTF = DateTimeFormat.forPattern( DATE_FORMAT );
    String dayAndMonth = removeDayOfWeek( date );
    try {
        return MonthDay.parse( dayAndMonth, matDTF );
    } catch ( IllegalArgumentException iae ) {
        logger.error( "Could not parse MAT date {}, expected format [{}].", date, DATE_FORMAT );
        throw new PageStructureException( iae );
    }
}

I believe it's OK. This way, it is immediately clear from the code that the method returns a value only when no exception is thrown during parsing. If you used a variable to store the result, it becomes less clear. And, the reader has to track where the variable is initialized/modified/returned.

Logging an exception when it's rethrown is a bit confusing, because it is possible the caller expect the possibility of receiving an exception and deals with it. But sometimes a programmer needs it so that it's possible to examine what's happening even if the caller catches the exception. In such a case, I'd suggest using only debug or trace logging levels.