Compensating for typos in strings by removing spaces and making uppercase for comparison I'm working with a database that has typos in the values. To compensate for them I've made a function called standardizer() which removes spaces and converts all letters to uppercase. I do this so the value red in from the database can correctly be interpreted by the program. Also in values starting with the prefix 'sp_etc_', I noticed a common mistake that the 't' is left out, giving 'sp_ec_' so I compensate for this as well. Bellow is an example:

import sys

#remove all spaces and convert to uppercase, for comparision purposes
def standardizer(str):
    str = str.replace("sp_ec_", "sp_etc_")#this is  a common typo in the db, the 't' is left out
    str = str.replace(" ", "")
    str = str.upper()
    return str

#this function is for testing purposes and would actually read in values from db
def getUserInput():
    return ' this  is a test'

def main():
    str1 = "I'mok"
    str2 = 'this is a test'
    str3 = 'two spaces'
    str4 = ' spaces at front and end '
    str5 = 'sp_ec_blah'
    print(str1, standardizer(str1))
    print(str2, standardizer(str2))
    print(str3, standardizer(str3))
    print(str4, standardizer(str4))
    print(str5, standardizer(str5))
    #this is an example of how the function would actually be used
    print(standardizer(str2), standardizer(getUserInput()))
    if standardizer(str2) == standardizer(getUserInput()):
        print('matched')
    else:
        print('not a match')

if __name__ == '__main__':
    main()

Any suggestions on the standardizer() function? First off I think it needs a better name. I'm wondering if I should break it into two functions, one being for the missing 't' and the other being for removing spaces and making upper case (by the way, from what I've seen it's more common to convert everything to upper case than lower case, for comparison purposes). Also, how would you comment something like this?

You can simplify avoiding reassignement:

def standardizer(str):
    return str.replace("sp_ec_", "sp_etc_").replace(" ", "").upper()

You should use a for loop to avoid repetition in the following lines:

print(str1, standardizer(str1))
print(str2, standardizer(str2))
print(str3, standardizer(str3))
print(str4, standardizer(str4))
print(str5, standardizer(str5))


Factory using Structuremap to inject concrete implementation of BaseTask The problem statement that I am trying to solve is this;

We will retrieve a list of Tasks from an external repository. These tasks will have a type and for each type, there needs to be a specific implementation that defines the Model it needs it order to perform the Task and then the logic needed to Process the task, along with any dependencies.

At one point, the Factory was setup to use Generics, but reverted to this as it seemed simpler.

 public interface ITask
 {
     bool ProcessTask();
 }

 public interface ITask <TModel>
 {

 }

 public interface IModelBuilder <out TModel>
 {
     TModel Build(ITaskDefinition taskDefinition);
 }

 public abstract class BaseTask <TModel> : ITask<TModel> , ITask where TModel: IModel
 {
     public TModel Model {  get; set;}

     public ITaskDefinition TaskDefinition { get; set; }

     protected BaseTask(ITaskDefinition definition, IModelBuilder <TModel> builder)
     {
         Model = builder.Build(definition);
     }

     public abstract bool ProcessTask();
 }

 public interface ITaskFactory
 {
     ITask GetTask(ITaskDefinition taskDefinition, TaskType type);
 }

Concrete classes

public class TaskFactory: ITaskFactory
{
    private readonly IContainer _container;

    public TaskFactory(IContainer container)
    {
        _container = container;
    }

    public ITask GetTask(ITaskDefinition taskDefinition, TaskType type)
    {
        return _container
            .With(taskDefinition)
            .GetInstance < ITask > (type.ToString());
    }
}

public class DateTask: BaseTask < DateModel >
{
    private readonly ITaskManagementFacade _taskManagementFacade;

    public CaptureCompletionDateTask(ITaskDefinition taskDefinition, IModelBuilder < DateModel > builder, ITaskManagementFacade taskManagementFacade): base(taskDefinition, builder)
    {
        _taskManagementFacade = taskManagementFacade;
    }
    public override bool ProcessTask()
    {
        return _taskManagementFacade.CompleteTask(Model.SomeDate < DateTime.Now);
    }

}

public class DateModelBuilder: IModelBuilder < DateModel >
{
    private readonly ITaskManagementFacade _taskManagementFacade;

    public DateModelBuilder(ITaskManagementFacade taskManagementFacade)
    {
        _taskManagementFacade = taskManagementFacade;
    }

    public DateModel Build(ITaskDefinition taskDefinition)
    {
        var model = new DateModel
        {
            MyDate = DateTime.Now,
                Notes = _taskManagementFacade.GetTask((Guid.NewGuid()))
        };

        return model;
    }
}

public class DateModel: IModel
{
    public DateTime MyDate
    {
        get;
        set;
    }

    [Required]
    public string Notes
    {
        get;
        set;
    }
}

StructureMap container config

config.For<ITask>().Use<CaptureCompletionDateTask>().Named(TaskType.Task1.ToString());
config.For<IModelBuilder<DateModel>>().Use<DateModelBuilder>();

Code to get Task

var task = _taskFactory.GetTask(taskDefinition, TaskType.Task1);

What am I looking for? General comments, this is my first attempt at this kind of object construction. It would be great if I could get around the Named config in Structuremap and go back to using a Generic Factory to Load a Task based on TaskType.

Thanks

I believe it is not good to pass builder to constructor as an argument. I recommend to read Constructor Design guidelines from Microsoft. If you have any non-simple calculation in constructor it will be better to move it to a static method. In your case I would firstly create model and then pass the built one to the constructor. Unfortunately I don't see where and how a builder is created so maybe your solution keeps a good balance between complexity, flexibility and beauty.

It seems that here

public TModel Model {  get; set;}

you don't need setter since Model is set via constructor and is not supposed to be changed.

Small note: don't use extra spaces in generics :) IModelBuilder < DateModel > will look better as IModelBuilder<DateModel>.


Refreshing Google map until new data has been added to the database

  <title>GPS Tracker</title>
  <link rel="stylesheet" type="text/css" href="bootstrap/css/bootstrap.min.css">
  <link rel="stylesheet" type="text/css" href="view/style.css">

Tracking

       <?php
         include"controller/connection.php";
         $query=mysql_query("SELECT id_history FROM tb_history");
         $count=mysql_num_rows($query);
       ?>
       <form method="POST" action="<?=$_SERVER['PHP_SELF']?>">
         <input type="hidden" name="count" value="<?php echo $count ?>">
         <input type="submit" name="submit" class="btn btn-primary" value="Get Location" style="width:280px"> </button>

       </form>

 kanvas peta

<?php
 if(isset($_POST['submit']))
   {
     include "controller/connection.php";
     $count1 = $_POST['count'];
     $count2 = $count1+1;
     $sql = mysql_query("SELECT id_history FROM tb_history");
     $count3 = mysql_num_rows($sql);
     if($count3==$count1)
     {
       echo"";
     }
     else if
     {
       echo"";
     }
   }
 ?>

The code that I wrote above will show a button and a Google map.

Once the button has been clicked, the code will send the amount of data that exist in the tb_history.

The PHP code at the bottom will always check if there's any new data added or not. If not, then it will refresh the Google map until new data has been added to the table tb_history.

Can anyone suggest a better algorithm for this?

In my opinion the best way to achieve what you want is to work with Event Listeners.

So basically, breaking it down to software design, you would fire a event if something is written to the database, so everytime you insert into the database. Maybe also when you update or delete something if you want to trigger these too.

Then you need an Event Listener which will run php code which runs and does what you want when new Data is added only when new Data is added.

Then you could send frequently requests to the PHP file, which gives you the message back if data is updated or not, but that would be counterproductive behavior because your already working event driven.

So you fire a Response to the JavaScript from the PHP file when the Event got fired. So your whole code will only run if it must, which depends only on if there's new data or not. Without continious requests whatsoever.

This would be the best attempt, you'll need some things for that, like the before mentioned Event Listeners, jQuery Ajax Calls or Plain JavaScript Ajax Calls.

Also some thougts to your SQL code

$count1 = $_POST['count']; //NEVER use data from Requests directly!

You're vulnerable to SQL Injection! Always escape the input and use prepared statements

$count2 = $count1+1; //Attempting to the above code there's no need for this line

And at least you can mix these 2 lines togther to one line:

$sql = mysql_query("SELECT id_history FROM tb_history");
$count3 = mysql_num_rows($sql);

Would be

$count3 = mysql_query("SELECT count(id_history) FROM tb_history");


Simple Area of Shape Calculator So I made this calculator the other day as a small project. I just wanted to know if I remembered the fundamentals. It works fine from the looks of it, but I know there are always ways to improve.

    package Main;
    import java.util.*;
    public class calc {
@SuppressWarnings("resource")
private static void circle() {
    // TODO Auto-generated method stub
    double a_c = 0; //a_c = area of circle

    System.out.println("Please enter the radius of the circle.");

    Scanner read_r = new Scanner(System.in); // This is the scanner that scan user input.

    double r = read_r.nextDouble(); // This is the radius the user enters.

    a_c = Math.PI * r *r ; //a_c stands for area of circle followed by equation.

    System.out.print("The area of the circle equals: ");

    System.out.println(a_c);// the system is printing the result; the value of a_c

    main(null);
}
@SuppressWarnings("resource")
private static void square() {
    // TODO Auto-generated method stub

    double a_s = 0; // a_s = area of square initial value

    System.out.println("Please enter the value of the length of one side of the square.");

    Scanner read_s = new Scanner(System.in); // This is the scanner that scan user input.

    double s = read_s.nextDouble(); // This is the side length the user enters.

    a_s = s*s; //equation for square area = s^2

    System.out.print("The area of the square equals: ");

    System.out.println(a_s);

    main(null);
}
@SuppressWarnings("resource")
private static void triangle() {
    // TODO Auto-generated method stub
    double a_t = 0; //a_t = area of triangle initial value

    System.out.println("Please enter the height of your triangle.");

    Scanner read_h = new Scanner(System.in); // This is the scanner that will read height.

    int h = read_h.nextInt(); // This is the height the user enters.

    System.out.println("Please enter the value for the base of your triangle");

    Scanner read_b = new Scanner(System.in); // This is the scanner that reads base

    Double b = read_b.nextDouble(); // This is the base the user enters

    a_t = .5*b*h;

    System.out.print("The area of the traingle equals: ");

    System.out.println(a_t);

    main(null);
}
@SuppressWarnings("resource")
private static void rectangle() {
    // TODO Auto-generated method stub
    double a_r = 0;// a_r = area of rectangle initial value

    System.out.println("Please enter the width of your rectangle.");

    Scanner read_w = new Scanner(System.in); // This is the scanner that scan user input.

    double w = read_w.nextInt(); // This is the width value the user enters.

    System.out.println("Please enter a value for the height of your triangle.");

    Scanner read_h = new Scanner(System.in);

    double h = read_h.nextDouble();

    a_r = h*w;

    System.out.print("The area of the rectangle equals: ");

    System.out.println(a_r);

    main(null);
}
private static void quit(){
    System.out.println("Quiting....");

    System.exit(0);

}
@SuppressWarnings("resource")
public static void main(String [] args1){

    System.out.println("What would you like to do? Type the number of your desired function.");

    System.out.println("1. Area of a Circle");

    System.out.println("2. Area of a Square");

    System.out.println("3. Area of a Triangle");

    System.out.println("4. Area of a Rectangle");

    System.out.println("5. Quit");

    Scanner readc = new Scanner(System.in);

    int c = readc.nextInt(); //This is the choice method 1-4

switch (c){
case 1:
    circle();
    break;
case 2:
    square();
    break;
case 3:
    triangle();
case 4:
    rectangle();
case 5:
    quit();
    break;
default:
    System.out.println("I'm sorry, you didn't select a function.");
    System.out.println("Please choose again");
    main(null);
    }
 }

}

First of all, well done on building a working area calculator. Lets see how we can improve it!

To start off, you have quite a few indentation problems. The definition of the class should not be indented at all, and the methods inside the class should be indented once. For each nesting level, you indent another time.

Also, your class is named calc, but Java classes are supposed to be PascalCase, meaning it should be Calc, with an uppercase c.

Functions

You are using too much vertical space. Not every line needs to have an empty line all around it. Try to group blocks of related lines of code together.

Most, if not all of your functions, do more than one thing. In programming we strive for functions that perform a single task and do so very well. For example, your main function is also in charge of how the user interface is presented to the user. This should be offloaded to another function like so:

private static void showUserOptions() {
    System.out.println("What would you like to do? Type the number of your desired function.");

    System.out.println("1. Area of a Circle");
    System.out.println("2. Area of a Square");
    System.out.println("3. Area of a Triangle");
    System.out.println("4. Area of a Rectangle");
    System.out.println("5. Quit");
}

Now we can simply call this function from the main method. As you can see, I have also grouped the System.out.println() statements that specify the options very close to each other. This helps with readability a lot.

Your functions also contain a lot of code duplication, such as printing the area of the selected shape and creating a new Scanner instance in every function, which is totally unnecessary and can be replaced with a single static instance like so:

private static Scanner inputScanner = new Scanner(System.in);

Furthermore, your variable names are not very clear. a_r is a useless variable name. How about rectangleArea?

User Input

For the user input of your program, we can user Java enums:

private enum UserInput { CIRCLE, SQUARE, TRIANGLE, RECTANGLE, QUIT, INVALID }

Here I have created 4 enum values for the different shapes, a QUIT enum value for the quit command and an INVALID value for any user input that is invalid.

Now we do have to change the showUserOptions() function a bit to make it correspond with the new enum values. This is done as follows:

private static void showUserOptions() {
    System.out.println("What would you like to do? Type the number of your desired function.");

    System.out.println(UserInput.CIRCLE.ordinal() + ". Area of a Circle");
    System.out.println(UserInput.SQUARE.ordinal() + ". Area of a Square");
    System.out.println(UserInput.TRIANGLE.ordinal() + ". Area of a Triangle");
    System.out.println(UserInput.RECTANGLE.ordinal() + ". Area of a Rectangle");
    System.out.println(UserInput.QUIT.ordinal() + ". Quit");
}

Calling ordinal() on an enum will give you the index it has in the list of enum values. Meaning that UserInput.SQUARE.ordinal() will result in 0 and UserInput.QUIT.ordinal() will result in 4.

To retrieve an actual enum object from the integer input provided by the user, the following code can be used:

int input = inputScanner.nextInt();
UserInput userInput = UserInput.values()[input];

UserInput.values() creates an array of the enum values, which looks like this: [CIRCLE, SQUARE, TRIANGLE, RECTANGLE, QUIT, INVALID]. Indexing it with the integer input from the user gives us the actual enum value. The UserInput.values() call actually creates a new array every time it is called, so we will be caching it as a static variable right underneath the declaration of the enum itself:

private enum UserInput { CIRCLE, SQUARE, TRIANGLE, RECTANGLE, QUIT, INVALID }
private static UserInput[] userInputs = UserInput.values();

Since the user can enter any integer value, even ones that are outside the range of the userInputs() array, we create a special function to deal with this and ensure that we always get a UserInput enum:

private static UserInput getUserInputFromInt(int userInput) {
    if (userInput >= 0 && userInput < userInputs.length) {
        return userInputs[userInput];
    }

    return UserInput.INVALID;
}

Since your code contained a lot of duplication of retrieving user input, I have created this function:

private static double askUserForValue(String valueName, UserInput userInput) {
    System.out.println(String.format("Please enter the %s of your %s.", valueName, userInput.toString().toLowerCase()));

    return inputScanner.nextDouble();
}

Which we can call like this:

double radius = askUserForValue("radius", userInput);

To ask the user to provide us the radius of a circle.

Main method

The main method of your program should be concise and clear. Also it should definitely not be called manually from other functions. These other functions have no business calling the main method, as they should only be looking to fulfill their single task. I would therefore suggest a rewrite of your main() function:

public static void main(String[] args) {
    while (true) {
        showUserOptions();

        UserInput userInput = getUserInputFromInt(inputScanner.nextInt());
        actOnUserInput(userInput);
    }
}

A few things stand out here. I have renamed your scanner to inputScanner, as readc isn't that good of a name for a Scanner object. Also I am calling a method that we have not yet defined, called actOnUserInput(). I will show this function in detail below. Finally, I have wrapped these two function calls in a while loop, to ensure that they will repeatedly be called. This makes it so that we can remove the calls to main() from all other places in the code.

Object Oriented Programming

Why not practice some object oriented programming while we are at it?

What we are dealing with here is a bunch of shapes. This allows us to create an abstraction of a Shape object. This is achieved by defining a common interface that all shapes share:

interface Shape {
    public double getArea();
}

Now that we have the Shape interface, we can create some concrete implementations of it. Lets start out with the circle object:

private static class Circle implements Shape {
    double radius;

    public Circle(double radius) {
        this.radius = radius;
    }

    @Override
    public double getArea() {
        return Math.PI * this.radius * this.radius;
    }
}

Now we can do the same for the triangle and rectangle shapes:

private static class Triangle implements Shape {
    double height;
    double base;

    public Triangle(double height, double base) {
        this.height = height;
        this.base = base;
    }

    @Override
    public double getArea() {
        return 0.5 * this.height * this.base;
    }
}

private static class Rectangle implements Shape {
    double length;
    double width;

    public Rectangle(double length, double width) {
        this.length = length;
        this.width = width;
    }

    @Override
    public double getArea() {
        return this.length * this.width;
    }
}

All that is missing now is the Square class. However, in mathematics, a square is a subclass of rectangle. Implementing this using inheritance would look like this:

private static class Square extends Rectangle {
    public Square(double sideLength) {
        super(sideLength, sideLength);
    }
}

Here, the square class only calls the parent constructor using the same length parameter for the width and height. The getArea() functionality is inherited from the Rectangle class and doesn't need to be implemented again.

Whether or not it is a good idea to have Square inherit from Rectangle is debatable and often discouraged, since inheritance can become quite messy. However, for the sake of this example I believe it is justified.

Now that we have defined a bunch of concrete Shape implementations, we would like to create them!

We can use the UserInput enum to determine which type of shape we want to create. This can be put into a single factory-like function that will simply return a Shape object:

private static Shape createShapeFromUserInput(UserInput userInput) {
    switch (userInput) {
        case CIRCLE:
            double radius = askUserForValue("radius", userInput);

            return new Circle(radius);
        case SQUARE:
            double sideLength = askUserForValue("side length", userInput);

            return new Square(sideLength);
        case TRIANGLE:
            double height = askUserForValue("height", userInput);
            double base = askUserForValue("base", userInput);

            return new Triangle(height, base);
        case RECTANGLE:
            double length = askUserForValue("length", userInput);
            double width = askUserForValue("width", userInput);

            return new Rectangle(length, width);
    }

    return null;
}

Now we can incorporate this function into the yet to be defined actOnUserInput() function:

private static void actOnUserInput(UserInput userInput) {
    switch (userInput) {
        case CIRCLE:
        case SQUARE:
        case TRIANGLE:
        case RECTANGLE:
            Shape shape = createShapeFromUserInput(userInput);

            if (shape != null) {
                System.out.println("The area is equal to: " + String.valueOf(shape.getArea()));
            }
            break;
        case QUIT:
            quit();
            break;
        case INVALID:
            System.out.println("I'm sorry, you didn't select a function.");
            System.out.println("Please choose again");
            break;
    }
}

Here we can see the purpose of the Shape interface come through. After calling createShapeFromUserInput(), we don't care whether a Rectangle, Circle, or whatever type of shape is returned, only that it is a Shape instance. This is because now we can simply call the getArea() method defined on the Shape interface and rely on that.

The entire program now looks like this:

import java.util.Scanner;

public class Calc {
    private enum UserInput { CIRCLE, SQUARE, TRIANGLE, RECTANGLE, QUIT, INVALID }
    private static UserInput[] userInputs = UserInput.values();

    private static Scanner inputScanner = new Scanner(System.in);

    public static void main(String[] args) {
        while (true) {
            showUserOptions();

            UserInput userInput = getUserInputFromInt(inputScanner.nextInt());
            actOnUserInput(userInput);
        }
    }

    private static void showUserOptions() {
        System.out.println("What would you like to do? Type the number of your desired function.");

        System.out.println(UserInput.CIRCLE.ordinal() + ". Area of a Circle");
        System.out.println(UserInput.SQUARE.ordinal() + ". Area of a Square");
        System.out.println(UserInput.TRIANGLE.ordinal() + ". Area of a Triangle");
        System.out.println(UserInput.RECTANGLE.ordinal() + ". Area of a Rectangle");
        System.out.println(UserInput.QUIT.ordinal() + ". Quit");
    }

    private static UserInput getUserInputFromInt(int userInput) {
        if (userInput >= 0 && userInput < userInputs.length) {
            return userInputs[userInput];
        }

        return UserInput.INVALID;
    }

    private static void actOnUserInput(UserInput userInput) {
        switch (userInput) {
            case CIRCLE:
            case SQUARE:
            case TRIANGLE:
            case RECTANGLE:
                Shape shape = createShapeFromUserInput(userInput);

                if (shape != null) {
                    System.out.println("The area is equal to: " + String.valueOf(shape.getArea()));
                }
                break;
            case QUIT:
                quit();
                break;
            case INVALID:
                System.out.println("I'm sorry, you didn't select a function.");
                System.out.println("Please choose again");
                break;
        }
    }

    private static Shape createShapeFromUserInput(UserInput userInput) {
        switch (userInput) {
            case CIRCLE:
                double radius = askUserForValue("radius", userInput);

                return new Circle(radius);
            case SQUARE:
                double sideLength = askUserForValue("side length", userInput);

                return new Square(sideLength);
            case TRIANGLE:
                double height = askUserForValue("height", userInput);
                double base = askUserForValue("base", userInput);

                return new Triangle(height, base);
            case RECTANGLE:
                double length = askUserForValue("length", userInput);
                double width = askUserForValue("width", userInput);

                return new Rectangle(length, width);
        }

        return null;
    }

    private static double askUserForValue(String valueName, UserInput userInput) {
        System.out.println(String.format("Please enter the %s of your %s.", valueName, userInput.toString().toLowerCase()));

        return inputScanner.nextDouble();
    }

    private static void quit() {
        System.out.println("Quiting....");

        System.exit(0);

    }

    private interface Shape {
        double getArea();
    }

    private static class Circle implements Shape {
        double radius;

        public Circle(double radius) {
            this.radius = radius;
        }

        @Override
        public double getArea() {
            return Math.PI * this.radius * this.radius;
        }
    }

    private static class Triangle implements Shape {
        double height;
        double base;

        public Triangle(double height, double base) {
            this.height = height;
            this.base = base;
        }

        @Override
        public double getArea() {
            return 0.5 * this.height * this.base;
        }
    }

    private static class Rectangle implements Shape {
        double length;
        double width;

        public Rectangle(double length, double width) {
            this.length = length;
            this.width = width;
        }

        @Override
        public double getArea() {
            return this.length * this.width;
        }
    }

    private static class Square extends Rectangle {
        public Square(double sideLength) {
            super(sideLength, sideLength);
        }
    }
}


Numbers to Strings in Python part 2 After implementing most of the suggestions from the first question, This is the code I got:

from math import log

zero_to_nineteen = ("zero", "one", "two", "three", "four",
                    "five", "six", "seven", "eight", "nine",
                    "ten", "eleven", "twelve", "thirteen", "fourteen",
                    "fifteen", "sixteen", "seventeen", "eighteen", "nineteen")

tens = ("zero", "ten", "twenty", "thirty", "forty",
        "fifty", "sixty", "seventy", "eighty", "ninety")

# Powers of a thousand (US system of naming)
suffixes = ("zero", "thousand", "million", "billion", "trillion",
            "quadrillion", "quintillion", "sextillion", "septillion",
            "octillion", "nonillion", "decillion", "undecillion",
            "duodecillion", "tredecillion", "quattuordecillion",
            "quinquadecillion", "sedecillion", "septendecillion",
            "octodecillion", "novendecillion", "vigintillion",
            "unvigintillion", "duovigintillion", "tresvigintillion",
            "quattuorvigintillion", "quinquavigintillion", "sesvigintillion",
            "septemvigintillion", "octovigintillion", "novemvigintillion",
            "trigintillion", "untrigintillion", "duotrigintillion",
            "trestrigintilion", "quattuortrigintillion",
            "quinquatrigintillion", "sestrigintillion", "septentrigintillion",
            "octotrigintillion", "noventrigintillion", "quadragintillion")

def spell_out(number):
    """Returns a string representation of the number, in the US system"""
    try:
        number = int(number)
    except OverflowError:
        # This will be triggered with large float values such as 1e584
        return "infinity"

    if number < 0:
        return "negative " + spell_out(-1 * number)

    if number < 20:
        return zero_to_nineteen[number]

    if number < 100:
        tens_digit, ones_digit = divmod(number, 10)
        return _createNumber(tens[tens_digit], ones_digit, "-")

    if number < 1000:
        hundreds_digit, rest = divmod(number, 100)
        return _createNumber(spell_out(hundreds_digit) + " hundred", rest)

    suffix_index = int(log(number, 1000))

    if suffix_index < len(suffixes):
        suffix_value = 1000 ** suffix_index
        prefix, rest = divmod(number, suffix_value)
        prefix = spell_out(prefix) + " " + suffixes[suffix_index]
        return _createNumber(prefix, rest, ", ")

    return "infinity"

def _createNumber(prefix, suffix, seperator=" "):
    # Returns a number with given prefix and suffix (if needed)
    if not isinstance(prefix, str):
        prefix = spell_out(prefix)
    return prefix if suffix == 0 else prefix + seperator + spell_out(suffix)

I'd like to hear any comments about readability/performance/pythonicity and anything else you can think about that could use improving.

It looks pretty nice! I can only nitpick.

Instead of:

return "negative " + spell_out(-1 * number)

You could simply:

return "negative " + spell_out(-number)

Some PEP8 recommendations:

Use 2 blank lines before each function definition
Use snake case in function names, _create_number instead of _createNumber


Bank account supporting deposit, withdrawal, and interest This is my completed code I was wondering whether it needs a correction

This is my first class

public class Account
{
    private double balance; //STATE
    private double interestRate; //STATE
    private double rate;//STATE

    public Account()
    {
        balance = 0;
        interestRate = 0;
    }

    public Account(double amount, double interestRate)
    {
        balance = amount;
        rate = interestRate;

    }

    public void deposit(double amount)
    {
        balance=balance+amount;
    }

    public void withdraw(double amount)
    {
        balance = balance - amount;
    }

    public void setInterest(double rate)
    {
        balance = balance + balance * rate;
        //this.setInterst = setInterest;
        //setInterest = InterestRate / 12;
    }

    public double computeInterest(int n)
    {
        balance=Math.pow(balance*(1+rate),n/12);
        return balance;
    }

    public double getsetInterest()
    {
        return rate;
    }

    public double getBalance()
    {
        return balance;
    }

    public void close()
    {
        balance =0;
    }

}

This is my second class

public class TestAccountInterest
{
    public static void main (String[] args)
    {
        Account acc1 = new Account(500, 0.1);//0.10);
        Account acc2 = new Account(400, 0.2); //0.20);

      /*************************************
       ACC1 ACCOUNT BELOW
       *************************************/
        acc1.deposit(500);
        acc1.withdraw(300);
        acc1.computeInterest(12);
        System.out.println(acc1.computeInterest(12));

        /**************************************
        ACC2 ACCOUNT BELOW
         **************************************/
        acc2.withdraw(200);
        acc2.deposit(800);
        acc2.computeInterest(24);
        System.out.println(acc2.computeInterest(24));

    }

}

Can anyone see whether I would be able to make the code more compact and is the way that I have coded perfectly valid. The code is about an Accounts class with a test account class for the second class it is supposed to calculate the compute interest of 12 months for the first one and for the second one is supposed to be about 24 months.

Well, this is really vague.

Regarding Account :

Your Account class looks like a anemic class except the computeInterest method. For this one, you should rename the n parameter to reveal his role.

I does not understand your getsetInterest method.

One thing that seems wrong is your close method. When an Account is closed it cannot be used anymore. You may maintains another state to know if the account is closed. Or create another ClosedAccount.

For TestAccountInterest :

You should have a look at a test framework that will run your tests and report failures.

Your class is called TestAccountInterest but you are doing more than testing the interest, you are also testing the deposit and withdraw methods.


Insert k copies of each element in ArrayList This method takes an ArrayList<String> s and int k , then creates k copies of each element.

public static void copyStrings(ArrayList<String> s, int k) {
        if (k <= 0) {
            s.clear();
        } else {
            int limit = s.size();
            for(int i = 0; i < limit; i++) {
                for( int j = 0; j < k; j++) {
                    s.add(s.get(i));
                }
            }
            for( int i = 0; i < limit; i++) {
                s.remove(0);
            }
    }
}

You have double the indenting that you should inside your method.

Rather than calling s.get(i) every time, store it in a variable and use from that instead. Incidentally, this means that you can use an advanced for instead of a simple one.

I'd recommend renaming limit to originalSize, so that it's clear what it is.

s isn't descriptive. Why not call it source? Ditto with k -- why not call it copies?

Why are you making it specific to Strings, when it's almost just a find-and-replace away from being a generic method? If you make it generic, it'll work with any type, not just the one.

Aside from that, there's not much. Your code is pretty short, and there isn't much to say. Here's your code with these tips applied:

public static <E> void copyStrings(ArrayList<E> source, int copies) {
    if (copies <= 0) {
        source.clear();
    } else {
        int limit = source.size();
        for(E element : source) {
            for( int j = 0; j < copies; j++) {
                source.add(element);
            }
        }
        for( int i = 0; i < limit; i++) {
            source.remove(0);
        }
    }
}


My implementation of lexicographic permutation in F# is 3x slower than C# Can someone help me with the following micro optimization for the F# code for lexicographic permutation?

I have code in C# which runs for 0.8s. As a learning practice, I translated it into F#. However, it becomes 2.9s. Just out of curiosity, I am wondering why my code in F# runs that slow? Are there any improvement can be made to my F# code without changing the algorithm?

C#

 static bool ToNextLexicographic(int[] myArray)
    {
        int pivot = -1;

        for (int i = myArray.Length - 1; i > 0; i--)
        {
            if (myArray[i] > myArray[i - 1])
            {
                pivot = i - 1;
                break;
            }
        }

        if (pivot == -1)
            return false;

        for (int j = myArray.Length - 1; j > pivot; j--)
        {
            if (myArray[j] > myArray[pivot])
            {
                // swap
                var tmp = myArray[j];
                myArray[j] = myArray[pivot];
                myArray[pivot] = tmp;

                // reverse
                for (int i = pivot + 1, k = myArray.Length - 1; i < k;i++,k-- )
                {
                    var tmp = myArray[i];
                    myArray[i] = myArray[k];
                    myArray[k] = tmp;
                }
                break;
            }
        }
        return true;
    }
    static IEnumerable<int[]> GetPermutationsLexicographic(int[] myArray)
    {
        Array.Sort(myArray);
        yield return myArray;

        while (ToNextLexicographic(myArray))
        {
            //yield return myArray.ToArray();
            yield return myArray;
        }
    }

F# (much readable than C#)

let inline toNextLexicographic (myArray: _[]) =
    let rec findPivot i =
        if i = 0 then -1
        else
            if myArray.[i] > myArray.[i-1] then i - 1
            else findPivot (i - 1)

    let rec findTarget value i =
        if (myArray.[i] > value) then i
        else findTarget value (i - 1)

    let inline swap i j =
        let tmp = myArray.[i]
        myArray.[i] <- myArray.[j]
        myArray.[j] <- tmp

    let inline reverse i =
        let mutable a = i
        let mutable b = myArray.Length - 1
        while a < b do
            swap a b
            a <- a + 1
            b <- b - 1

    let pivot = findPivot (myArray.Length - 1)
    if pivot = -1 then false
    else
        let target = findTarget myArray.[pivot] (myArray.Length - 1)
        swap pivot target
        reverse (pivot + 1)
        true;

let inline getPermutationsLexicographic myArray = seq {
    Array.sortInPlace myArray
    yield myArray
    while toNextLexicographic myArray do
        yield myArray
    }

// benchmark
let mutable d = 0
for x in getPermutationsLexicographic([|1..11|]) do
    d <- d + 1
d

You're not comparing apples to apples. The F# code is generic whereas the C# only works with int arrays. Remove every occurrence of inline and add a type annotation to toNextLexicographic:

let toNextLexicographic (myArray: int[])

In my tests, that takes the time from 2.152 to 0.146 seconds.

The reason for the poor performance is inline isn't working. You can see this by decompiling. Most of the time is spent in LanguagePrimitives.HashCompare.GenericGreaterThanIntrinsic as a result. This is much more expensive than op_GreaterThan.

I'm not sure under what conditions inline is ignored, but I can only guess, in this case, it's due to the complexity of your code and the closures.


MYSQL select count two table left join I have two table. I want count how many times a value is in other table. So the codice from table "sorgente" is different in table contatore, because I have the suffix 'BRANO-' before the code.
sorgente
| codice | nome  |
|   15   | mario |
|   16   | mary  |

contatore
| nome_evento |   data     |
| BRANO-15    | 2020-08-15 |
| BRANO-15    | 2020-08-16 |
| BRANO-16    | 2020-08-14 |

the query for select is
select
    s.codice,
    (
        select count(*)
        from contatore c
        where c.nome_evento = concat('BRANO-', s.codice)
    ) no_matches_in_contatore
from sorgente s

but the original "sorgente" table has 25000 records, the "contatore" table has at now 60000 records.
when I try to execute the query it takes 600 seconds....
can you help me?

You should try below query, also let me know what indexes are on your tables.
  WITH cte1 as (select SUBSTR(nome_evento, instr(nome_evento, '-') + 1) as nome_evento  from contatore)
SELECT count(b.codice),
        b.codice
FROM cte1 a
INNER JOIN
sorgente b
ON (a.nome_evento = b.codice)
group by b.codice;


Find the area of the bounding box of a robot's moves on a grid Given a string of directions (example: "LLRUDDULRDRU") find the area of the rectangle defined by the lowest-left point (minx, miny) and the highest-right point (maxx, maxy) the robot was in.

The canonical solution I think would be.

int area(char* directions) {
    char * c = directions;
    int x=0, y=0, minx=0, maxx=0, miny=0, maxy=0;
    while (*c) {
        if (*c == 'U') {
            y++;
        } else if (*c == 'D') {
            y--;
        } else if (*c == 'R') {
            x++;
        } else if (*c == 'L') {
            x--;
        }

        if (y < miny)
            miny = y;
        if (x < minx)
            minx = x;
        if (y > maxy)
            maxy = y;
        if (x > maxx)
            maxx = x;

        c++;
    }
    return (maxx - minx) * (maxy - miny);
}

The problem I see with this is that the indentation is switched often making it hard to read. That's why I tend to exploit the fact C's logical operators return numbers 1 or 0.

int area(char * directions) {
    char * c = direction;
    int x=0, y=0, minx=0, maxx=0, miny=0, maxy=0;
    while (*c) {
        y += (*c == 'U') - (*c == 'D');
        x += (*c == 'R') - (*c == 'L');

        minx = (x < minx) ? x : minx;
        miny = (y < miny) ? y : miny;
        maxx = (x > maxx) ? x : maxx;
        maxy = (y > maxy) ? y : maxy;

        c++;
    }

     return (maxx - minx) * (maxy - miny);
}

However doing this feels 'too hacky'.

Is doing this a good practice at all?

Function signature

It appears you're only reading from directions, so pass that as a pointer to const.

while loop

You initialize c before the while loop and increment it at the end of each iteration.  Writing it as for will convey the intent more clearly.

Chained conditions

Your first if/else if chain is always testing the same variable, so a switch statement seems more appropriate:

    switch (*c) {
    case 'U': ++y; break;
    case 'D': --y; break;
    case 'R': ++x; break;
    case 'L': --x; break;
    }

You might want to add a default case to indicate when malformed input is received.  I can't tell from the context whether it would be appropriate to print something to stderr or to return an error code (perhaps as a negative return value), so I'll leave that to you.

Don't test all bounds each time

We can now observe that we only need to check y against maxy if we've incremented y, so we can move that test into the switch; similarly with the other cases:

int area(char const *directions)
{
    int x=0, y=0,
        minx=0, maxx=0,
        miny=0, maxy=0;
    for (char const *c = directions;  *c;  ++c) {
        switch (*c) {
        case 'U':
            ++y;
            if (y > maxy) maxy = y;
            break;
        case 'D':
            --y;
            if (y < miny) miny = y;
            break;
        case 'R':
            ++x;
            if (x > maxx) maxx = x;
            break;
        case 'L':
            --x;
            if (x < minx) minx = x;
            break;
        }
    }
    return (maxx - minx) * (maxy - miny);
}

You might want to consider using a max() function to re-write ++x; if (x > maxx) maxx = x; as maxx = max(maxx, ++x);, but I can't really say that one is better than the other - too close to call.  Similarly, you might decide that returning an unsigned type is more appropriate, but that really depends on the code you're calling it from.


Shorten runtime of this Python program This is an answer to this question on Code-Golf.SE (but obviously not golfed). How can I shorten the runtime of my program? It is taking far too long to run some of the test cases. Can I utilise Python map() or lambda?

def main():

    # Store number of test cases
    T = int(input())

    # Loop for each test case:
    for testCase in range(0, T):

        # Store number of members each team can have
        N = int(input())

        # Store power of beyblades of Team G-Revolution members
        teamG_beybladePowers = [int(x) for x in input().split()]

        # Store power of beyblades of opponent team members
        opponentBeybladePowers = [int(x) for x in input().split()]

        # Store the calculated order of our team members as a list
        teamG_calculatedOrder = []
        # Store the caclulated order of opponent team members as a list
        opponentCalculatedOrder = []

        # Loop through each of the team members and opponent team members, and match them
        for index in range(0, N):

            # Check if Team G's highest beyblade power is lower than or equal to opponent's highest
            if max(teamG_beybladePowers) <= max(opponentBeybladePowers):

                # Team G's highest beyblade power is weaker than opponent's highest.
                # We need not waste our best player on their best player, when we will lose.
                # Match our lowest player against their highest player.
                teamG_calculatedOrder.append(min(teamG_beybladePowers))
                opponentCalculatedOrder.append(max(opponentBeybladePowers))

                # Remove these members from original list
                teamG_beybladePowers.remove(min(teamG_beybladePowers))
                opponentBeybladePowers.remove(max(opponentBeybladePowers))

            else:
                # Team G's highest beyblade power is more than opponent's highest.
                # We can use our best player to beat the opponent's best player.
                # Match them up:
                teamG_calculatedOrder.append(max(teamG_beybladePowers))
                opponentCalculatedOrder.append(max(opponentBeybladePowers))

                # Remove these members from the original list
                teamG_beybladePowers.remove(max(teamG_beybladePowers))
                opponentBeybladePowers.remove(max(opponentBeybladePowers))

        # Store total number of wins
        totalWins = 0

        # Calculate number of wins
        for index in range(0, N):

            # Check if current match was won
            if teamG_calculatedOrder[index] > opponentCalculatedOrder[index]:
                # This match was won
                # Increment total wins:
                totalWins += 1

            else:
                # This match was lost
                pass

        # Print total number of wins
        print(totalWins)

main()

Specific suggestions

Using command-line arguments is the idiomatic way to collect input. This makes the code scriptable and reusable as a library.
The idiomatic way to run main is this:

if __name__ == "__main__":
    main()

Several bits of code are duplicated, such as min(teamG_beybladePowers). Pull those out into variables to avoid re-iterating through the whole list more than necessary.

Tool support suggestions

black can automatically format your code to be more idiomatic. It'll do things like adjusting the vertical and horizontal spacing, while keeping the functionality of the code unchanged. In your case the code does look decently formatted, so it may not change much (or at all).
flake8 can give you hints to write idiomatic Python. I would start with this configuration:

[flake8]
max-complexity = 4
ignore = W503,E203


Benchmark of Perfect Numbers I wrote a piece of code to benchmark the detection of perfect numbers up to 10000.  The original code is here, in C:

#include <stdio.h>

int isPerfectNumber(int num) {
    if (num == 1) { return 0; }

    int i, c = num - 1;
    for (i = 2; i <= num / 2; ++i) {
        if (num % i == 0) { c -= i; }
    }

    return c == 0;
}

int main(int argc, char* argv[]) {
    int i;
    for (i = 0; i < 10000; ++i) {
        if (isPerfectNumber(i)) {
            printf("%d\n", i);
        }
    }
    return 0;
}

I then wrote the same code in a number of different languages (C, C#, Java, V8 Javascript, Python and Go).

After benchmarking, I got the following results:

C: 92.8 ms
C#: 97.9 ms
Go: 208.85 ms
Java: 130.65 ms
V8 Javascript: 279.15 ms
Python: 1126.35 ms

I expected pretty much exactly this pattern of fastest to slowest, except for Go.  Why is my Go implementation so slow?  I admit I know very little about Go but I was under the impression that since it is a compiled language it is likely to be as fast as at least C# was.

Here is my Go implementation:

package main

import "fmt"

func main() {
    for i := 0; i < 10000; i++ {
        if (isPerfectNumber(i)) {
            fmt.Println(i)
        }
    }
}

func isPerfectNumber(num int) bool {
    if num == 1 { return false }

    c := num - 1
    for i := 2; i <= num / 2; i++ {
        if num % i == 0 { c -= i }
    }

    return c == 0
}

Have I just mucked something up or are the results I am seeing accurate?

FYI: I benchmark by running the code 10 times without timing it, then running another 20 times and take the average of those 20 tests.

I've bumped the number to 100000 and profiled. It seems that modulo operation is the most expensive here:

ROUTINE ======================== main.isPerfectNumber in /tmp/so/main.go
    31.72s     31.72s (flat, cum)   100% of Total
         .          .     19:   if num == 1 {
         .          .     20:       return false
         .          .     21:   }
         .          .     22:
         .          .     23:   c := num - 1
     2.35s      2.35s     24:   for i := 2; i <= num/2; i++ {
    29.37s     29.37s     25:       if num%i == 0 {
         .          .     26:           c -= i
         .          .     27:       }
         .          .     28:   }
         .          .     29:
         .          .     30:   return c == 0

It seems that C compilers are able to optimize modulo here, while the Go compiler cannot. I'd suggest opening an issue about it on the Go bug tracker. If C can do that, Go should as well.


Video downloader app implemented in RxJs I have used multiple map/flatmap combinations. Could anybody help me with the review?

var Rx = require('rx');
var RxNode = require('rx-node');
var request = require('request');
var _ = require('lodash');
var cheerio = require('cheerio');

exports.getResult = function(req, res) {
    var result = '';
    fetchContent({
            url: req.body.url
        }, req.body.season)
        .flatMap(getEpisodeRange)
        .map(formEpisodeUrl.bind(null, req.body.url, req.body.season))
        .flatMap(downloadHtml)
        .map(getGorillaUrl)
        .flatMap(postToGorilla)
        .map(getVideoUrl)
        .flatMap(prepareHtml)
        .subscribe(function(data) {
            result += data;
        }, function(error) {
            res.send(JSON.stringify(error))
        }, function() {
            res.send(result);
        });
}

function fetchContent(params) {
    console.log(params.url);
    var args = [].slice.call(arguments, 1)
    return Rx.Observable.create(function(observer) {
        request(params, function(error, response, body) {
            if (error) {
                observer.onError();
            } else {
                observer.onNext({
                    response: response,
                    body: body,
                    args: args
                });
            }
            observer.onCompleted();
        })
    });
};

function prepareHtml(url) {
    return `<a href=${url}></a>`;
}

function fileName(url, season, episode) {
    return Rx.Observable.just(url.split('/'))
        .last().map(function(name) {
            return name.replace('_', ' ').concat(` S${season}E${episode}`);
        });
}

function getVideoUrl(data) {
    var pattern = /"http(.*)(\.flv|\.mkv|\.mp4)"/;
    var matches = pattern.exec(data.body);
    if (matches && matches[0]) {
        return matches[0]
    } else {
        return null;
    }
}

function postToGorilla(url) {
    var params = {
        url: url,
        method: 'POST',
        headers: {
            'content-type': 'application/x-www-form-urlencoded',
        },
        form: {
            op: 'download1',
            method_free: 'Free+Download',
            id: _.last(url.split('/')),
        }
    }
    return fetchContent(params)
}

function getGorillaUrl(data) {
    try {
        var $ = cheerio.load(data.body);
        var url = $('a[title="gorillavid.in"]').first().attr('href').split('=')[1]
        return url = new Buffer(url, 'base64').toString('ascii');
    } catch (error) {
        return Rx.Observable.just(null);
    }
}

function downloadHtml(url) {
    return fetchContent({
        url
    });
}

function formEpisodeUrl(url, season, episode) {
    return url.replace('/serie/', '/episode/').concat(`_s${season}_e${episode}.html`);
}

function getEpisodeRange(data) {
    var $ = cheerio.load(data.body);
    return _.range(1, ($('span').filter(function() {
        return $(this).text() === `Season ${data.args[0]}`;
    }).parents('h2').siblings('ul').children('li').length) + 1 || 0, 1);
}

Overall, I think this is very decent code, some observations:

JSHint

Use JsHint ;)
You have a number of missing semicolons, and an unnecessary semicolon
return url = new Buffer(url, 'base64').toString('ascii'); <- I dont think you wanted to assign to buffer?
You are not using the function fileName(url, season, episode){ anywhere

Odd stuff

You use the extremely cool fetchContent({ url }); but also the mundane

{
  response: response,
  body: body,
  args: args
}

You should stick to 1 approach
Your regex /"http(.*)(\.flv|\.mkv|\.mp4)"/

Is not supporting avi, wmv, or mpg file extensions.
Is not supporting uppercase incoming file names extentions.
Keeps repeating the \. for each extension, so I would keep that out of the brackets.
If you plan for many entries, then I would keep the regex outside of the function, and generate that regex only 1 single time for performance

You have a stray console.log(params.url);, get rid of it
It seemed odd that the HTML you prepare has no text in the anchor:

return `<a href=${url}></a>`;

Also, do you want to consider at least surrounding the URL with double quotes?

Sugar

You use req.body 3 times in getResult, I would have had a var params = req.body; as a shortcut
This seems a good use case for a ternary:

if (matches && matches[0]) {
    return
} else {
    return null;
}

could be

return (matches && matches[0]) ? matches[0] : null;

I am still not sure why you return null actually. PrepareHtml will return , so perhaps consider

return (matches && matches[0]) ? matches[0] : '';

Anonymous functions kill kittens every day

The 3 functions within your subscribe call are anonymous. It is in general terrible to look at a stack trace filled with anonymous functions
You also have one in getEpisodeRange
I do like that you declare your other functions 'the right way'

Comments go a long way

getEpisodeRange needs comments, it is the most intense code
There are actually no comments at all in this code, and I can parse everything just fine except the above mentioned function

Style

Formatting looks good to me
Naming looks good to me
I did not find obvious repetitions from a DRY perspective


Serializing multiple objects to a file I am very new to Java and although my code works, I know that it must be possible to write this using fewer lines of code. I am serializing multiple objects, eventually I will look into serializing to multiple files, for now, I am using just one file, so it constantly gets over-written. How can I nest or reduce the number of if statements I am using. In my code snippet, I am using an if statement for each individual object, and duplicating my try/catch block. This is horrible code, but I am stumped as to how I can write this more efficiently.

In very simple terms, please give me an example of how to nest or reduce code duplication. I shouldn't need to re-do my try/catch block constantly!

public class Serializer
{
   public void Serialize()
   {

    MainMenu pass_choice = new MainMenu();
    int passed_choice = pass_choice.chooseTeam();

    if(passed_choice==1){
    ClubInfo club = new ClubInfo();
     club.teamName = "Arsenal";
     club.stadium = "Emirates";
     club.division = "Premier League";
      club.SSN = 11122333;
      club.stadiumCapacity = 60000;

    try
      {
         FileOutputStream fileOut =
         new FileOutputStream("/home/cg/root/club.ser");
         ObjectOutputStream out = new ObjectOutputStream(fileOut);

         out.writeObject(club);

         out.close();
         fileOut.close();
         System.out.printf("Serialized data is saved in C:/tmp/club.ser");
         System.out.println("Choice is :" + passed_choice);
      }catch(IOException i)
      {
          i.printStackTrace();
      }
} // end if 1
   else
    if(passed_choice==2){
    ClubInfo club1 = new ClubInfo();
    club1.teamName = "Aston Villa";
    club1.stadium = "Villa Park";
    club1.division = "Premier League";
    club1.SSN = 111223334;
    club1.stadiumCapacity = 40000;

    try
      {
         FileOutputStream fileOut =
         new FileOutputStream("/home/cg/root/club.ser");
         ObjectOutputStream out = new ObjectOutputStream(fileOut);

         out.writeObject(club1);

         out.close();
         fileOut.close();
         System.out.printf("Serialized data is saved in C:/tmp/club.ser");
         System.out.println("Choice is :" + passed_choice);
      }catch(IOException i)
      {
          i.printStackTrace();
      }
} // end if 2
   }
}

I'm going to assume Java 7 for this review. I'm sure somebody else will come along with a Java 8 version that does the whole thing in half a line. :-)

Also, pretty much everything I'm saying here has already been mentioned to you in answers to your earlier questions.

Naming

Methods in java start with a lowercase letter. Variables start with a lowercase letter (ssn, not SSN) and use camelCase, not snake_case.

Formatting

Curly braces should typically not be on their own line in cases like try { and } catch (.... Use whitespace consistently - your code is indented all over the place.

Correctness

Use the try-with-resources paradigm where possible to ensure that resources get cleaned up correctly.

SSN should be a String to preserve any leading zeros.

Design

You are just on the edge of having few enough variables in ClubInfo to have a constructor that just takes them all. That will help simplify the code. A builder would be another reasonable option.

Use methods! Short methods are easy to read and can greatly reduce repetition in your code.

It's generally, though not universally, considered a bad practice to have struct-like objects with public-visibility member variables. While it's easier to develop against, it can make life hard on you later. Using accessor methods is generally preferred.

Separate the building of ClubInfo objects from their serialization. Using a Map will trade off memory footprint and slower startup performance for faster runtime performance and easier-to-read code. Note that in this case the performance differences are insignificant and should be ignored. You might also consider an enum instead.

It's unlikely that ClubInfo initialization should be in your serialization class, but this project is probably extremely small, so it's not awful. Just bear in mind the preferred design practice would be to do it elsewhere.

Putting it all together, a simpler implementation might look more like:

import java.io.File;
import java.io.FileOutputStream;
import java.io.IOException;
import java.io.ObjectOutputStream;
import java.util.Collections;
import java.util.HashMap;
import java.util.Map;

public final class Serializer {

    private static final File OUTPUT_FILE = new File("/home/cg/root/club.ser");

    private static final Map<Integer, ClubInfo> CLUBS = buildClubs();

    public void serialize() {

        final MainMenu passChoice = new MainMenu();
        final int passedChoice = passChoice.chooseTeam();

        final ClubInfo club = CLUBS.get(Integer.valueOf(passedChoice));
        if (club == null) {
            return;
        }

        try {
            this.writeToFile(club);
        } catch (final IOException e) {
            e.printStackTrace(System.err);
            return;
        }
        System.out.printf("Serialized data is saved in " + OUTPUT_FILE);
        System.out.println("Choice is :" + passedChoice);
    }

    private void writeToFile(final ClubInfo clubInfo) throws IOException {
        try (final FileOutputStream fileOut = new FileOutputStream(OUTPUT_FILE);
                final ObjectOutputStream out = new ObjectOutputStream(fileOut)) {
            out.writeObject(clubInfo);
        }
    }

    private static Map<Integer, ClubInfo> buildClubs() {
        final Map<Integer, ClubInfo> clubs = new HashMap<>();

        clubs.put(
                Integer.valueOf(1),
                new ClubInfo("Arsenal", "Emirates", "Premier League", "11122333", 60000));

        clubs.put(
                Integer.valueOf(2),
                new ClubInfo("Aston Villa", "Villa Park", "Premier League", "111223334", 40000));

        return Collections.unmodifiableMap(clubs);
    }
}


Vectorize or otherwise speed up a double for loop I have this function containing some loops and a double for loop. What it does is set up a matrix first to store results in (the task at hand is comparing genomes) and then with sliding window, calculate the differences between sequences. It's very slow for very long sequences, I was hoping somebody know how to speed this up, the part causing the bottleneck is:

n1 <- 0

  for(i in seq(windowp1)) {

    n1 <- n1+1
    n2 <- 0
    setTxtProgressBar(prog, n1)
    for(i in seq(1:ncol(Distances))) {
      n2 <- n2+1

      Distances[n1,n2] <- sum(x[pairs[n2,1],c(windowp1[n1]:windowp2[n1])] != x[pairs[n2,2],c(windowp1[n1]:windowp2[n1])])

    }

    Distances2 <- (Distances/winsize)*100

  }

Where windowp1 is a vector of numbers defining the start of each window. Distances is the pre-made matrix for the results, it has as many cols as there are pairwise comparisons of sequences and as many row's as there are sliding window frames, determined by windowp1 and windowp2. The differences between the sequences are calculated simply by the hamming distance. The loop 1 ticks up the window, the second loop then ticks over to do all comparisons between sequences to do in the window, then the first loop ticks to select the next window. The final line makes the hamming distances in the matrix a % of the widowsize.

Can't see exactly what's going with the sliding windows but it looks like you're calculating the full distance matrix: (i,j) and (j,i). If so, you probably only need half (usually lower) but you can make both with Distances[n1,n2] <- Distances[n2,n1] <- sum ... If this is correct, you'll basically double your speed.

But, if I misunderstood the above, you could still win a little time by reducing the number of index lookup, since these are really slow in R. In particular c(windowp1[n1]:windowp2[n1]) could be pulled out of the loop. Depending on what "x" looks like, you might try "xor(a, b)" rather than "a != b".

Adding a few other changes (eg, drop n1,n2 in favor of the loop variables) but leaving the possible "full matrix" thing I mentioned, here's a suggestion:

distances.ncol <- ncol(Distances)
for(i in seq_along(windowp1)) {
    wp12 <- c(windowp1[i]:windowp2[i])
    setTxtProgressBar(prog, i)

    for(j in seq_len(distances.ncol)) {
        Distances[i,j] <-
            sum(x[pairs[j,1],wp12] != x[pairs[j,2],wp12])
    }
    Distances2 <- (Distances/winsize)*100
}


single byte xor decryption code I've recently started learning rust and doing cryptopals as exercise.
I coded a program that decrypts single xor cypher. The algorithm I came up with is very simple and probably not efficient, but I would like more emphasis on rust philosophy and syntax
extern crate hex;
use std::collections::HashMap;

fn main() {
    let ciphertext = "1b37373331363f78151b7f2b783431333d78397828372d363c78373e783a393b3736";
    let ciphertext_bytes = hex::decode(ciphertext).unwrap();
    let xor_key = find_xor_key(&ciphertext_bytes);
    println!("{}", ciphertext_bytes.iter().map(|x|(x^xor_key) as char ).collect::<String>());
}

fn find_xor_key(ciphertext: &Vec<u8>) -> u8{

    let mut max_score = 0;
    let mut best_key = 0;
    for xor_key in 1..255{
        let xored_vec = ciphertext.iter().map(|x|x^xor_key).collect();
        let score = get_score(xored_vec);
        if score > max_score{
            max_score = score;
            best_key = xor_key;
        }
    }
    println!("{}", max_score);

    best_key
}

fn get_score(text: Vec<u8>) -> u8
{
    let ideal_frequencies: HashMap<char, f32> = [

('E', 11.1607),
('M', 3.0129),
('A', 8.4966),
('H', 3.0034),
('R', 7.5809),
('G', 2.4705),
('I', 7.5448),
('B', 2.0720),
('O', 7.1635),
('F', 1.8121),
('T', 6.9509),
('Y', 1.7779),
('N', 6.6544),
('W', 1.2899),
('S', 5.7351),
('K', 1.1016),
('L', 5.4893),
('V', 1.0074),
('C', 4.5388),
('X', 0.2902),
('U', 3.6308),
('Z', 0.2722),
('D', 3.3844),
('J', 0.1965),
('P', 3.1671),
('Q', 0.1962),

    ].iter().cloned().collect();

    for byte in text.clone(){
        let char_rep: char =  byte as char;
        if !char_rep.is_ascii(){
            return 0;
        }
    }
    let mut score = 1;
    let chars: Vec<char> = text.iter().cloned().map(|x|x as char).collect();
    let frequencies: HashMap<char, f32> = calculate_frequencies(&chars.into_iter().collect::<String>());

    for (letter, frequency) in &frequencies{
        if ideal_frequencies.contains_key(&letter) && //already checked here that it exists in the hashmap
        f32::abs(frequency - ideal_frequencies.get(&letter).unwrap()) <= 10.0{
            score += 1;
        }
    }
    score
}

fn calculate_frequencies(text: &str) ->HashMap<char, f32>{

    let mut frequencies: HashMap<char, f32> = HashMap::new();
    let mut letter_counts:HashMap<char, i32> = HashMap::new();
    let text_upper = text.to_ascii_uppercase();
    for c in text_upper.chars(){
        let count = letter_counts.entry(c).or_insert(0);
        *count += 1;
    }
    for (c, count) in letter_counts{
        frequencies.entry(c).or_insert(count as f32 / text_upper.len() as f32 * 100.0);
    }
    frequencies
}
```

extern crate hex;

extern crate is generally not necessary for programs since the 2018 (currently latest) edition of Rust. You should be able to delete this line, provided that edition = "2018" is in your Cargo.toml file.

let ciphertext_bytes = hex::decode(ciphertext).unwrap();

If you change .unwrap to .expect("failed to decode ciphertext") then there'll be a little more information in the error message. Of course, for a well-made command line program, you'd want to print your own better-formatted error message, instead of a Rust "panic" error.

fn find_xor_key(ciphertext: &Vec<u8>) -> u8{

Accepting &Vec<u8> is usually overly restrictive because it requires the caller to have constructed a Vec, whereas if you change the type to &[u8] they can pass any slice of bytes. This will work without needing to change main, because Vec derefs to a byte slice.
The same applies to get_score's parameter, but you'll need to tweak a couple things to make that work, like giving xored_vec the explicit type Vec<u8> and changing byte as char to *byte as char.

    let ideal_frequencies: HashMap<char, f32> = [
...
    ].iter().cloned().collect();

This is basically a constant, so it'd be nice if we can compute this hash map only once rather than every time we check a score. We can do that with the once_cell crate, which will hopefully soon become part of the standard library. Like this:
static IDEAL_FREQUENCIES: Lazy<HashMap<char, f32>> = Lazy::new(|| [
    ('E', 11.1607),
    ...
].iter().cloned().collect());

for byte in text.clone(){

Use text.iter() instead of text.clone(); it avoids allocating a copy. In general, where both are available, .into_iter() consumes the structure and iterates over its elements by value (moves them), and .iter() does not consume the structure but iterates over references to its elements instead.

let char_rep: char =  byte as char;
if !char_rep.is_ascii(){

You can just use if !byte.is_ascii() { here.

let chars: Vec<char> = text.iter().cloned().map(|x|x as char).collect();
let frequencies: HashMap<char, f32> = calculate_frequencies(&chars.into_iter().collect::<String>());

.cloned().map(|x|x as char) can be written as .map(|x| *x as char).
You're collecting into chars and then immediately iterate to convert it to a String; that isn't necessary because you can just write
calculate_frequencies(&text.iter().map(|x| *x as char).collect::<String>())

But, there's a bigger point: text was already an &[u8], and if we want a &str out of that, that's just std::str::from_utf8.
calculate_frequencies(std::str::from_utf8(text).unwrap())

(.unwrap() will never fail here because we already checked that every byte is ASCII, so it cannot be invalid UTF-8.)

Your loop over the frequencies has several unnecessary &s, and also looks up in the hash map twice instead of once; here's both fixed:
for (letter, frequency) in frequencies.iter() {
    if let Some(ideal_frequency) = IDEAL_FREQUENCIES.get(letter) {
        if (frequency - ideal_frequency).abs() <= 10.0 {
            score += 1;
        }
    }
}

fn calculate_frequencies(text: &str) ->HashMap<char, f32>{

If we want to optimize for performance, which a real code-breaking tool would want to do, then this function should be accepting &[u8], since the only thing it does with the str is split back to characters, and we know all the characters are ASCII at this point. But for more general idiomatic programming, taking &str is fine.

let mut frequencies: HashMap<char, f32> = HashMap::new();
let mut letter_counts:HashMap<char, i32> = HashMap::new();

You can leave out the type declarations here and they will be inferred.

let text_upper = text.to_ascii_uppercase();
for c in text_upper.chars(){

This allocates a String of uppercase unnecessarily; we might as well do it per-character instead:
for c in text.chars() {
    let c = c.to_ascii_uppercase();

(Note, however, that this strategy does not work for Unicode-aware uppercasing, where one char may occasionally become several.)

This is not a matter of Rust idiom at all, but I notice that frequencies is identical to letter_counts except for the values getting scaled. So, we can reuse a single HashMap instead of duplicating it:
fn calculate_frequencies(text: &str) ->HashMap<char, f32>{
    let count_to_frequency_scale = 100.0 / text.len() as f32;

    let mut counts = HashMap::new();
    for c in text.chars() {
        *counts.entry(c.to_ascii_uppercase()).or_insert(0.0) += 1.0;
    }

    // Rescale counts to frequencies
    for count in counts.values_mut() {
        *count *= count_to_frequency_scale;
    }
    counts
}

Note: Using f32 instead of i32 for counting does not introduce any floating-point error, for reasonable string lengths: f32 exactly represents all integers up to 224 - 1 = 16,777,216. (However, there would be accumulated error if we tried to apply count_to_frequency_scale while counting instead of after.)
If it didn't happen to be possible to reuse the same map, I'd be suggesting using iterators to do the transformation instead:
letter_counts
    .into_iter()
    .map(|(key, count)| (key, count as f32 * count_to_frequency_scale))
    .collect()

Here's your code with all of my suggestions, and also formatted with rustfmt. I haven't tested it because I was working in the Rust Playground which doesn't include the hex crate in its available libraries.
extern crate hex;
use once_cell::sync::Lazy;
use std::collections::HashMap;

#[allow(dead_code)]
fn main() {
    let ciphertext = "1b37373331363f78151b7f2b783431333d78397828372d363c78373e783a393b3736";
    let ciphertext_bytes = hex::decode(ciphertext).unwrap();
    let xor_key = find_xor_key(&ciphertext_bytes);
    println!(
        "{}",
        ciphertext_bytes
            .iter()
            .map(|x| (x ^ xor_key) as char)
            .collect::<String>()
    );
}

fn find_xor_key(ciphertext: &[u8]) -> u8 {
    let mut max_score = 0;
    let mut best_key = 0;
    for xor_key in 1..255 {
        let xored_vec: Vec<u8> = ciphertext.iter().map(|x| x ^ xor_key).collect();
        let score = get_score(&xored_vec);
        if score > max_score {
            max_score = score;
            best_key = xor_key;
        }
    }
    println!("{}", max_score);

    best_key
}

fn get_score(text: &[u8]) -> u8 {
    for byte in text.iter() {
        if !byte.is_ascii() {
            return 0;
        }
    }
    let mut score = 1;
    let frequencies = calculate_frequencies(std::str::from_utf8(text).unwrap());

    for (letter, frequency) in frequencies.iter() {
        if let Some(ideal_frequency) = IDEAL_FREQUENCIES.get(letter) {
            if (frequency - ideal_frequency).abs() <= 10.0 {
                score += 1;
            }
        }
    }
    score
}

fn calculate_frequencies(text: &str) -> HashMap<char, f32> {
    let count_to_frequency_scale = 100.0 / text.len() as f32;

    let mut counts = HashMap::new();
    for c in text.chars() {
        *counts.entry(c.to_ascii_uppercase()).or_insert(0.0) += 1.0;
    }

    // Rescale counts to frequencies
    for count in counts.values_mut() {
        *count *= count_to_frequency_scale;
    }
    counts
}

static IDEAL_FREQUENCIES: Lazy<HashMap<char, f32>> = Lazy::new(|| {
    [
        ('E', 11.1607),
        ('M', 3.0129),
        ('A', 8.4966),
        ('H', 3.0034),
        ('R', 7.5809),
        ('G', 2.4705),
        ('I', 7.5448),
        ('B', 2.0720),
        ('O', 7.1635),
        ('F', 1.8121),
        ('T', 6.9509),
        ('Y', 1.7779),
        ('N', 6.6544),
        ('W', 1.2899),
        ('S', 5.7351),
        ('K', 1.1016),
        ('L', 5.4893),
        ('V', 1.0074),
        ('C', 4.5388),
        ('X', 0.2902),
        ('U', 3.6308),
        ('Z', 0.2722),
        ('D', 3.3844),
        ('J', 0.1965),
        ('P', 3.1671),
        ('Q', 0.1962),
    ]
    .iter()
    .cloned()
    .collect()
});
```


Prefered Style Container for the Components in the DataTemplate The following XAML is a close (tidied up) version of what I have use in my WPF application. If you have noticed there are two Styles applied to the same component (Border in this example.)

I would like to know how can I merge the 2nd Style into 1st to make it apply to all the Borders included in the DataTemplate.

<DataTemplate DataType="{x:Type m:myFoo}">
  <DataTemplate.Resources>
    <!-- 1) Style Applied to the Border -->
    <Style TargetType="Border">
      <Setter Property="Margin" Value="1"/>
    </Style>
  </DataTemplate.Resources>
  <Border>
    <Grid>
      <Border>
        <!-- Guts -->
      <Border>
    </Grid>
    <Border.Style>
      <!-- 2) Style Applied to the Border -->
      <Style TargetType="Border">
        <Style.Triggers>
          <DataTrigger Binding="{Binding Path=Active}">
            <Setter Property="Background" Value="OrangeRed"/>
          </DataTrigger>
        </Style.Triggers>
      </Style>
    </Border.Style>
  </Border>
</DataTemplate>

I am familiar with the fact that 2nd Style will only apply to its own component unlike the 1st which will be applied to any Border included in the DataTemplate.

You can merge the two style in the DataTemplate.Resources so that it can be applied in all the Borders inside the DataTemplate. Refer the below code.

<DataTemplate DataType="{x:Type m:myFoo}">
        <DataTemplate.Resources>
            <!-- 1) Style Applied to the Border -->
            <Style TargetType="Border">
                <Setter Property="Margin" Value="1"/>
                <Style.Triggers>
                    <DataTrigger Binding="{Binding Path=Active}">
                        <Setter Property="Background" Value="OrangeRed"/>
                    </DataTrigger>
                </Style.Triggers>
            </Style>
        </DataTemplate.Resources>
        <Border>
            <Grid>
                <Border>
                    <!-- Guts -->
                 </Border>
            </Grid>
        </Border>
    </DataTemplate>


line_to, fill_rect, and clear_rect functions def line_to(x: 0, y: 0, rect: nil)
  unless rect.nil?
    x, y, width, height = rect.x, rect.y, rect.width, rect.height
  end

  `#{@context}.lineTo(#{x}, #{y})`
end

def fill_rect(x: 0, y: 0, width: 0, height: 0, rect: nil)
  unless rect.nil?
    x, y, width, height = rect.x, rect.y, rect.width, rect.height
  end

  `#{@context}.fillRect(#{x}, #{y}, #{width}, #{height})`
end

def clear_rect(x: 0, y: 0, width: 0, height: 0, rect: nil)
  unless rect.nil?
    x, y, width, height = rect.x, rect.y, rect.width, rect.height
  end

  `#{@context}.clearRect(#{x}, #{y}, #{width}, #{height})`
end

I am looking for a way to refactor the above code to take out the first three lines in the methods. I don't want to use Metaprogramming in this specific case; is there any other way?

In case full code will help, see the repo.

If I understand this correctly, you want to be able to call these methods with two different signatures:

method(x: .., y: .., height: .., width: ..)
method(rect: ..)

There has to be some logic in the methods to decide which signature to use. I like your current solution for that. Using metaprogramming would make things too complicated.

Since the signatures are different, you could argue they are actually two different methods: #line_to and #line_to_rectangle for example. You could simply define two different methods:

def line_to(x, y)
  `#{@context}.lineTo(#{x}, #{y})`
end

def line_to_rectangle(rectangle)
  line_to(rectangle.x, rectangle.y)
end

The best solution would be to only expect a single signature in the first place.

One way to do that is to use ** to pass a rectangle object as keyword arguments. For this to work you have to define a #to_hash.

class Rectangle
  def to_hash
    { :x => self.x, :y => self.y, :width => self.width, :height => self.height }
  end
end

...

line_to(**rectangle)

You'll have to make sure that #line_to takes all keys returned by Rectangle#to_hash. That means it should also a width and height keyword argument. You can do this by taking them explicitly or implicitly (def line_to(x: 0, y: 0, **discard)). If you don't take those unnecessary arguments it will raise an exception. Hardly ideal.

Another way to do that would be to create a second type of object to replace the keyword arguments (x, y, width and height). It should have the same interface as rectangle. I'd recommend not complicating things this much, though, and pick a single signature.


Reversing an array in C++ 14, my second array requires an extra element Hackerrank problem to reverse an array of size n.
https://www.hackerrank.com/challenges/30-arrays/
Sample Input:
4
1 4 3 2

Sample output:
2 3 4 1

Much of this code was already given, you can skip to the "This is what I did" section.
#include <bits/stdc++.h>

using namespace std;

vector<string> split_string(string);

int main()
{
    int n;
    cin >> n;
    cin.ignore(numeric_limits<streamsize>::max(), '\n');

    string arr_temp_temp;
    getline(cin, arr_temp_temp);

    vector<string> arr_temp = split_string(arr_temp_temp);

    vector<int> arr(n);
    vector<int> arr2(n);

    for (int i = 0; i < n; i++) {
        int arr_item = stoi(arr_temp[i]);

        arr[i] = arr_item;
    }

    for (int i = 0; i <= n; i++){
        arr2[n-i] = arr[i];
    }
    for (int i = 1; i <= n; i++){
        cout << arr2[i] << " ";
    }
    return 0;
}

vector<string> split_string(string input_string) {
    string::iterator new_end = unique(input_string.begin(), input_string.end(), [] (const char &x, const char &y) {
        return x == y and x == ' ';
    });

    input_string.erase(new_end, input_string.end());

    while (input_string[input_string.length() - 1] == ' ') {
        input_string.pop_back();
    }

    vector<string> splits;
    char delimiter = ' ';

    size_t i = 0;
    size_t pos = input_string.find(delimiter);

    while (pos != string::npos) {
        splits.push_back(input_string.substr(i, pos - i));

        i = pos + 1;
        pos = input_string.find(delimiter, i);
    }

    splits.push_back(input_string.substr(i, min(pos, input_string.length()) - i + 1));

    return splits;
}

This is what I did
I declared another array named array2 of size n to hold the reversed value which would then be printed.
I then wrote this snippet to reverse the array and copy it to array2:
  for (int i = 0; i <= n; i++){
        arr2[n-i] = arr[i];
    }
    for (int i = 1; i <= n; i++){
        cout << arr2[i] << " ";
    }

This code works, but I feel like I have made a mistake since I have iterated n+1 times in the for loop. I tried iterating n times but one element of the array got the value 0. Indexing is also done differently since I am indexing from 1 to n. This is confusing to me, because I declared array2 as size n. How and when did the vector get resized?
Is there a more elegant solution using n iterations? If not, why not?

The first couple of lines show that this is shoddy code:

#include <bits/stdc++.h>

using namespace std;

<bits/stdc++.h> is not a standard header, and dragging all of std into the default namespace is a maintainability disaster (see other Code Review answers, ad nauseum).
None of the code which reads from std::cin ever checks whether the reading was successful, and there's no error handling whatsoever.
arr is a terrible name for a vector.  arr2 is worse.
There's no need for handcrafted loops, when we could use std::copy_n() to read elements, and std::reverse() to reorder them.
Actually, there's no need to reorder the stored data - just write the output using a reverse iterator.

The low quality shows up when we analyse the memory accesses:
valgrind ./255602 <<<$'4\n1 4 3 2 5'
==5474== Memcheck, a memory error detector
==5474== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==5474== Using Valgrind-3.14.0 and LibVEX; rerun with -h for copyright info
==5474== Command: ./255602-orig
==5474==
==5474== Invalid write of size 4
==5474==    at 0x10B466: main (255602.cpp:30)
==5474==  Address 0x4d49000 is 0 bytes after a block of size 16 alloc'd
==5474==    at 0x480BDEF: operator new(unsigned long) (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==5474==    by 0x10E741: __gnu_cxx::new_allocator<int>::allocate(unsigned long, void const*) (new_allocator.h:115)
==5474==    by 0x10E437: allocate (allocator.h:173)
==5474==    by 0x10E437: std::allocator_traits<std::allocator<int> >::allocate(std::allocator<int>&, unsigned long) (alloc_traits.h:460)
==5474==    by 0x10DE3F: std::_Vector_base<int, std::allocator<int> >::_M_allocate(unsigned long) (stl_vector.h:346)
==5474==    by 0x10D60C: std::_Vector_base<int, std::allocator<int> >::_M_create_storage(unsigned long) (stl_vector.h:361)
==5474==    by 0x10CEA0: std::_Vector_base<int, std::allocator<int> >::_Vector_base(unsigned long, std::allocator<int> const&) (stl_vector.h:305)
==5474==    by 0x10C46C: std::vector<int, std::allocator<int> >::vector(unsigned long, std::allocator<int> const&) (stl_vector.h:511)
==5474==    by 0x10B3B6: main (255602.cpp:21)
==5474==
==5474== Invalid read of size 4
==5474==    at 0x10B449: main (255602.cpp:30)
==5474==  Address 0x4d48fb0 is 0 bytes after a block of size 16 alloc'd
==5474==    at 0x480BDEF: operator new(unsigned long) (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==5474==    by 0x10E741: __gnu_cxx::new_allocator<int>::allocate(unsigned long, void const*) (new_allocator.h:115)
==5474==    by 0x10E437: allocate (allocator.h:173)
==5474==    by 0x10E437: std::allocator_traits<std::allocator<int> >::allocate(std::allocator<int>&, unsigned long) (alloc_traits.h:460)
==5474==    by 0x10DE3F: std::_Vector_base<int, std::allocator<int> >::_M_allocate(unsigned long) (stl_vector.h:346)
==5474==    by 0x10D60C: std::_Vector_base<int, std::allocator<int> >::_M_create_storage(unsigned long) (stl_vector.h:361)
==5474==    by 0x10CEA0: std::_Vector_base<int, std::allocator<int> >::_Vector_base(unsigned long, std::allocator<int> const&) (stl_vector.h:305)
==5474==    by 0x10C46C: std::vector<int, std::allocator<int> >::vector(unsigned long, std::allocator<int> const&) (stl_vector.h:511)
==5474==    by 0x10B382: main (255602.cpp:20)
==5474==
==5474== Invalid read of size 4
==5474==    at 0x10B495: main (255602.cpp:33)
==5474==  Address 0x4d49000 is 0 bytes after a block of size 16 alloc'd
==5474==    at 0x480BDEF: operator new(unsigned long) (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==5474==    by 0x10E741: __gnu_cxx::new_allocator<int>::allocate(unsigned long, void const*) (new_allocator.h:115)
==5474==    by 0x10E437: allocate (allocator.h:173)
==5474==    by 0x10E437: std::allocator_traits<std::allocator<int> >::allocate(std::allocator<int>&, unsigned long) (alloc_traits.h:460)
==5474==    by 0x10DE3F: std::_Vector_base<int, std::allocator<int> >::_M_allocate(unsigned long) (stl_vector.h:346)
==5474==    by 0x10D60C: std::_Vector_base<int, std::allocator<int> >::_M_create_storage(unsigned long) (stl_vector.h:361)
==5474==    by 0x10CEA0: std::_Vector_base<int, std::allocator<int> >::_Vector_base(unsigned long, std::allocator<int> const&) (stl_vector.h:305)
==5474==    by 0x10C46C: std::vector<int, std::allocator<int> >::vector(unsigned long, std::allocator<int> const&) (stl_vector.h:511)
==5474==    by 0x10B3B6: main (255602.cpp:21)
==5474==
2 3 4 1 ==5474==
==5474== HEAP SUMMARY:
==5474==     in use at exit: 0 bytes in 0 blocks
==5474==   total heap usage: 9 allocs, 9 frees, 78,336 bytes allocated
==5474==
==5474== All heap blocks were freed -- no leaks are possible
==5474==
==5474== For counts of detected and suppressed errors, rerun with: -v
==5474== ERROR SUMMARY: 3 errors from 3 contexts (suppressed: 0 from 0)

Such failures fall short of working, in my opinion.

This is what a decent C++ program to do this looks like:
#include <algorithm>
#include <cstdlib>
#include <iostream>
#include <iterator>
#include <string>
#include <vector>

int main()
{
    // read the inputs
    std::size_t n;
    std::cin >> n;
    if (!std::cin) {
        std::cerr << "Failed to read number of elements\n";
        return EXIT_FAILURE;
    }

    std::vector<std::string> elements;
    elements.reserve(n);
    std::copy_n(std::istream_iterator<std::string>{std::cin},
                n, std::back_inserter(elements));
    if (!std::cin) {
        std::cerr << "Failed to read element content\n";
        return EXIT_FAILURE;
    }

    // write the values, in reverse order
    std::copy(elements.crbegin(), elements.crend(),
              std::ostream_iterator<std::string>{std::cout, " "});
}

See - no loops or arithmetic required.
Or, if you prefer an exception-based error handling strategy:
#include <algorithm>
#include <cstdlib>
#include <iostream>
#include <iterator>
#include <string>
#include <vector>

// read size from stream, then read that many elements into a vector
template<typename T>
std::vector<T> read_size_and_vector(std::istream& is)
{
    std::size_t n;
    is >> n;
    std::vector<T> elements;
    elements.reserve(n);
    std::copy_n(std::istream_iterator<T>{is}, n, std::back_inserter(elements));
    return elements;
}

int main()
{
    std::cin.exceptions(std::istream::badbit|std::istream::failbit);
    try {
        auto const elements = read_size_and_vector<std::string>(std::cin);
        std::copy(elements.rbegin(), elements.rend(),
                  std::ostream_iterator<std::string>{std::cout, " "});
        return EXIT_SUCCESS;
    } catch (std::ios_base::failure&) {
        std::cerr << "Failed to read the input\n";
        return EXIT_FAILURE;
    }
}


First Dockerfile, I would appreciate critique As the title says, this is my first Dockerfile. It won't be published on any registry, just for my internal use at home. I'd appreciate any critique of what I could do better. I'm currently working on a way to not hard-code the password into the docker-compose file...

This image is to run Postfix on Alpine as a SMTP relay for my devices to alert me (e.g., NAS, security cameras, etc...).

Dockerfile

FROM alpine:3.10

ARG BUILD_DATE

LABEL maintainer="MyNameGoesHere" \
      org.label-schema.schema-version="1.0" \
      org.label-schema.name="postfixrelay" \
      org.label-schema.build-date=$BUILD_DATE

RUN apk add --no-cache --update \
    bash ca-certificates cyrus-sasl-login postfix rsyslog tzdata && \
    rm -rf /var/cache/apk/*

EXPOSE 25

VOLUME [ "/var/spool/postfix" ]

COPY ./entrypoint.sh /

RUN chmod +x /entrypoint.sh

ENTRYPOINT ["/entrypoint.sh"]

entrypoint.sh

#!/bin/sh -e

#Set timezone
if [ ! -f /etc/timezone ] && [ ! -z "$TZ" ]; then
  cp /usr/share/zoneinfo/$TZ /etc/localtime
  echo $TZ >/etc/timezone
fi

# Set Postfix options
postconf -e "inet_interfaces = all" && \
postconf -e "mynetworks = 0.0.0.0/0" && \
postconf -e "relayhost = [$RELAY_IP]:$RELAY_PORT" && \
postconf -e "smtp_use_tls = yes" && \
postconf -e "smtp_sasl_auth_enable = yes" && \
postconf -e "smtp_sasl_security_options = noanonymous" && \
postconf -e "smtp_sasl_password_maps = hash:/etc/postfix/sasl_passwd"&& \
postconf -e "smtp_tls_CAfile = /etc/ssl/certs/ca-certificates.crt" && \

# Create password file
echo "[$RELAY_IP]:$RELAY_PORT   $EMAIL:$PASS" > /etc/postfix/sasl_passwd && \
chown root:root /etc/postfix/sasl_passwd && \
chmod 600 /etc/postfix/sasl_passwd && \
postmap /etc/postfix/sasl_passwd

# Start postfix
postfix start-fg

docker-compose.yml

version: '3'
services:
  postfixrelay:
    container_name: postfixrelay
    restart: unless-stopped
    environment:
      - TZ=America/New_York
      - RELAY_IP=smtp.gmail.com
      - RELAY_PORT=587
      - EMAIL=my_email_address@gmail.com
      - PASS=password
    networks:
      - postfixrelay
    ports:
      - '25:25'
    volumes:
      - 'postfixrelay_data:/var/spool/postfix'
    image: postfixrelay

networks:
  postfixrelay:

volumes:
  postfixrelay_data:
    driver: local

I would set the execute permissions on the entrypoint.sh before building the docker image - RUN, COPY and ADD create layers - COPY preserves the file permissions so you can remove the RUN chmod +x /entrypoint.sh (a layer from your final image).

FROM alpine:3.10

ARG BUILD_DATE

LABEL maintainer="MyNameGoesHere" \
      org.label-schema.schema-version="1.0" \
      org.label-schema.name="postfixrelay" \
      org.label-schema.build-date=$BUILD_DATE

EXPOSE 25

VOLUME [ "/var/spool/postfix" ]

COPY ./entrypoint.sh /

RUN apk add --no-cache --update \
        bash \
        ca-certificates \
        cyrus-sasl-login \
        postfix \
        rsyslog \
        tzdata && \
    rm -rf /var/cache/apk/*

ENTRYPOINT ["/entrypoint.sh"]

... moved the EXPOSE, VOLUME and COPY instructions above the RUN - the VOLUME and EXPOSE are not likely to change and the build cache for the COPY will only be invalidated if the contents of the file changes as explained in the ADD or COPY best practices. Finally, split the packages being added on multiple lines to make it clearer at a glance in i.e. a git diff when something changes.

Concerning the entrypoint.sh I'd suggest taking a look at the Exec form ENTRYPOINT example to ensure that your process is receiving the Unix signals.

For the docker-compose I'd pass the PASS environment variable from the hosts environment and name the default network explicitly:

version: '3'
services:
  postfixrelay:
    container_name: postfixrelay
    restart: unless-stopped
    environment:
      ...
      - PASS=${PASS}
...
networks:
  default:
    name: postfixrelay
...

If you want to docker-compose up --scale postfixrelay=3 you'd need to stop using container_name, trying to start multiple containers with the same name will conflict. You'd also need to stop publishing the ports to the host, will have port conflicts - would need to set up a tcp load balancer (i.e. nginx, traefik, ...) and proxy the connections to your upstream services.

Then there is docker swarm and kubernetes (or other) as proper orchestration managers for your service(s).


C++ member function wrapper for interfacing with C APIs Some C API interfaces use callbacks. Often these callbacks have a user-defined parameter of type void * which can be used as a class instance pointer in C++. Usually in order to call the member function you need to write a static class function which would cast the void * parameter to an instance pointer and call the member function. I wanted to write a wrapper that would instantiate a static function which takes void * as a parameter and the rest of arguments is exactly like in the member function passed to wrapper.

template<
    typename Result,
    typename Function,
    auto Func
>
struct instance_call_helper;

template<
    typename Result,
    typename Base,
    typename... Args,
    auto Func
>
struct instance_call_helper<
    Result,
    Result(Base::*)(Args...),
    Func
> {
    static Result call(void * ref, Args ... args) {
        Base * instance = reinterpret_cast<Base *>(ref);
        return (instance->*Func)(std::forward<Args>(args)...);
    }
};

template <
    typename Base
> struct instance_type_helper;
template <
    typename Result, typename Base, typename... Args
>
struct instance_type_helper<Result(Base::*)(Args...)> {
    using result_t = Result;
};

template<auto Func>
#if __cpp_concepts >= 201507
requires std::is_member_function_pointer<decltype(Func)>::value
#endif
class instance_call {
    using functor_t = decltype(Func);
    using type_helper = instance_type_helper<functor_t>;

public:
    using helper = instance_call_helper<
        typename type_helper::result_t,
        functor_t,
        Func
    >;
};

Intended usage:

class Bar {
private:
    int m_a1;

public:
    Bar(int a1):
        m_a1(a1) {}

    int baz(int a2) {
        return m_a1 + a2;
    }
};

int foo() {
    Bar b { 3 };
    // Pointer we are going to pass somewhere as a callback
    // f has signature int (*)(void*, int)
    auto * f = &instance_call<&Bar::baz>::helper::call;
    // Intended way of calling it — returns 5
    return f(&b, 2);
}

After using the code for a while I noticed the following changes could be used:

I don't need instance_type_helper class. Result type will be taken out of the function type. Thus, instance_call_helper turns into following:

template<
    typename Function,
    Function F
>
struct instance_call_helper;

template<
    typename Result,
    typename Base,
    typename... Args,
    Result(Base::* Func)(Args...)
>
struct instance_call_helper<
    Result(Base::*)(Args...),
    Func
>

In order to use this code with GCC 6, one can check the value of __cpp_deduction_guides macro:

#if __cpp_deduction_guides >= 201606
template<
    auto F
>
#else
template<
    typename Function,
    Function F
>
#endif

For use with C code (it's not portable, as noticed in the comments), some more advanced typechecking can be used for arguments, like std::is_fundamental or std::is_pod