A simple Dice Game Using only Static methods This is a simple dice game that requires you  to get two numbers from the user and then it generates 15 other numbers and checks if the generated numbers are the same as the numbers typed in.

Try to run it on your IDE it will look more cool.

As seen below  I have  used only static methods:

import java.util.Random;

import java.util.Scanner;

public class DiceGame{

    static int first,second,dice;
    static int elments[] = new int [15];

    public static void main(String [] args){

        /*
        Hello Guys this is a simple dice game that requires you  type  two numbers from the user and then it
        generates 15 other numbers and checks if generated numbers are the same with your numbers typed in
        Try to run it on your IDE it will look more cool.
        As seen below  I have  used only static methods
        */

        getFirstNumberInput();
        getSecondNumberInput();
        validateFirstNumber();
        validateSecondNumber();
        generateDice();
        printAllelments();
        checkFirstNumberifSameAsGenerated();
        checkSecondNumberifSameAsGenerated();

    }

    static void getFirstNumberInput(){

        Scanner sc = new Scanner(System.in);

        System.out.print("\n type your first number from 1 to 6: ");
        first = sc.nextInt();
    }
    static void getSecondNumberInput(){
        Scanner sc = new Scanner(System.in);

        System.out.print("\n type your second number from 1 to 6 :");
        second = sc.nextInt();
    }
    static void validateFirstNumber(){
        if(first > 6){
            System.out.print("\nyou cant type a number greater than 6");
            getFirstNumberInput();
        }else if(first < 0){
            System.out.print("\n you cant type a negative number ");
            getFirstNumberInput();
        }else{
            validateSecondNumber();
        }
    }

    static void validateSecondNumber(){
        if(second> 6){
            System.out.print("\nyou cant type a number greater than 6");
            getSecondNumberInput();
        }if(second < 0) {
                System.out.println("\nyou cant type a negative number");
                getSecondNumberInput();
            }else {
                generateDice();
            }
        }

    static void generateDice(){
        Random generate = new Random();

        for(int iterator = 0; iterator < 15; iterator++){
            dice = 1+generate.nextInt(6);
            elments[iterator] = dice;

            }

    }

    static void printAllelments(){
        for(int i = 0;i<=14;i++){
            System.out.print(elments[i]+"\t");

            if(i % 5 == 0){
                System.out.println("\n\n\n\n");
            }

        }
    }

    static void checkFirstNumberifSameAsGenerated(){

            int same = 1;
        for(int i = 0 ;i< elments.length;i++){
            if(elments[i] == first){
                System.out.println();
                System.out.println("your  first number that you typed : [ "+first+" ] is the same as this [ "+elments[i]+" ] that was generated  { "+
                                    same+++" } similar numbers");

            }
        }
    }
    static void checkSecondNumberifSameAsGenerated(){

            int same = 1;
        for(int i = 0 ; i< 15;i++){
            if(elments[i] == second){
                System.out.println();
                System.out.println("your second number that you typed : [ "+second+" ] is the same as this [ "+elments[i]+" ] that was generated  { "+
                                    same+++" } similar numbers");

            }
        }
    }

}

Here's my list of points (don't be discouraged by it):

You have multiple functions which appear to do similar things with only minute differences. For example, getFirstNumberInput() and getSecondNumberInput() appear to be the same, however one uses the word "first" in its print and the other uses "second". So that you avoid repeating your code, I would create a single function which can handle both operations. Either you pass in the text "first" or "second" as an input to that function, or you reword the print to make those words unnecessary to state (for example "Enter a number from 1 to 6").
Don't forget to close your scanners when you're done with them.
Not sure if this was an issue you faced when copying your code into the question, but if it isn't then you should also sort out your indentations by having indentation before closing braces line up with the indentation of the line where the matching opening brace was.
Your code uses recursion for the validation - I would recommend changing it so that the input is not accepted until a valid value is provided, rather than sending your program into a potentially infinite stack of function calls. This means that you should use an until loop to check if input is valid before moving out of the loop and setting the input value.
Your code uses class variables when it doesn't need to - the scope of the variables should be restricted to main and then the functions should return values which will be assigned to the variable, so rather than what you currently have you end up with int firstInput = getInput(). I would never personally have the function itself assign a value, but rather it should always return a value which the main method can handle.
Variable naming is important. Whilst this is quite a small program, and a programmer would easily be able to look at the code to see how your variables are being used, it would be much clearer for someone to see a variable called "firstInput" rather than "first". Concise variable names are important.
Despite the fact that you call validateSecondNumber as part of the main method, you call it within the validateFirstNumber method as well. Not only does that mean that the method runs more times than it needs to, but you shouldn't really be calling validateSecondNumber from validateFirstNumber. Each method should only do the task it is set - therefore, validateFirstNumber should only validate the first number. It should not also validate the second number.
Due to the way your validateSecondNumber program logic is handled, if you enter a value above 6 multiple times then it will be accepted as a valid input after a few tries.
You should validate inputs as you receive them, so there is no confusion for the user, rather than getting both inputs and then validating them.
A minor point at this stage, but it builds off of what I said about a method only handling what it is designed to. Many of the methods here use prints to tell the user of a mistake that has been made. Instead (in the case of validation routines), you should simply make the main method aware of the mistake made, such as inputting a value that is below zero, and having the main method handle it. This can be in the form of throwing a relevant exception and allowing the main method to catch that and handle it.
You ask for a value inclusively between 1 and 6, but also seem to accept 0 as a valid input. This is because you check if the input < 0 instead of <= 0.
In your last routine, you typed same+++. Instead, I would increment the variable on a separate line to avoid confusion due to multiple plus signs.
\n isn't always enough to present a new line, depending on the environment which the program is running in. Use \r\n for more certainty that it will work on all platforms. The best thing to do would be to get the line.separator property and use that in place of \n.
Unless you have been given a task which specifically says to use only static methods, I would avoid that approach in the future.


Filter an array of objects by an array in AngularJS This code is the whole logic for the AngularJS ui-select lib. I'm not (yet) a JS pro and still learning AngularJS as well but I'm pretty sure this can be done a little more efficient? I'm talking especially about the search: part of the object. In php I would probably do something like array_diff. Any critiques are welcome!

The reason for the selectedIds: [] is that I use ng-repeat to render a list of hidden fields with name="users[]" to be able to fetch the list as a php array on the server side. I'm not suing Angular as a single page app here, I'm just using it for all the widgets but the data itself is submitted with the whole page as POST.

The for (var key in result.data) is done to filter already selected ids out of the array. It was the most easy solution to avoid the error Duplicates in a repeater are not allowed.. Sure I could pass the already existing ids to the lookup GET request but I think this works fine on the client side as well.

$scope.usersList = {
    items: [],
    selected: [],
    selectedIds: [],
    extractIds: function() {
        $scope.usersList.selectedIds = [];
        for (var i = 0; i < $scope.usersList.selected.length; i++) {
            $scope.usersList.selectedIds.push($scope.usersList.selected[i].id);
        }
    },
    select: function($item, $model) {
        this.extractIds();
    },
    remove: function($item, $model) {
        this.extractIds();
    },
    search: function(term) {
        if (term.length > 2) {
            UsersService.lookup(term).then(function(result) {
                for (var key in result.data) {
                    if (result.data.hasOwnProperty(key)) {
                        if ($scope.usersList.selectedIds.indexOf(result.data[key].id) != -1) {
                            delete result.data[key];
                        };
                    }
                }
                $scope.usersList.items = result.data;
            });
        }
    }
};

welcome to CodeReview :-)

Firstly, I would mention that your $item and $model parameters should just be item and model. The dollar-sign prefix is conventionally reserved for official vendor libraries from Angular, and should not be used in your code to prevent confusion. I initially thought $model was a derivative of ngModel!

Secondly, do you need selectedIds? Can you not just use a function to extract those IDs? You do not need to watch a seperate array of IDs.

Thirdly - delete result.data[key] - this is not how you use delete. You use delete to remove properties from objects, not key/value pairs from collections. Instead, you should just set it to undefined.. with that said, you don't even need to use delete here anyway.

Fourthly.. what the hell is going on with for(key in result.data) if(result.data.hasOwnProperty(key)... that code is iterating the data then checking that the data has that property. wat. This works for circumventing prototypal inheritance, however you can just as easily use Object.keys(). Not to mention that the result of a $http request is only going to inherit from Object.

With that said, here's a refactor of your code. You will need lodash for this code to work, but no worries - lodash is probably the most handy library you will ever come across in JS. I use it on every project these days.

$scope.usersList = {
  items: [],
  search: function(term) {
    if(term.length > 2) {
      UsersService.lookup(term)
        .then(function(xhr) {
          return _.reject(xhr.data, function(item) {
            return _.contains($scope.usersList.items, item);
          });
        })
        .then(function(data) {
          $scope.usersList.items = data;
        });
    }
  }
};

I've removed the select, remove and extractId functions as they do nothing in my example (and they did nothing anyway!)

This could definitely be refactored even better in my opinion, but I would need more scope on the actual problem to do that. For example, there's no need to use the $scope here aside from exposing the results to the view. This would be much better done if it was encapsulated inside of a function and only the result was exposed. This should really be in two separate classes at least.


Increase Perfomance of Nested Object Instantiation C# I'm filling a list of items from the database, which is taking extremly long time to run (Approx 60 seconds)

Here is the code with the relevant breakdown

97% of the performance issues come in the foreach loop at the bottom.   Is there a better way to match list items on the nested object.

private List<ROPSetting> TestMethod()
{

    using DataSet ds = DB.GetDs($"EXEC GetSupplyChainROP_C @DateFrom={DateTime.Now.AddYears(-1).ToDbQuote()}");
    // load DS takes 3 seconds.

    using var itemReader = ds.Tables[0].CreateDataReader();

    var items = itemReader.ParseList<ROPSetting>().ToList();

    using var usageReader = ds.Tables[1].CreateDataReader();
    var usageItems = usageReader.ParseList<ItemUsage>().ToList();

    // 97% of the performance impact is below.  Is there a better way to match
    foreach (var itm in items)
    {
        itm.Usage = usageItems
                     .Where(a => a.ItemCode == itm.ItemCode && a.WarehouseCode == itm.WarehouseCode).ToList();

    }

    return items;

}

public class ItemUsage
{
    public DateTime Date { get; set; }
    public decimal Quantity { get; set; }
    internal string ItemCode { get; set; } // for matching
    internal string WarehouseCode { get; set; } // for matching
}
public class ROPSetting
{
    public string ItemCode { get; set; }
    public string PartNumber { get; set; }
    public decimal AverageCost { get; set; }
    public string SupplierCode { get; set; }
    public string SupplierName { get; set; }
    public int TotalLeadTime { get; set; }
    public int IntervalTime { get; set; }
    public int ReorderPoint { get; set; }
    public string WarehouseCode { get; set; }
    public IList<ItemUsage> Usage { get; set; }
}

Option 1) You can create a DataRelations between the two Data Tables and change the loop to fill in both classes at the same time.  This would require you do change the code that converts the results set into C# classes.  Microsoft has an example of GetChildRows and navigation here

Option 2) Otherwise you could also create a DataView and set the Sort property on the child Data Table and use FindRows method. Again will require you to change how you convert from datatable to C# classes.  I'd lean toward the first option instead of creating another set of data objects just for sorting.

Option 3) I'm guessing it will not be much better performance but you can try is using the Linq GroupJoin

foreach (var data in items.GroupJoin(usageItems,
    x => new {x.ItemCode, x.WarehouseCode},
    x => new {x.ItemCode, x.WarehouseCode},
    (itm, usg) => new
    {
        item = itm,
        usage = usg.ToList()
    }))
{
    data.item.Usage = data.usage;
}

Update #1 - Couple more options.

Option 4) Parallel the finding of data. Take Option 3 and add the AsParallel()

foreach (var data in items.AsParallel().GroupJoin(usageItems.AsParallel(),
    x => new {x.ItemCode, x.WarehouseCode},
    x => new {x.ItemCode, x.WarehouseCode},
    (itm, usg) => new
    {
        item = itm,
        usage = usg.ToList()
    }))
{
    data.item.Usage = data.usage;
}

I still think option 1 would be the best option as it will create a binary search tree for the relationship and give best performance.

Option 5) if you know the usageItems are sorted by the ItemCode and WarehouseCode you could just iterate over the list once and fill in the Item as it scans over the list.  This would be the most efficient but would require more complex code and also required that sort order is always correct.  While having this sort order would help Option 1 it wouldn't be required.


Method to return date ranges of 1 year I made a method that takes 2 DateTimes as a parameter, a startDate and a stopDate.

The routine should return a list of DateTime ranges by checking if the range in the parameters is above one year TimeSpan. If it is, it should separate the range into smaller ranges of one year, starting form startDate and adding a year until reaching the limit, and then adjusting the last range accordingly (as it might only contain a few months).

Can this method be made better or even recursive?

// startDate = 2016-01-01
// endDate = 2020-06-01

List<Tuple<DateTime,DateTime>> list = GetYearsBetweenDates(startDate, endDate);

//list = 2016-01-01 2016-12-31
//       2017-01-01 2017-12-31
//       2018-01-01 2018-12-31
//       2019-01-01 2019-12-31
//       2020-01-01 2020-06-01

private List<Tuple<DateTime, DateTime>> GetYearsBetweenDates(DateTime startDate, DateTime stopDate)
{
    var list = new List<Tuple<DateTime, DateTime>>();
    var oneYear = TimeSpan.FromDays(365);
    var tempStopDate = startDate + oneYear;
    var tempStartDate = startDate;
    bool first = true;

    if (stopDate - startDate < oneYear)
    {
        list.Add(new Tuple<DateTime, DateTime>(startDate, stopDate));
        return list;
    }

    while (tempStopDate != stopDate)
    {
        list.Add(new Tuple<DateTime, DateTime>(tempStartDate, tempStopDate));
        if (first)
        {
            tempStartDate += oneYear.Add(TimeSpan.FromDays(1)); // We should not have the same date twice
            first = false;
        }
        else
            tempStartDate += oneYear;

        tempStopDate += oneYear;

        if (tempStopDate > stopDate)
        {
            tempStopDate = stopDate;
            list.Add(new Tuple<DateTime, DateTime>(tempStartDate, stopDate));
        }
    }

    return list;
}

If you perform all calcuclations dynamically your method becomes a simple few lines long loop:

private IEnumerable<DateTimeRange> GetYearsBetweenDates(DateTime startDate, DateTime stopDate)
{
    for (int i = startDate.Year; i <= stopDate.Year; i++)
    {
        yield return new DateTimeRange
        (
            start: new DateTime(i, startDate.Month, startDate.Day),
            end:
             i == stopDate.Year
             ? new DateTime(i, stopDate.Month, stopDate.Day).AddYears(1)
             : new DateTime(i, startDate.Month, startDate.Day).AddYears(1)
        );
    }
}

So what I have I changed?

The method returns an IEnumerable<T> and uses the yield return which makes it deferred (this means it is executed only if enumerated).

The T is a new struct that is easier to understand than a Tuple:

struct DateTimeRange
{
    public DateTimeRange(DateTime start, DateTime end)
    {
        Start = start;
        End = end;
    }
    public DateTime Start { get; }
    public DateTime End { get; }
}

To get all dates you call it with ToList or ToArray

var dates = GetYearsBetweenDates(startDate, endDate).ToList();

or you put it in a foreach loop

foreach(var range in GetYearsBetweenDates(startDate, endDate))
{
    // do something
}

You can also exagerate and make it pure LINQ method without any manual loop but personaly I think the for loop looks better:

private IEnumerable<DateTimeRange> GetYearsBetweenDates3(DateTime startDate, DateTime stopDate)
{
    return Enumerable.Range(startDate.Year, stopDate.Year - startDate.Year + 1)
        .Select(x => new DateTimeRange
        (
            start: new DateTime(x, startDate.Month, startDate.Day),
            end:
             x == stopDate.Year
             ? new DateTime(x, stopDate.Month, stopDate.Day).AddYears(1)
             : new DateTime(x, startDate.Month, startDate.Day).AddYears(1)
        ));
}

What else?

var list = new List<Tuple<DateTime, DateTime>>();

You should always choose meaningful names for all variables. You did it well for the others. This could have been results or dates.

var oneYear = TimeSpan.FromDays(365);

It's safer to use the AddYear(1) method rather then hardcoding the number. Especially if you don't want to care about leapyears.


Discord bot for ordering food I'm new to JavaScript, and I made a Discord bot using discord.js that serves food and drinks and replies without the need of a prefix. My issue is that I want to try cleaning up the code and get rid of all the if-statements, but I don't know how to do so.
bot.on("message", message => {
  const reply = [
    "Here's your order!",
    "Enjoy!",
    "Here you go!",
    "Let me know if you need anything else!",
    "You've been served.",
    "That'll be $3.50.",
    "That'll be $5.50",
  ];
  const response = reply[Math.floor(Math.random() * reply.length)];

  if (message.content.toLowerCase().includes("order coffee")) {
    message.channel.send("Coming right up!").then(msg => {
    msg.delete({ timeout: 5000 })
  })
  .catch(console.error);
    setTimeout(function(){
      message.reply(response);
      message.channel.send("https://cdn.glitch.com/1f3ad41e-6c0c-4160-902b-a50330e419bd%2FCoffee%20cropped.gif?v=1613444146008");
    }, 5000);
  }

  if (message.content.toLowerCase().includes("order latte")) {
    message.channel.send("Coming right up!").then(msg => {
    msg.delete({ timeout: 5000 })
  })
  .catch(console.error);
    setTimeout(function(){
      message.reply(response);
      message.channel.send("https://cdn.glitch.com/1f3ad41e-6c0c-4160-902b-a50330e419bd%2FLatte.gif?v=1613444145762");
    }, 5000);
  }

  if (message.content.toLowerCase().includes("order muffin")) {
    message.channel.send("Coming right up!").then(msg => {
    msg.delete({ timeout: 5000 })
  })
  .catch(console.error);
    setTimeout(function(){
      message.reply(response);
      message.channel.send("https://cdn.glitch.com/1f3ad41e-6c0c-4160-902b-a50330e419bd%2FMuffin.gif?v=1613444146225");
    }, 5000);
  }

  if (message.content.toLowerCase().includes("order strawberry cake roll")) {
    message.channel.send("Coming right up!").then(msg => {
    msg.delete({ timeout: 5000 })
  })
  .catch(console.error);
    setTimeout(function(){
      message.reply(response);
      message.channel.send("https://cdn.glitch.com/1f3ad41e-6c0c-4160-902b-a50330e419bd%2FStrawberry%20cake%20roll.gif?v=1613444145762");
    }, 5000);
  }
});

The first thing to look at is: what is the same for each if statement and what is different. The key differences are:
message.content.toLowerCase().includes("this string here is what's different")

and
message.channel.send("links are different")

To clean it up, we can get rid of the if statements all together and create an object that contains key:value pairs with key message.content you want to check against with and value the associated media link. That would look like this:
const gifs = {
  "coffee": "https://cdn.glitch.com/1f3ad41e-6c0c-4160-902b-a50330e419bd%2FCoffee%20cropped.gif?v=1613444146008",
  "latte": "https://cdn.glitch.com/1f3ad41e-6c0c-4160-902b-a50330e419bd%2FLatte.gif?v=1613444145762",
  "muffin": "https://cdn.glitch.com/1f3ad41e-6c0c-4160-902b-a50330e419bd%2FMuffin.gif?v=1613444146225",
  "strawberry cake roll": "https://cdn.glitch.com/1f3ad41e-6c0c-4160-902b-a50330e419bd%2FStrawberry%20cake%20roll.gif?v=1613444145762",
};

We can take out the string "order" because that is also common to all of the keys. If the data you are getting back requires you to check on "order coffee" then change it back.
Now you can map through the keys of the gifs object to see if that key is included in the message content. If it is, return the value associated with that key.
Object.keys(gifs).forEach(key => {
  if(message.content.toLowerCase().includes(key)) {
    return gifs[key]; // get the value at key in the gifs object
  }
}

Object.keys() takes an object and returns an array of the keys in that object. _.forEach is a method on an Array that loops through an array just like a for loop. It takes a callback function as a param.
Putting that all together we get:
// can define this in a helper / util file or at the top of this file since it is a constant list
const REPLIES_LIST = [
    "Here's your order!",
    "Enjoy!",
    "Here you go!",
    "Let me know if you need anything else!",
    "You've been served.",
    "That'll be $3.50.",
    "That'll be $5.50",
];

bot.on("message", message => {
  const { channel, content, reply: messageReply} = message; // destructure variables off of message, reply: messageReply renames reply to messageReply
  const response = REPLIES_LIST[Math.floor(Math.random() * REPLIES_LIST.length)];

  Object.keys(gifs).map(key => {
   if(content.toLowerCase().includes(key)) {
    channel.send("Coming right up!").then(msg => {
      msg.delete({ timeout: 5000 })
    }).catch(console.error);

     setTimeout(() => {
      messageReply(response);
      channel.send(gifs[key]); // get the value at key in the gifs object
    }, 5000);
   }
  });
});

Code Walkthrough

The bot gets a message "message" we declare temporary variables equal to message.content, message.channel, and message.reply by destructuring
Create another variable equal to a random response from the reply list or array
Traverse through the keys in the gifs object using forEach
For each item in the gifs object we check to see if the message.content.toLowerCase() includes that key, if it does we send a message "Coming right up!" to the channel
Handle some logic and a catch after sending
Create a setTimeout that waits 5 seconds before replying with the response and channel.sending the gif url

Updated: 02/17/21
There were comments questioning the use of _.map over _.forEach or _.some. We should be using _.forEach in this case and I updated the original answer to reflect that. Below I highlighted the differences between the three and ultimately what works best for this scenario.
Array.prototype.map((item) => // do something)
A method that iterates through every element of an array and returns a new array resulting from the callback function you passed.
Example:
 const list = [1, 2, 3, 4, 5];
 const newList = list.map((item) => item + 2); // [1+2, 2+2, 3+2, 4+2, 5+2]

 console.log(list); // [1, 2, 3, 4, 5]
 console.log(newList); // [3, 4, 5, 6, 7]

We only use map if we are using the array that the map returns (in this example newList. In the original answer, we were using map without returning anything or using the returned array which is an anit-pattern.
Array.prototype.forEach((item) => // do something)
A method that iterates through an array and uses a callback to mutate the array, returning undefined.
Example:
 const list = [1, 2, 3, 4, 5];
 list.forEach((item, index, arr) => arr[index] = item + 2);

 console.log(list); //[3, 4, 5, 6, 7]

The forEach provides you with the current item (item), index of the current item in the array (index), and the original array (arr).
We use forEach when we want to use the callback passed to mutate the array without returning a new array. Key thing to note, you cannot exit a forEach without raising an exception.
Array.prototype.some((item) => // do something)
A method that checks if at least one element in the array "passes" the callback you provide, returning a boolean.
Example:
 const list = [1, 2, 3, 4, 5];
 list.some((item) => item === 2); // true, stops when it reads 2 in the array because it matches

 console.log(list); //[1, 2, 3, 4, 5]

For the question that was asked, forEach is most appropriate. You then have to contemplate, will my data have duplicates or not have anything at all and handle those conditions. If you will have duplicates, you may want to you the find method approach Daniël van den Berg suggests. find is similar to forEach and some. It looks for the first item that matches the condition and returns that item.


Return Unique items in a Go slice I'm trying to return unique items in a Go slice. I've landed on the following, but it seems rather sub-optimal. I'm new to golang (first project in it) and wondering if there's a better way to approach this?

//
// Returns unique items in a slice
//
func Unique(strSlice []string) []string {
    keys := make(map[string]bool)
    list := []string{}
    for _, entry := range strSlice {
        if _, value := keys[entry]; !value {
            keys[entry] = true
            list = append(list, entry)
        }
    }
    return list
}

This is not idiomatic writing style in Go, because the value variable is misleading:

if _, value := keys[entry]; !value {

The first returned value is the value in the map, the second value indicates success or failure of the lookup.
So rename it to ok or found.

If the slice is very large,
then list = append(list, entry) may lead to repeated allocations.
In that case, you can optimize by preallocating list to the maximum possible capacity (len(strSlice)), and assign elements by index rather than using append.
But if you have no reason for this optimization then don't do it,
that would be premature optimization,
the current solution is fine.


python (optimizing) bubble sort I saw in Wikipedia (the first under Optimizing bubble sort) a pseudo-code to bubble sort and I implemented it in Python.

I am posting my implementation here because I'm new to Python, and I'm looking mostly for programming in a more pythonic way.

def bubbleSort(array):
    n = len(array)
    swapped = True
    while swapped:
        swapped = False
        for i in range(n - 1):
            if array[i] > array[i + 1]:
                array[i], array[i + 1] = array[i + 1], array[i]
                swapped = True
        n -= 1
    return array

def main():
    array = [1, 7, 4, 3, 2, 9, 8, 5, 6]
    array = bubbleSort(array)
    print(array)

if __name__ == '__main__':
    main()

I'd love to know if there are any stuff that I can optimize (syntactic-wise), like the fact that I'm writing swapped = True, and afterward changing it again, and again ... which thing annoys me.

Also a beginner but one thing I would like to point out is naming function, Style Guide for Python Code states:

  Function Names
  Function names should be lowercase, with words separated by underscores as necessary to improve readability.

  mixedCase is allowed only in contexts where that's already the prevailing style (e.g. threading.py), to retain backwards compatibility.

So, in your case write bubble_sort instead of bubbleSort.


Bluebird: promisify xhr request I use bluebird promises. I want to promisify my requests from communication layer:

Utils.js

//Question: is there more beautiful way to do that?
promisifyXMLHttpRequest(xhr, timeout = 0, timeoutCallback = _.noop, responseHandler = response => JSON.parse(response).data) {
    const requestPromise = new Promise((resolve, reject) => {
        xhr.onload = () => {
            if (xhr.readyState === 4) {
                if (xhr.status === 200) {
                    resolve(responseHandler(xhr.responseText));
                } else {
                    reject(xhr.statusText);
                }
            }
        };
        xhr.onerror = function (e) {
            reject(xhr.statusText);
        };
    });

    if (timeout) {
        requestPromise.timeout(timeout)
            .catch(TimeoutError, error => {
                timeoutCallback(error);
            });
    }

    return requestPromise;
}

Communication

_sendRequest(body, sessionId, timeout, timeoutCallback, responseHandler) {
    const xhr = new XMLHttpRequest();
    xhr.open('POST', SOME_URL, true);
    if (sessionId) xhr.setRequestHeader('swarm-session', sessionId);
    const requestPromise = Utils.promisifyXMLHttpRequest(xhr, timeout, timeoutCallback, responseHandler);
    xhr.send(JSON.stringify(body));
    return requestPromise;
}

This seems quite fine to me.
One thing that pops to mind is that I lose the ability to see the status code of the error as you only reject with the message text. I would wrap that in an object where the statuscode is also exposed.


Sieve of Eratosthenes optimization I am making a program to find primes between the given range of numbers. Unfortunately, the algorithm I created is too slow. Do you have any ideas of how to optimize the code?

import java.io.*;
import java.util.*;

public class Main {

    public static void main(String[] args)throws IOException {

        Scanner s = new Scanner(System.in);
        int numberOfTests;
        int l, n;
        numberOfTests = s.nextInt();

        for (int q = 0; q < numberOfTests; q++) {
            l = s.nextInt();
            n = s.nextInt();
            if (l==1)l++;
            BitSet primeBitSet = new BitSet((int) (Math.sqrt(n) + 1));
            ArrayList <Integer> primeTable = new ArrayList <Integer>((int) Math.sqrt(n));
            if (l < 1 || n < 1 || l > n || n > 1000000000) {
                System.exit(0);
            }

            //classic sieve from 1 to sqrt(n)
            for (int j = 2; j * j <= n; j++) {
                if (primeBitSet.get(j - 1) == true) {
                    continue;
                } else {

                    primeTable.add(j);
                    for (int k = 2 * j; k <= n; k += j) {
                        primeBitSet.set(k - 1);
                    }
                }
            }

            //sieve in the expected range using primes generated by the classic sieve
            BitSet primesInRangeBitSet = new BitSet(n - l + 1);
            int pomoc;
            for (int i = 0; i < primeTable.size(); ++i) {

                pomoc = l / ((int) primeTable.get(i));
                pomoc = pomoc * ((int) primeTable.get(i));
                for (int j = pomoc; j <= n; j = j + (int) primeTable.get(i)) {

                    if (j < l || primesInRangeBitSet.get(j - l) == true ) {
                        continue;
                    } else {
                        primesInRangeBitSet.set(j - l);
                    }
                }

            }

            //outputting the primes
            for (int k = 0; k < primeTable.size(); k++){
                if((int)primeTable.get(k)>= l &&(int) primeTable.get(k) <= n)
                    System.out.println(primeTable.get(k));
            }
            for (int k = 0; k < primesInRangeBitSet.size(); k++) {
                if (primesInRangeBitSet.get(k) == false && k <= n - l ) {
                    System.out.println((k + l));
                }
            }
            System.out.println("");
        }
    }
}

Variable scope

In order to make things easier to read/write, it is prefered to declare local variables as late as possible and in the smallest possible scope.

In your case, this holds for :

int numberOfTests = s.nextInt();

int l = s.nextInt();

int n = s.nextInt();

int pomoc = l / ((int) primeTable.get(i));

Also, BitSet primeBitSet can be declared just before the loop using it.

Continue

The way you use continue make things complicated for no obvious reasons.

            if (primeBitSet.get(j - 1) == true) {
                continue;
            } else {
                primeTable.add(j);
                for (int k = 2 * j; k <= n; k += j) {
                    primeBitSet.set(k - 1);
                }
            }

can be written :

            if (!primeBitSet.get(j - 1)) {
                primeTable.add(j);
                for (int k = 2 * j; k <= n; k += j) {
                    primeBitSet.set(k - 1);
                }
            }

New lines

The way you use new lines does not seem to make much sense. You should try to separate things into different "paragraphs".

After taking into accounts these comments about style, here's what the code would be like :

public class Main {

    public static void main(String[] args)throws IOException {
        Scanner s = new Scanner(System.in);
        int numberOfTests = s.nextInt();

        for (int q = 0; q < numberOfTests; q++) {
            int l = s.nextInt();
            int n = s.nextInt();
            if (l==1)l++;
            ArrayList <Integer> primeTable = new ArrayList <Integer>((int) Math.sqrt(n));
            if (l < 1 || n < 1 || l > n || n > 1000000000) {
                System.exit(0);
            }

            //classic sieve from 1 to sqrt(n)
            BitSet primeBitSet = new BitSet((int) (Math.sqrt(n) + 1));
            for (int j = 2; j * j <= n; j++) {
                if (!primeBitSet.get(j - 1)) {
                    primeTable.add(j);
                    for (int k = 2 * j; k <= n; k += j) {
                        primeBitSet.set(k - 1);
                    }
                }
            }

            //sieve in the expected range using primes generated by the classic sieve
            BitSet primesInRangeBitSet = new BitSet(n - l + 1);
            for (int i = 0; i < primeTable.size(); ++i) {
                int pomoc = (l / ((int) primeTable.get(i))) * ((int) primeTable.get(i));
                for (int j = pomoc; j <= n; j = j + (int) primeTable.get(i)) {
                    if (j >= l && !primesInRangeBitSet.get(j - l)) {
                        primesInRangeBitSet.set(j - l);
                    }
                }
            }

            //outputting the primes
            for (int k = 0; k < primeTable.size(); k++){
                if((int)primeTable.get(k)>= l &&(int) primeTable.get(k) <= n)
                    System.out.println(primeTable.get(k));
            }
            for (int k = 0; k < primesInRangeBitSet.size(); k++) {
                if (primesInRangeBitSet.get(k) == false && k <= n - l ) {
                    System.out.println((k + l));
                }
            }
            System.out.println("");
        }
    }
}

Algorithm

Now, this being said, there is a much bigger issue here. If tou want to implement Sieve of Eratosthene properly, everything you need is just a BitSet of size 1.
If you really want to (but I don't really see the point), you can add primes numbers in a list as you detect them. If you just need to print them, you can print them as you detect them. In any case, I guess that from a performance point of view, it is not such a big deal to reiterate over the whole BitSet looking from prime numbers.


Adding 0 to a balance on an object How can I refactor this into one line? Is that possible, even with ES6 syntax?

export const zeroBalance = (coin) => {
    coin.balance = 0;
    return coin;
};

How can I refactor this into one line? Is that possible, even with ES6 syntax?

export const zeroBalance = (coin) => {
  coin.balance = 0;
  return coin;
};

Yes, that's easily possible. Newlines aren't required in ECMAScript:

export const zeroBalance = coin => { coin.balance = 0; return coin; };

Ruby has an implementation of the K Combinator as Object#tap for exactly that use case. ECMAScript doesn't, but it's easy to implement:

 function k(obj, f) { f(obj); return obj };

This would allow you to rewrite your code like this:

export const zeroBalance = coin => k(coin, c => c.balance = 0);


Exception handling with expression lambda in Presenter class Following is a code snippet from MVP Win Forms application.

My DAL doesn't handle exceptions and it will be propagated up to the calling method in the presenter classes where the exception will be handled.

I'm using a single handler called ExecutAction(Action action) so I'm catching exceptions in one place rather than repeating it in every method.

At the moment, I'm not logging errors just alert the user for an action, trying to keep the system alive.

When showing messages to users, Presenters will use static class MessagingService's ShowErrorMessage(). So that I can customize all massage boxes in one place.

    class BankAccountPresenter : BasePresenter
    {
        IBankAccount _Model;
        IBankAccountView _View;
        IDataService _DataService;

        public BankAccountPresenter(IBankAccount model, IBankAccountView view, IDataService dataService)
        {
            _DataService= dataService;
            _Model = model;
            _View = view;

            this.SetViewPropertiesFromModel(_Model, _View);

            this.WireUpViewEvents();
        }

        private void WireUpViewEvents()
        {
            this._view.Search += new EventHandler(_view_Search);
        }

        private void _view_Search(object sender, EventArgs e)
        {
            this.Search();
        }

        private void Search()
        {
            ExecutAction(() =>
            {
                var info = _DataService.GetByACNo(_model.AccountNumber);

                if (info != null)
                {
                    _Model = info ;
                    this.SetViewPropertiesFromModel(_Model, _View);
                }
                else
                {
                    MessageBox.Show ("Bank account not found");
                }
            });
        }

        private void ExecutAction(Action action)
        {
            try
            {
                action();
            }

            catch (NullReferenceException e) { MessagingService.ShowErrorMessage(e.Message); }
            catch (System.Data.SqlTypes.SqlTypeException e) { MessagingService.ShowErrorMessage(e.Message); }
            catch (System.Data.SqlClient.SqlException e) { MessagingService.ShowErrorMessage(e.Message); }
        }
    }

Could you please review the above code and let me know whether this approach is good?

Also could you show me a better way to handle showing user messages than using a static?

Does use of lambda statements in every method (ExecutAction(() =>) degrade code readability? or is it a good practice keep things in one place in this way?

To avoid a discussion in comments I'll go ahead and make my comments an answer.

I think this a much better than your original way. Instead of catching Exception in your DAL, you're allowing all exceptions to propagate up until you can properly handle them. Catching Exception can be a bad thing and is generally considered a bad practice. The problem is that whenever a critical exception such as OutOfMemoryException is thrown, you swallow it and throw a new exception with a message of "Bank account is empty" if I remember correctly. It's just not right because you were hiding what actually went wrong. Your stacktrace would show the exception being thrown from the wrong place and it wouldn't have a meaningful message.

But that was the past and this is the present. You're handling the exceptions where they should be handled and you're not catching Exception. Those were my only real concerns.

The only thing I don't really like (note it's not actually a problem with your example) is that you handle all exceptions in one method. If one method in your presenter does file IO and another does sql stuff - you'd have to include catches for both types of exceptions in one place. What if your presenter is big and can possibly catch dozens of different exceptions? Is this a problem for you? I don't think so because all you're really doing is displaying the exception message through your MessagingService. I would only see it as a problem if you had to uniquely react to exceptions.


Determining if a palindrome exists in a linked list using recursion Summary of code:

This code recurses through half of the LinkedList, "adding" the left half node's to the call stack. Then, once at the middle, the code returns the first node of the right side (or more accurately, it assigns it to a variable, "right_node"). As the call stack collapses, the code compares the left nodes with the right nodes, and the right node is assigned to right node's next node.

The full code and updates are available here.

Specific questions:

I have a recursive function with two base cases, and both involve returning True, but each have to do a different step beforehand. Is there a better way to write out this if else statement structure?

This is the code snippet that features the if/else structure:

def recurse(head, length, node):
    if length == 1:  # odd number of nodes
        node[0] = head.next
    if length == 0:  # even number of nodes
        node[0] = head
    if length == 0 or length == 1:
        return True

    recurse(head.next, length/2, node)

Generally, when you end up with an expression like this, it is a sign that something is not right, and something needs rethinking or refactoring.

I don't see the point of node here - since you always write to node[0] -  and if you would eliminate it, that is, use it as the return value, your function would be a lot simpler:

def recurse(head, length):
    if length == 1:  # odd number of nodes
        return head.next
    elif length == 0:  # even number of nodes
        return head

    return recurse(head.next, length//2)

or, on one line

def recurse(head, length):
    return head if length==0 else head.next if length==1 else recurse(head.next, length//2)

BUT - 1//2 == 0, so both of these get a lot easier if you realize you have only one base case:

def recurse(head, length):
    if length == 0:
        return head

    return recurse(head.next, length//2)

or, on one line:

def recurse(head, length):
    return head if length==0 else recurse(head.next, length//2)

Of course, this might be better not to recurse:

def nonrecurse(head, length):
    while (length > 0):
        head = head.next
        length//=2

    return head


Templated Game Resource Manager After scanning some of the questions here about resource managers I've made some changes, and while it is much improved I still feel it's not quite up to snuff.

It tried to make it work for all data types that inherit from resource and load the resource automatically if not yet available.

Resource.h

class Resource {
public:
    Resource() {};
    virtual ~Resource() {};

    virtual void load(const std::string& path) = 0;
    virtual void unload() = 0;

protected:
    unsigned mResourceId;
    std::string mResourcePath;

};

// Custom destructor for smart pointer
struct ResourceFree {
    void operator()(Resource* resource) {
        resource->unload();
        delete resource;
    }
};

class Texture : public Resource {
public:
    int width;
    int height;
    int channels;
    unsigned char* data;

    void load(const std::string& path);
    void unload();
};

And ResourceManager.h

class ResourceManager {
public:
    template <typename T>
    T& load(const std::string& name) {
        static_assert(std::is_base_of<Resource, T>::value, "T must inherit from Resource");

        auto found = resources.find(name);
        if (found == resources.end()) {
            T* resource_instance = new T;
            Resource* resource_base = dynamic_cast<Resource*>(resource_instance);
            resource_base->load(name);
            resources.emplace(name, resource_base);

            return *resource_instance;
        } else {
            return *dynamic_cast<T*>(found->second.get());
        }
    }

    void unload(const std::string& path) {
        resources.erase(path);
    }
private:
    std::unordered_map<std::string, std::unique_ptr<Resource, ResourceFree>> resources;
};

I'm doing a lot of dynamic casting, but I don't see how to do it differently. Maybe there are some C++17 things that make this a little nicer? Or perhaps I'm going at it the wrong way

Small nitpicks

AFAICT Resource::mResourceId is unused, and Resource::mResourcePath might not be necessary (in worst case, it could be queried from the ResourceManager).
For better portability, make the type of Texture::data uint8_t, as char might not be 8 bits wide on all platforms.
Inconsistent naming: sometimes it's name, sometimes it's path. Choose one and stick with it!
Also, hopefully all paths follow the same convention (e.g. no backslashes under windows, no superfluous slashes, etc.), otherwise a resource with the same physical location might be loaded for multiple different paths. Maybe introduce a way to make paths compliant to a convention (e.g. a method make_unique_path or similar).

Implementation

If a resource path is already loaded as another resource type, ResourceManager::load returns a dangling (null) reference. Maybe throw an exception instead?

Design

Are there any actual cases where Resource::unload() actually has to do anything (e.g. write the resource back)? Usually resources are read-only, so this member function might not be needed.
I would make it the Resource constructors responsibility to load the resource (and the destructors responsibility to unload it if necessary). This way, there is never an "unloaded/not yet loaded" state of a resource that has to be checked everywhere.
ResourceManager has a communication issue if the same resource is required by 2 or more clients: Who does the cleanup (who gets to call ResourceManager::unload(const std::string&))? If done to early, every other client has a dangling reference. If not done at all, the resource will only be cleaned up when the ResourceManager is destructed - which uses memory unnecessarily in the meantime. (Careful: calling virtual functions from constructor/destructor is a bad idea!)
Why store a std::unique_ptr in ResourceManager::resources if the resource is going to be shared anyways? This is the usage std::shared_ptr and std::weak_ptr were made for!

Example:
#include <memory>
#include <unordered_map>

class Resource {
public:
    Resource(const std::string&) {}
    virtual ~Resource() {}
};

class Texture : public Resource {
public:
    int width;
    int height;
    int channels;
    uint8_t* data;

    Texture(const std::string& path) : Resource(path) {
        load(path);
    }

    virtual ~Texture() {
        // if needed for whatever reason
        unload();
    }
protected:
    void load(const std::string& path) {
        // loading logic in here
    }

    void unload() {
        // unloading logic in here
    }
};

class ResourceManager {
public:
    template<typename T>
    std::shared_ptr<T> load(const std::string& path) {
        static_assert(std::is_base_of<Resource, T>::value, "T must inherit from Resource");

        auto res = resources[path].lock();
        if(!res) {
            // assuming constructor loads resource
            resources[path] = res = std::make_shared<T>(path);
        }

        auto return_value = std::dynamic_pointer_cast<T>(res);
        if(!return_value) {
            throw std::runtime_error(std::string("Resource '") + path + "' is already loaded as another type");
        }
        return return_value;
    }
private:
    std::unordered_map<std::string, std::weak_ptr<Resource>> resources;
};

Some benefits of this design:

Resource cleanup is handled by std::shared_ptr.
Resources are unloaded as soon as they aren't needed anymore.
No dangling references!
Additional error handling if resource with the same name as another resource type is already loaded
ResourceFree isn't needed anymore, because the usual destructor already does all the work.

Other considerations

path gets passed around a lot. While I get that the high-level caller would uses that one, it might be more fitting to pass a stream (or maybe file handle / pointer to a byte array / a gsl::span) to the resource constructor. This way, the resource doesn't have to care whether it's loaded from disk, or from a zip file, or from a network stream, or from (somewhere else). However, this would actually require an ID to uniquely identify a resource (how would a path refer to a resource transmitted over the net?) - which by itself isn't a bad idea if used correctly.


Saves Individual Worksheets as Workbooks in destination folder This code takes, on average, 6 minutes to fully run with the workbook I am using. The workbook currently has 148 worksheets and will grow consistently with time.

I am curious if there is a way to edit the code from doing the task one worksheet at a time; and, instead complete the task all in one moment. In my mind, the code would run for a much shorter time and then populate all the worksheets as separate workbooks in the file destination at the same time. The worksheets would also not "pop-up" before saving themselves in the destination folder. Much like stopping screen updating and automatic calculations making a world of difference - except that did not work with this code.

Sub SaveFilesInFolder()
'
'Macro for saving each worksheet as a workbook in a destination folder
'

'
Dim sh As Worksheet
Dim wb As Workbook

For Each sh In Worksheets
    SheetName = sh.Name
    sh.Copy

        With ActiveWorkbook
            .SaveAs FileName:="FolderDestination" & SheetName
            .Close SaveChanges:=True
        End With

    Next sh

End Sub

Any suggestions are appreciated.

What you're looking for is to turn off screenupdating, turn off alerts and auto-save and close.

    Application.DisplayAlerts = False
    ThisWorkbook.Save
    ThisWorkbook.Close False

So, like this -

Option Explicit

Sub SplitSheetsToBooks()
    Application.ScreenUpdating = False
    Application.DisplayAlerts = False
    Const PATH_TO As String = "C:\TEMP\"
    Const EXCEL_EXTENTION As String = ".xlsx"

    Dim index As Long
    Dim numberOfSheets As Long
    Dim targetSheet As Worksheet
    numberOfSheets = ThisWorkbook.Sheets.Count

    For index = 1 To numberOfSheets
        Set targetSheet = ThisWorkbook.Sheets(index)
        targetSheet.Copy
        ActiveWorkbook.SaveAs PATH_TO & targetSheet.Name & EXCEL_EXTENTION
        ActiveWorkbook.Close False
    Next
    Application.DisplayAlerts = True
    Application.ScreenUpdating = True
End Sub


Function that returns words in string not exceeding max number of characters I'm trying to create a robust function that receives a string -- typically a paragraph or two -- and the max number of characters in the output. One condition is to make sure that we return full words and never cut one in half.

Here's the code I have and I feel it needs to be improved. This is normal ES2015/ES6 with no additional libraries.

export const shortenTextToSpecifiedNumberOfCharacters = (input, numberOfCharacters) => {

    if (!input) return "";
    else if (input.length <= numberOfCharacters) return input;

    for (var i = numberOfCharacters; i > 0; i--) {
        if (input[i] == ' ') {
            return input.slice(0, i) + ' ...';
        }
    }

}

You code can end up falling out of the loop and implicitly returning undefined if there are no spaces within the first numberOfCharacters characters in the input string.  You might want to add an explicit return statement at the end of the function to handle that case.

Also, in your loop you only test for spaces, not for newlines, tabs or other whitespace characters.  You could simply add tests for those into your if statement, like this:

if (input[i] == ' ' || input[i] == '\t' || input[i] == '\r' || input[i] == '\n') {

but it would be shorter and cleaner to use either indexOf():

if (" \t\r\n".indexOf(input[i]) >= 0) {

or a regexp test:

if (/\s/.test(input[i])) {

You may also wish to ensure that input[i-1] is not a whitespace character.  You could do it like this:

const spaces = " \t\r\n";
for (var i = numberOfCharacters; i > 1; i--) {
    if (spaces.indexOf(input[i]) >= 0 && spaces.indexOf(input[i-1]) < 0) {
        return input.slice(0, i) + ' ...';
    }
}

or, using a regexp, like this:

for (var i = numberOfCharacters; i > 1; i--) {
    if (/\S\s/.test(input.slice(i-1, i+1))) {
        return input.slice(0, i) + ' ...';
    }
}

Also note that the output of your function may be up to 4 characters (the length of the " ..." suffix) longer than the specified length limit.  If that's not what you want, you may wish to change your for loop to:

for (var i = numberOfCharacters - 4; i > 1; i--) {


Criticize my first attempt at using the itertools module Here's the exercise:

  Given a string of X's and O's and a maximum number of swaps, determine
  the longest possible sequence of X's after swaps.

Here's my solution using itertools.

import itertools as iter

def swaps_gen(s):
    '''This generator yields all the possible outcome of applying one swap
    to the input string'''
    X_positions = [i[0] for i in enumerate(s) if i[1] == "X"]
    O_positions = [i[0] for i in enumerate(s) if i[1] == "O"]

    for x in X_positions:
        tmp_x = X_positions[:]
        tmp_o = O_positions[:]

        tmp_x.remove(x)
        tmp_o.append(x)

        for o in O_positions:
            ttmp_x = tmp_x[:]
            ttmp_o = tmp_o[:]

            ttmp_o.remove(o)
            ttmp_x.append(o)

            s = []
            for i in ttmp_x: s.insert(i, "X")
            for i in ttmp_o: s.insert(i, "O")
            yield "".join(s)

def flatten(l):
    '''flatten one level. [[a], [b, c]] --> [a, b, c]'''
    return iter.chain.from_iterable(l)

def max_x_after_swap(s, n):
    '''repeat the generate swap_gen n times.
    I used a set() to avoid duplicates.
    Then returns the highest number of consecutive "X"s it finds.'''
    outcomes = [s]
    while n > 0:
        outcomes = set(flatten([list(swaps_gen(x)) for x in outcomes]))
        n -= 1
    return max(map(nbr_of_consecutive_x, outcomes))

def nbr_of_consecutive_x(s):
    '''return the number of consecutive "X" '''
    return max([len(list(v)) for k, v in iter.groupby(s) if k == "X"])

def main():
    input = "XXOXOXOXO"
    swaps_nbr = 2
    print(max_x_after_swap(input, swaps_nbr))

main()

Any criticism is welcome. It'd be great if you could point out any anti-pattern I may present, or show me better (by better I mean more pythonic) way to do things. I am more interested in improving my Python style than improving my algorithm.

I used Python3.1 to run the above.

import itertools as iter

I dislike abbrevations like this. Itertools is widely used within the python community, and it only hinders code readability by using iter instead of itertools

def swaps_gen(s):
    '''This generator yields all the possible outcome of applying one swap
    to the input string'''
    X_positions = [i[0] for i in enumerate(s) if i[1] == "X"]
    O_positions = [i[0] for i in enumerate(s) if i[1] == "O"]

PEP 8 recommends lowercase_with_underscores for local variable names. Your comprehensions would be more clearly written as

X_positions = [index for index, value in enumerate(s) if value == 'X']

Because you do it twice, I'd consider writing a function

    for x in X_positions:
        tmp_x = X_positions[:]
        tmp_o = O_positions[:]

I despise uses of tmp in variable names. All variables are temporary.

        tmp_x.remove(x)
        tmp_o.append(x)

That's going to be expensive. The remove has to move the whole list back to make it fit.

        for o in O_positions:
            ttmp_x = tmp_x[:]
            ttmp_o = tmp_o[:]

Especially egregious is using tmp again with an extra t.

            ttmp_o.remove(o)
            ttmp_x.append(o)

            s = []

I wouldn't reuse s from before

            for i in ttmp_x: s.insert(i, "X")
            for i in ttmp_o: s.insert(i, "O")

Inserts may be expensive. It won't be so bad here because your inserts will be in mostly sorted order. But the one unsorted element in each list will cost you somewhat.

            yield "".join(s)

Your whole approach here is a bit awkward. I'd suggest something like

for x_index in x_positions:
    for y_index in y_positions:
         swapped_list = list(s) # copy string into list, so I can modify it
         swapped_list[x_index], swapped_list[y_index] = swapped_list[y_index],swapped_list[x_index] # swap elements
         yield ''.join(swapped_list) # back to list

Shorter and probably more efficient.

def flatten(l):
    '''flatten one level. [[a], [b, c]] --> [a, b, c]'''
    return iter.chain.from_iterable(l)

def max_x_after_swap(s, n):
    '''repeat the generate swap_gen n times.
    I used a set() to avoid duplicates.
    Then returns the highest number of consecutive "X"s it finds.'''

PEP 257 recommends using """ not ''' for docstrings. Also, the closing ''' should be on a line by itself

    outcomes = [s]

I'd make this a set, just for consistency with the next line

    while n > 0:

Why a while loop instead of a for loop?

        outcomes = set(flatten([list(swaps_gen(x)) for x in outcomes]))

I'd use outcomes = set(flatten(map(swaps_gen, outcomes))) There is no reason to use list here, as the generator will flatten just fine.

        n -= 1
    return max(map(nbr_of_consecutive_x, outcomes))

def nbr_of_consecutive_x(s):

I dislike abbreviation like nbr, I'd call this count_of_consecutive_x

    '''return the number of consecutive "X" '''
    return max([len(list(v)) for k, v in iter.groupby(s) if k == "X"])

You don't need the [], because max will take a generator expression just fine

def main():
    input = "XXOXOXOXO"
    swaps_nbr = 2
    print(max_x_after_swap(input, swaps_nbr))

main()


Angular JS Switcharoo multiple select module I have created a script for going a multiple select option in Angular without using a select box that you have to hold control in order to select multiple items.

Demo: http://jsfiddle.net/qwertynl/xfw9f/

Code on GitHub

Here is my code:

Javascript:

var switcharoo = angular.module('switcharoo', []).directive('multiSelect', function(){
    return {
        restrict: 'E',
        scope: {
            items: '=',
            default: '=',
            leftTitle: '@',
            rightTitle: '@'
        },
        templateUrl: "switcharoo.html",
        link: function(scope)   {
            scope.switchItem = function(item)   {
                var index = scope.default.indexOf(item);
                if(index == -1) {
                    //add it in
                    scope.default.push(item);
                }
                else    {
                    //remove it
                    scope.default.splice(index, 1);
                }
            }
        }
    };
})

switcharoo.directive('switchitem', function() {
    return {
        restrict: 'E',
        scope: {
            value: '='
        },
        template: '
{{value}}
'
    };
});

HTML Template:

            {{leftTitle}}
            {{rightTitle}}

                    <switchitem ng-repeat="(key, value) in items" ng-if="default.indexOf(key) == -1" value="value" ng-click="switchItem(key)"></switchitem>

                    <switchitem ng-repeat="(key, value) in items" ng-if="default.indexOf(key) > -1" value="value" ng-click="switchItem(key)"></switchitem>

Is there anything I can do to make my code slicker?

I really like your code, I only have 2 minor observations:

I understand the part where default has the default items. I am not sure that after initialization you should keep referring to the default variable ( from a naming perspective ), maybe call it selection instead of default ?
Related to the previous point, ng-if="default.indexOf(key) == -1" and ng-if="default.indexOf(key) > -1" only make sense during initialization. In my mind, it would be ideal to provide a isSelected( key ) that you can use for the ng-if. Not to mention that it will remove a > out of your HTML attribute and look better in Markup ;)


Recursively generating the look-and-say sequence I have a pattern where every number is counted by the number of repetitions. A new resulting number is formed by adding that repetition to the front of that value. The resulting pattern would look like this.

1
11
21
1211
111221

so:

indexnumbercomment
0      1
 one +  1 ->11
1      11       two + 1 -> 21
2      21       one + 2 + one + 1 -> 1211

etc...

I was given a number 4, and I needed to produce the resulting number 1211.

My approach was to use a recursive function that used every subsequent iteration starting from 0 to evaluate every resulting value until I reached my target iteration.

Runtime Complexity: I decremented the number recursively O(N) as I carried the resulting evaluation result.

At every recursive step, I produced every result by walking the resulting values, counting the repetitions as I went along. I stored every key-value pairs along an Index represented by a synecdochic key in a line of contiguous repetition. I tallied the repetitions until I had a key-value pair at the intersection of every index.

I then assembled the value by swapping the key values pairs at every index into a new object array which I used to repeat the process. O(C)

I then used reduce to format the results and repeat the process O(F)

O(N *C +F) = O(N^2)

space complexity is O(N^2) since the amount of space needed to process doubles on every resulting iteration.

Are my Time and Space analysis correct, did I miss a much simpler approach where I might have been able to take advantage of Fibonacci, XOR or some other simpler mathematical approach?  and could this have been done better programmatically by using some other type of data structure, or could it have been written in some other concise way?

function countStay(num) {
  if (num === 1) {
    return 1
  }

  return drill(num, [1])
}

function drill(exitCount, result) {
  const map = {
      0: {}
    },
    arr = [];
  let count = 0,
    index = 0;
  result = result.reduce((acc, a, i, aa) => {
    if (aa[i - 1] && aa[i] && a !== aa[i - 1]) {
      index += 1
      count = 0
      map[index] = {
        [a]: count
      }
    }
    if (((map || {})[index] || {})[a] && count !== 0) {
      map[index][a] = count += 1
      arr[index] = [count, a]
    }
    if (!((map || {})[index] || {})[a] && count === 0) {
      count += 1
      map[index] = {
        [a]: count
      }
      arr[index] = [count, a]
    }
    return acc = {
      arr
    }
  }, {})

  result = join(result)
  exitCount--
  if (exitCount <= 1) {
    return result.join('')
  }
  return drill(exitCount, result)
}

function join(result) {
  return result = result.arr.reduce((acum, a) => acum.concat(a), [])
}
console.log(countStay(4))

It seems like you tried too hard to avoid all loops by limiting yourself to .reduce() and recursion.  While JavaScript supports functional programming to some extent, sticking to pure FP everywhere will likely lead to clunky code.  A better strategy would be to define some functions that look pure from the outside, but might be implemented in an imperative style.  (Your code isn't pure FP anyway: the index += 1 statements perform mutation.)

I don't think that countStay and drill are well designed functions.  It's not clear to me what each of them is supposed to accomplish.  countStay seems to be a trivial wrapper around drill.  At first glance, all I can see is that drill contains a lot of code, and that it calls itself recursively.  In particular, it's hard to read because the variable names are cryptic.  map is obviously a map, and arr is obviously an array, but what are their purpose?

Suggested solution

I would define three functions, each of which has a clear purpose and is easily testable.  Instead of recursion, I've used iterateN, which is inspired by Haskell.  Then, it's easy to compose them to obtain the result.  This solution is simpler and more readable than yours.

/**
 * Split an array of items into runs of identical values;
 * yield [item, runLength] for each run.
 *
 * runLengthEncode('bookkeeper'.split('')) =>
 * ['b', 1], ['o', 2], ['k', 2], ['e', 2], ['p', 1], ['e', 1], ['r', 1]
 */
const runLengthEncode = function*(items) {
    if (items.length) {
        let groupStart = 0, groupItem = items[0];
        for (let i = 1; i < items.length; i++) {
            if (items[i] != groupItem) {
                yield [groupItem, i - groupStart];
                groupStart = i;
                groupItem = items[i];
            }
        }
        yield [groupItem, items.length - groupStart];
    }
};

/**
 * Given a number (as an integer or string), produce its successor
 * in the look-and-say sequence (as a string).
 *
 * nextLookAndSay('1211') => '111221'
 */
const nextLookAndSay = (n) => {
    let accum = [];
    for (let group of runLengthEncode(n.toString().split(''))) {
        accum.push(group[1], group[0]);
    }
    return accum.join('');
};

/**
 * The result of f(f(f ... f(init))), where the function f is applied n times
 * to the initial value.
 *
 * iterateN(2, (s) => s + 'o', 'b') => 'boo'
 */
const iterateN = (n, f, init) => {
    while (n--) { init = f(init); }
    return init;
};

console.log(iterateN(3, nextLookAndSay, '1'));


Is wrapping the service layer worthwhile? I have been trying to design a good data layer that will eventually be generated. I am wondering if I have missed anything. The basic architecture contains a service class that handles connecting and eventually transactions. I wrap the IDbConnection in the service class to make sure it gets disposed properly so that the user will not need to worry about that. Are there any drawbacks to wrapping the IDbConnection in a using statement?

Sample code:

public abstract class SqlService
{
    private readonly string _connectionString;

    protected Service(string connectionString)
    {
        _connectionString = connectionString;
    }

    private IDbConnection CreateConnection()
    {
        var connection = new SqlConnection(_connectionString);
        connection.Open();

        return connection;
    }

    protected T Execute<IDbConnection, T>(Func<IDbConnection, T> query)
    {
         using(var connection = CreateConnection())
         {
             return query(connection);
         }
    }

    protected T ExecuteTransaction<T>(Func<IDbConnection, IDbTransaction, T> query, IsolationLevel level)
    {
        return Execute(c =>
                            {
                                using (var transaction = c.BeginTransaction(level))
                                {
                                    try
                                    {
                                        var result = query(c, transaction);
                                        transaction.Commit();

                                        return result;
                                    }
                                    catch (SqlException)
                                    {
                                        transaction.Rollback();

                                        throw;
                                    }
                                }
                            });
    }
}

public interface ICanGetById<TEntity, TKey>
{
    TEntity GetById(TKey id)
}

public interface IOrderService : ICanGetById<OrderDTO, int>
{
}

public sealed partial class OrderService : SqlService, IOrderService
{
    public OrderService (string connectionString) : base(connectionString) { }

    public OrderDTO GetById(int id)
    {
        var dynamicParameters = new DynamicParameters();
        dynamicParameters.Add("@Id", id);

        return Execute(c => c.Query<OrderDTO>(
                                            "usp_OrderSelect",
                                            dynamicParameters,
                                            commandType: CommandType.StoredProcedure));
    }
}

The drawback is that you are limiting the functionality of your service to perform one query per connection and you have no "batch mode"

But maybe you can implement a query batching method!

For this reason, the user of the database service perhaps should have the power to open and close the connection. Your SQLService could expose Open, Close, and Dispose.


Test if a key-value pair exists in nested object in Typescript I wrote a function to check if a nested object contains the key-value pair matched as parameter as following.
eq:
const obj ={
   "a":{
      "b":{
         "c1":["1","2"],
         "c2":"3"
      },
      "b2":"values",
      "b3": {"c5": "v"},
   },
   "a2":["v1","v2"],
   "a3":"v1"
}
checkNestedObjectByKeyValue(obj, "a3", "v1") // true
checkNestedObjectByKeyValue(obj, "a3", "v2") // false
checkNestedObjectByKeyValue(obj, "a2", ["v1","v2"]) // true
checkNestedObjectByKeyValue(obj, "a3", "v1") // true
checkNestedObjectByKeyValue(obj, "b3", {"c5": "v"}) // true

function checkNestedObjectByKeyValue(
  obj: Record<string, any>,
  objKey: string,
  objValue: string | Array<string>
): boolean {

  if (typeof obj !== 'object') return false;
  if (obj.hasOwnProperty(objKey)) {

    return JSON.stringify(obj[objKey]) === JSON.stringify(objValue)
  }
  for (const value of Object.values(obj)) {
    if (checkNestedObjectByKeyValue(value, objKey, objValue)) return true
  }
  return false
}

Based on your description of the function, I would expect this test to return true, but your implementation returns false.  So, either your description is unclear, or your implementation is buggy.
const obj ={
   "a":{
      "b":{
         "a3":"v2",
         "c1":["1","2"],
         "c2":"3"
      },
      "b2":"values",
      "b3": {"c5": "v"},
   },
   "a2":["v1","v2"],
   "a3":"v1"
}
checkNestedObjectByKeyValue(obj, "a3", "v2") // false, but I would expect true


Project Euler Problem 18 (Haskell) - Maximum path sum I I've implemented the solution to Project Euler Problem 18 in Haskell and I'd like to get some thoughts on my implementation.  Its the dynamic programming solution, working from the bottom up.

Basically, given a set of numbers in a triangular shape, the path whose numbers add up to the greatest possible sum from the top to the bottom must be found.

I believe its correct, but with Project Euler being partially down, I couldn't plug-in the answer so I had to rely on Google.  I get the following result: 1074.

My code reads in a text file that contains the problem data and looks like this:

75
95 64
17 47 82
18 35 87 10
20 04 82 47 65
19 01 23 75 03 34
88 02 77 73 07 63 67
99 65 04 28 06 16 70 92
41 41 26 56 83 40 80 70 33
41 48 72 33 47 32 37 16 94 29
53 71 44 65 25 43 91 52 97 51 14
70 11 33 28 77 73 17 78 39 68 17 57
91 71 52 38 17 14 91 43 58 50 27 29 48
63 66 04 68 89 53 67 30 73 16 69 87 40 31
04 62 98 27 23 09 70 98 73 93 38 53 60 04 23

Here is my full implementation:

-- Project Euler
-- Problem 18 (http://projecteuler.net/problem=18)
-- Nathan Jackson

import System.Environment
import System.IO

-- Read in a text file containing the problem data, run the algorithm and
-- print the result to the console.
main :: IO ()
main = do
    cmdargs <- getArgs
    withFile (head cmdargs) ReadMode (\handle -> do
        contents <- hGetContents handle
        let inputData = parse contents
        putStrLn $ show $ maxPathSum inputData)

-- Given a string whose format looks something like this:
--   1
--   2 3
--   4 5 6
--   7 8 9 0
-- Convert it to a list of lists that contains integers where each nested list
-- represents a line from the original string.
parse :: String -> [[Int]]
parse s  = map (map (read :: String -> Int)) (map words $ lines s)

-- Reduces a list of lists containing integers to its maximum path sum.
maxPathSum :: [[Int]] -> Int
maxPathSum [[x]] = x
maxPathSum xs =
    let maxPairs ys = zipWith max ys $ tail ys
        sx = reverse xs
    in maxPathSum $ ((init . init) xs) ++ [zipWith (+) (maxPairs (head sx)) $ sx !! 1]

I'll focus on maxPathSum as this is the core part of the algorithm.

The approach of zipWith max ys (tail ys) is correct and very nice, this is the essence of the recursive step.

But first, you're doing a lot of unnecessary computations at every step. In particular, you're reversing the argument list every time (reverse xs) and at the same time, you're computing its initial part (init . init $ xs), they're all O(n). If you instead start with an already reversed list, you can save all this effort, replacing O(n) init with O(1) tail, and O(n) ++ [...] with O(1) : ...:

maxPathSum2 :: [[Int]] -> Int
maxPathSum2 = mps . reverse
  where
    maxPairs ys = zipWith max ys $ tail ys
    mps [[x]] = x
    mps sx = mps $ (zipWith (+) (maxPairs (head sx)) (sx !! 1)) : (tail . tail) sx

Note that placing maxPairs out of mps also helps the readability as it's now clear nothing there depends on sx.

Next thing is that you're combining the intermediate results with the data structure you're processing. While conceptually it isn't incorrect, in this case it makes the algorithm harder to understand and more complex. So let's add a parameter to mps that will hold the intermediate path lengths (and initialize it with an infinite list of 0s):

maxPathSum3 :: [[Int]] -> Int
maxPathSum3 = mps (repeat 0) . reverse
  where
    maxPairs ys = zipWith max ys $ tail ys
    mps [r] [] = r
    mps rs (s:ss) = mps (zipWith (+) (maxPairs rs) s) ss

Not only this is much simpler;
notice that now we've just reinvented a fold over the list! We're consuming the list, passing the rest for further processing, and accumulating a result. Since we're consuming the elements from the start, this is a left fold. So let's rewrite it as

maxPathSum4 :: [[Int]] -> Int
maxPathSum4 = head . foldl f (repeat 0) . reverse
  where
    maxPairs ys = zipWith max ys $ tail ys
    f rs s = zipWith (+) (maxPairs rs) s

But left fold over a reversed list is just a right fold. So we get

maxPathSum5 :: [[Int]] -> Int
maxPathSum5 = head . foldr f (repeat 0)
  where
    maxPairs ys = zipWith max ys $ tail ys
    f s rs = zipWith (+) (maxPairs rs) s

And this is actually a very natural way how to express the solution: If we already know the longest paths in the part below a particular level (represented by rs and in recursive right-fold), we can compute the paths for this level as well.


Simple four-operation calculator in Lua I'm a beginner at Lua and programming in general and I just made a simple calculator and I was wondering where I could make improvements.

print("Choose numbers:")
print("Num1:")
local num1 = io.read()
print("num2:")
local num2 = io.read()
print("Choose operator: + / - / * / /")
local operator = io.read()
if operator == "+" then
   local add = num1 + num2
   print(add)
end
if operator == "-" then
   local subtract = num1 - num2
   print(subtract)
end
if operator == "*" then
   local multiply = num1 * num2
   print(multiply)
end
if operator == "/" then
   local devide = num1 / num2
   print(devide)
end
io.read()

Try some features, typically available from functional languages. For example, functions that is first class values.

local operators = {
    ["+"] = function(x,y) return x+y end,
    ["-"] = function(x,y) return x-y end,
    ["*"] = function(x,y) return x*y end,
    ["/"] = function(x,y) return x/y end
}

local function default()
    print "No such operator"
end

print("Choose numbers:")
print("Num1:")
local num1 = io.read()
print("num2:")
local num2 = io.read()
print("Choose operator: + / - / * / /")

local func = operators[io.read()] or default
print(func(num1,num2))

io.read()


Suggestions for improving error handling I am interested in finding out what is the correct way of implementing error handling for this situation in C#.

The system tries to do an operation. If the operation succeeded (returns a non-error code), the system logs in a log database that the operation was succesfull. If the operation returns a error code or something caused an exception before the operation, the system will log an error. However, if the an error is caught when logging, the system shouldn't log an error in the database.

I am using C#. The following code is what I have used until now, but I don't know if it is the best practice in this situation.

int response = -1;  //some error code
try
{
    //some code to prepare the operation - may cause exceptions
    response = DoOperation();
    //some code to clean after the operation - may cause exceptions
}
catch
{
    //error handling
}

try
{
    if (response > 0) //non-error code
        //log event in database
    else
        //log event error in database
}
catch
{
    //logging error handling
}

Do you have any suggestions for improving my code?

Note: The catch blocks include in the original code handling specific errors, I just used a general catch blocks for code simplicity in my question.

You could combine the try-catch blocks:

int response = -1;  //some error code
var logging = false;
try
{
    //some code to prepare the operation - may cause exceptions
    response = DoOperation();
    //some code to clean after the operation - may cause exceptions

    logging = true;
    if (response > 0) //non-error code
        //log event in database
    else
        //log event error in database
}
catch
{
    if(logging)
    //logging error handling
    else
    //error handling
}

Even better, if you know the logging could throw a particular type of exception that the other operations will not, then catch just that exception and handle it as a logging exception, and let all others trickle down to a general catch:

int response = -1;  //some error code

try
{
    //some code to prepare the operation - may cause exceptions
    response = DoOperation();
    //some code to clean after the operation - may cause exceptions

    if (response > 0) //non-error code
        //log event in database
    else
        //log event error in database
}
catch(LoggingSpecificException)
{
    //logging error handling
}
catch
{
    //error handling
}

As far as the general strategy, it doesn't look too bad. Getting a "return status code" from a method is more than a little outdated (try-throw-catch was designed to replace this style of method return), but there's a lot of "legacy code" out there, and some built-in methods still return values that indicate failure. The return value of DoOperation(), if any, should be its conceptual "product"; data produced by DoOperation as the result of a computation. If DoOperation, conceptually, doesn't make any such calculation, it would be better conceptually (and thus from an understandability standpoint) to return void and instead throw exceptions on errors. However, you'd have to throw and catch multiple types of exceptions, or keep track of multiple states instead of just "logging".

Also, I don't know if you simply omitted it for the operation, but I would think that knowledge of exactly what went wrong might be good to know, and so any general catch should be catch(Exception ex) or similar to allow use of exception data when handling the error.


Fast hamming distance function in Julia that returns a distance matrix I would like to use Julia to compute the hamming distance on a very large dataset.
I need to get back a distance matrix between rows in order to run further analysis on this matrix.
For my purposes, it is useful that the data are stored in a DataFrame type.
using DataFrames

The data looks something like this
a = [1 0 1 0 ; 1 1 1 1; 0 0 0 0; 0 0 0 0 ; 0 0 0 1]

df = convert(DataFrame, a);

nrows = size(df, 1)
ncols = size(df, 2)

I made a function in Julia
function hamjulia(df)

    nrows = size(df, 1)
    ncols = size(df, 2)
    m, n = nrows, nrows
    A = fill(0, (m, n))

    for i in 1:nrows
    for k in 1:nrows

        v = 0
        for j in 1:ncols
            if df[i,j] != df[k,j]
            v += 1
            end
        end
        A[i,k] = v
    end
    end
    return A
end

p = hamjulia(df)
p

My issue is that this code is super slow compared to some R packages. For instance, when I compared this function to the rdist package, rdist(df, metric = 'hamming'), R is faster.
How could I make this code really efficient? Especially that I would need to run it on a very large dataframe.
Thanks.

I have three areas of suggestions below. Here's a modified version of your code that implements these suggestions:
# For memory safety, restrict type to Matrix due to the use of @inbounds below.
function ham(df::Matrix)
    nrows, ncols = size(df)
    A = fill(0, ncols, ncols)
    for j in 1:ncols, i in j+1:ncols
        v = 0
        for k in 1:nrows
            v += @inbounds (df[k, i] != df[k, j])
        end
        A[i, j] = A[j, i] = v
    end
    return A
end

ham(df::DataFrame) = ham(convert(Matrix, df))

1. Write code for column-major arrays.
Julia is column-major (and packages like DataFrames.jl conform with this by storing data in columns), so when performance is important, you should iterate through the rows of a column. For example, when iterating through an array A[x, y, z], the leftmost index x should be changing the most rapidly. Looking at the innermost loop in your code,
for j in 1:ncols
    if df[i,j] != df[k,j]
    v += 1
    end
end

notice that it is iterating through the columns of a row instead (i.e., j is changing most rapidly in df[i,j]). To get a free performance boost, rewrite the code to exchange the roles of columns and rows. The data will also need to be transposed. (Note that R is also column-major.)
Julia documentation: memory order.

2. In performance-critical loops, annotate @inbounds where possible.
In the modified code I gave, the array access in the innermost loop is annotated with @inbounds to disable bounds checking. This gives a speedup in large part because it allows compiler to automatically vectorize the loop.
Only use @inbounds if you are certain that out-of-bounds access is impossible; in our case, we cannot guarantee this for custom types, so we restrict the input type to a Matrix (an alias for Array{T,2} where T) to ensure that df[k, i] and df[k, j] are always in bounds. Like so:
function ham(df::Matrix)
    [...]
    for k in 1:nrows
        v += @inbounds (df[k, i] != df[k, j])
    end
    [...]
end

If @inbounds is not used, I would not restrict the input type and would instead leave it generic.
Julia documentation: performance annotations.

3. Beware of type instability.
For performant code, functions should be type stable, meaning that given an input type, the output type can be inferred during compilation. Similarly, variables used within a function should be type stable.
As noted in its documentation, DataFrame columns are not type stable! That is, the type of a DataFrame does not contain information about the types of its columns; in fact, the DataFrame type is not parametric, so the type contains no information other than that it is a DataFrame. This can drastically degrade performance.
The @code_warntype macro is very helpful for diagnosing this.
In our case, assuming that all columns are of the same type, a simple workaround is to copy the columns into a Matrix before calling the function. (The time this takes is negligible compared to the rest of the function.) To do so, we can specialize ham for the DataFrame type:
ham(df::DataFrame) = ham(convert(Matrix, df))

Note that the compiler specializes code for types at function boundaries, so it is important that this conversion takes place outside the original ham function. In particular, don't put the conversion into the original function like so:
# Don't do this!
function ham(x)
    df = convert(Matrix, x)
    [rest of ham...]
end

because the type of x here might not contain sufficient information to infer the type of df. Such is the case for DataFrames.
Julia documentation: function barriers.

Simple benchmark
After implementing these suggestions, the speedup on calculating the distance matrix for 128 vectors of 256 integers each is by about 1000x: (note that the times are in microseconds vs milliseconds)
using BenchmarkTools
using DataFrames
M = rand(0:1, 256, 128);
df = DataFrame(M);
tdf = DataFrame(permutedims(M)); # transpose rows and columns

julia> @benchmark ham($df)
BenchmarkTools.Trial:
  memory estimate:  387.33 KiB
  allocs estimate:  133
  --------------
  minimum time:     493.841 μs (0.00% GC)
  median time:      537.689 μs (0.00% GC)
  mean time:        610.732 μs (2.74% GC)
  maximum time:     6.215 ms (81.69% GC)
  --------------
  samples:          8176
  evals/sample:     1

julia> @benchmark hamjulia($tdf)
BenchmarkTools.Trial:
  memory estimate:  128.08 KiB
  allocs estimate:  2
  --------------
  minimum time:     538.585 ms (0.00% GC)
  median time:      546.390 ms (0.00% GC)
  mean time:        551.220 ms (0.00% GC)
  maximum time:     567.238 ms (0.00% GC)
  --------------
  samples:          10
  evals/sample:     1

The additional memory allocations in ham are due to the conversion of the DataFrame to a Matrix.


Function to search for weekends left in month I have a function that returns true or false depending upon whether there are any weekend days left in the month.  Is there a better/cleaner way that I can write this?

function checkWeekendsLeft(month, year) {
  var iDaysInMonth   = new Date(year, month, 0).getDate();
  var iCurDayInMonth = new Date().getDate();
  var x              = new Date();
  var weekends       = [];

  if (iDaysInMonth >= iCurDayInMonth) {
    for(var i = iDaysInMonth; i >= iCurDayInMonth; i--) {
      var iDayofMonth = i;
      var date = new Date(year, month - 1, iDayofMonth);
      var iDayOfWeek = date.getDay();
      if(iDayOfWeek === 6 || iDayOfWeek === 0) {
        weekends.push(iDayOfWeek);
      }
    }
    if(weekends.length > 0) {
      return true;
    } else {
      return false;
    }
  }
}

You could use something like this :

function hasWeekendDaysLeft() {
  var today = new Date();
  var lastDayInMonth = new Date(today.getYear(), today.getMonth() + 1, 0).getDate();
  var numToday = today.getDate();

  return lastDayInMonth - numToday > 6 - today.getDay();
}


Php INSERT class function This is a INSERT function. It's working... I just pulled out the function from the class, atm to try it out. And improve. I think the code is a bit messy, And I am pretty sure I can improve the code, less code, for example.. Any ideas?

   $data['test'] = array('username' => 'john',
              'password' => 'hello',
              'userlevel' => '__d');

$table = 'users';

$numItems = count($data['test']);
$i = 0;

$sql = "INSERT INTO " . $table . "(". implode(", ", array_keys($data['test'])) .")";

$sql .= " VALUES (";

$i = 0;

foreach ($data['test'] as $value) {

    if ($i+1 == $numItems and $value == '__d') {
        $sql .= "" . 'NOW()' . ")";
    } else if ($i+1 == $numItems) {
        $sql .= "'" . $value . "')";
    } else if ($value == '__d') {
        $sql .= "" . 'NOW()' . ", ";
    } else {
        $sql .= "'" . $value . "', ";
    }

            $i++;

}

echo $sql;

This is what you have, cleaned a little

foreach ($data['test'] as $value) {

    if ($value == '__d') {
        $sql .= 'NOW()';
    } else {
        $sql .= "'" . $value . "'";
    }
    $i++; // add before the comparisson
    // make the comparrison at one place
    // if the number != total, then we need a comma.
    if($i != $numItems) $sql .= ',';
}
// always end with a close paren.
$sql .= ')';

But you are better off using implode and replace:

// implode using the quotes and commas.
$values = "'".implode("', '", array_values($values))."'";
// swap out __d for NOW
$sql .= str_replace("'__d'",'NOW()',$values);

Of course, you really need to make sure that you've cleaned all of those values with mysql_real_escape_string.


Refactor function that returns a comma Operator to pass jsHint Some background

This functions does some business logic before submitting a "add to cart" form

HTML

<form class="atcLoginRequired" method="get" action="/somePage">
    <input type="hidden" value="someVal" name="nextpage">
    <input name="ISODSCTX66WA_qty" type="text" value="1">
    <input name="ISODSCTX77WA_qty" type="text" value="1">
    <input type="Image" class="imageSubmit" src="ATCimage.jpg">
</p>

JavaScript

 var atcLoginRequired = $(".atcLoginRequired");
    if (!atcLoginRequired.length) return;
    atcLoginRequired.on("submit", function(){
        var jThis = $(this),
            nextPageInput = jThis.find("[name='nextpage']"),
            nextPageVal = nextPageInput.val(),
            skuValues = jThis.find("[name$='_qty']").map(function(){
                var jThat = $(this);
                return (jThat.remove(), jThat.attr("name") + "=" + jThat.val() + "&");
              // the return statement actually executes the first expression, and "returns" the second one..
            }).get().join("");
        $(".imageSubmit").remove(); // to avoid having the x and y passed
        skuValues = skuValues.substring(0, skuValues.length - 1);
        nextPageInput.val(nextPageVal + "" + skuValues);
    });

The question

What can I do to make this it pass jsHint?
What can I do to make this better in terms of readability, performance and extensibility ?

Simple answer:

var jThat = $(this).remove();
return jThat.attr("name") + "=" + jThat.val() + "&";

There's nothing wrong with calling .remove() right away; the element's still in memory so its attributes be still be accessed. As you said, the remove call is already executed in your code, before the rest.

Longer answer:
Don't write your own query string by concatenating stuff with "&" and "=". It's error-prone, a bother to deal with, and it also means you have to do the slice off the trailing "&" later (which, incidentally, you can do with just skuValues.slice(0, -1)).

jQuery has a $.param() function that's built for this sort of thing. Pass it an array of objects, each with name and a value property, and it'll make a properly encoded query string for you.

// no need to first store `$(".atcLoginRequired")` in a variable,
// and then check its length. If nothing was found, jQuery will
// just ignore the call to .on(). Same result as you checking, and
// and returning if length is zero.

$(".atcLoginRequired").on("submit", function (event) {
  var form          = $(this), // use a more descriptive name than "jThis"
      nextPageInput = form.find("[name='nextpage']"),
      skuValues     = form.find("[name$='_qty']").map(function () {
        var input = $(this).remove(); // again, more descriptive
        // return an object fit for jQuery's param() function
        return {
          name:  input.attr("name"),
          value: input.val()
        };
      }).get();

  // add the nextPage stuff, if there is any
  if( nextPageInput.val() ) {
    skuValues.push({
      name: "nextpage",
      value: nextPageInput.val()
    });
  }
  nextPageInput.val( $.param(skuValues) );
});

Overall, though, I'm wondering what the point is of stuffing a big query string into a hidden input. That's going against the grain. Why not send the inputs to the server as-is, and let it add a bunch of hidden inputs to the next page's form or something? Using an input to store other inputs seems like a kludge.

  What can I do to make this better in terms of readability, performance and extensibility ?

For readability, see above. Less code, more descriptive variable names. You can find better names than mine - I just went for simple ones.

Performance... well, there isn't a whole lot going on. It's not a complex algorithm. If you want performance, skip jQuery and go straight to the native DOM. Of course, that's a hassle to code, which is why jQuery is extremely useful.

Extensibility: In what way exactly? This is, I presume, not a library in itself. It's the end-goal code. So extensibility is sort of moot. But if you want to add more on-submit functionality (like, say, form validation), the first order of business is to not remove elements. If you do that, the next piece of code will just find an empty form. Validation would be sort of pointless if it comes after this code, as it's not going to find anything but a hidden input with a big query string as its value. And if you need to validate that input, you'll need to parse that string, just to get back to the individual values again.