Refactor the code which performs "cross-product", "reduce", "product of list" and "sum of list" I have come up with a sequence of steps to find the maximum product of y positive numbers which add up to x. But the program is highly inefficient.

Here's my code:

from itertools import product
from functools import reduce
from operator import mul

for i in range(int(input())):               # <-- number of pairs of input
    lgr = 1
    k = set()
    x, y = tuple(map(int, input().split())) #  <-- the input pairs
    p = product(range(1, x+1), repeat=y)    #  <-- gets the cross product
    for i in p:
        if sum(i) == x:                     #  <-- checks if sum is equal to x
            k.add(i)                        #  <-- adds it to a set() to remove duplicates
    for i in k:
        tmp = reduce(mul, i, 1)             #  <-- multiplies the items in the tuple to get the largest
        if tmp > lgr:
            lgr = tmp
    print(lgr)

But say for an input like 14 7 it just takes too long (around 20s). Is there a better way to do it?

Update 1:
I've managed to improve a bit by using combinations_with_replacement and set & list comprehension... but I believe it can be done better... any suggestions...?

from itertools import combinations_with_replacement
from functools import reduce
from operator import mul

for i in range(int(input())):
    lgr = 1
    x, y = tuple(map(int, input().split()))
    k = {i for i in combinations_with_replacement(range(1, x+1), y) if sum(i) == x}
    lgr = max([max(reduce(mul, i, 1), lgr) for i in k])
    print(lgr)

One immediate improvement is realizing that for e.g the input 14 7, all combinations like x x x x x x 9 and above can never sum to 14, because even for the minimal case x = 1, 9 + 6*1 = 15. So you can vastly reduce the number of combinations you need to check by using

k = {i for i in combinations_with_replacement(range(1, x - y + 2), y) if sum(i) == x}

For the input 14 7 this constitutes a difference of 77520 possible combinations down to 3432.

This adds a constraint only on the last number, but you could take this to its logical extreme and write code that always aborts the combinations once the sum has been reached.

One other thing you should do is use better names and use functions. I have no idea that k is a list of combinations of numbers, or what this means. Also, using i (and j, k, n, too, for that matter) for anything other than an integer is not helpful either.

I would at least rewrite your code to this:

from itertools import combinations_with_replacement
try:  # Python 3.8+
    from math import prod
except ImportError:
    from functools import reduce
    from operator import mul
    def prod(x):
        """Returns the product of all elements of `x`, similar to `sum`."""
        return reduce(mul, x, 1)

def maximum_product_with_sum(x, n):
    """Return the maximum product of all `n` positive numbers that sum to `x`."""
    r = range(1, x - n + 2)
    candidates = {candidate
                  for candidate in combinations_with_replacement(r, n)
                  if sum(candidate) == x}
    return max(map(prod, candidates), default=1)

if __name__ == "__main__":
    for _ in range(int(input())):
        x, n = tuple(map(int, input().split()))
        print(maximum_product_with_sum(x, n))

Note that I also added some short docstrings to describe what each function does and simplified getting the maximum, your inner call to max was not needed. The optional keyword argument argument default of max is the default value if the iterable is empty.

I also added a if __name__ == "__main__": guard to allow importing from this script from another script without the code being run.

Here is the performance measured against your second function (the first one is way too slow) once for the input x 7 and once for 14 n:

You can see that this code is not only faster in general, it also becomes much faster if n > x / 2.


Using session wrapper for accessing session data When users log in to our website, we store some information in a session to retrieve it across the site. We created a few wrappers to help with data retrieval/storage to lessen duplication of code.

I'm trying to redesign it as I don't like how the UserModel is written. I believe it's dangerous with the static properties, but so far it's been working.

However...

While we haven't noticed any issues, recently someone use something very similar to our code (slightly modified) for another project and had about 20+ people log in at the same time. Some users saw other user's information instead of their own. This brought a huge concern on the original code (below) and would like to get any feedback on how to make it "better" or avoid the same issue they ran into (if it even will happen).

I searched/read around Google/Stack Overflow about the possible issue being at our static method wrappers.

UserModel.cs

public class User {
    private const Boolean UseSessionAsDataStore = true;

    protected const String kUserId = "UserId";
    protected const String kUserName = "UserName";
    protected const String kUserEmail = "UserEmail";

    public static Int32 UserId {
        get { return GetInt32Value(kUserId); }
        set { SaveValue(kUserId, value.ToString()); }
    }

    public static String UserName {
        get { return GetStringValue(kUserName); }
        set { SaveValue(kUserName, value); }
    }

    public static String UserEmail {
        get { return GetStringValue(kUserEmail); }
        set { SaveValue(kUserEmail, value); }
    }

    protected static void SaveValue(String key, String value) {
        UseSessionAsDataStore ?
        SessionHelper.SaveSessionValue(key, value) :
        CookieHelper.SaveCookieValue(key, value);
    }

    protected static void GetStringValue(String key) {
        UseSessionAsDataStore ? SessionHelper.GetSessionStringValue(key) : CookieHelper.GetCookieStringValue(key);
    }

    protected static void GetInt32Value(String key) {
        UseSessionAsDataStore ? SessionHelper.GetSessionInt32Value(key) : CookieHelper.GetCookieInt32Value(key);
    }
}

UserLoginModel.cs

public class UserLoginModel {
    AssignUser (Int32 userId, String userName, String userEmail) {
        UserModel.UserId = userId;
        UserModel.UserName = userName;
        UserModel.UserEmail = userEmail;
    }
}

SessionHelper.cs

(won't provide CookieModel.cs as it's not the issue in question)

public class SessionHelper {
    public static void SaveSessionValue(String key, String value) {
        HttpContext.Current.Session.Add(key, value);
    }

    public static String GetSessionStringValue(String key) {
        if (HttpContext.Current.Session == null || HttpContext.Current.Session[key] == null) { return null; }
        return HttpContext.Current.Session[key] != null ? HttpContext.Current.Session[key].ToString() : String.Empty;
    }

    public static Int32 GetSessionInt32Value(String key) {
        if (HttpContext.Current.Session == null || HttpContext.Current.Session[key] == null) { return 0; }
        Int32 value;
        Int32.TryParse(HttpContext.Current.Session[key].ToString(), out value);
        return value;
    }
}

LoginPage

/* try to log in user and return user data */
...
if (loginSuccessful) {
    UserLoginModel.AssignUser(userId, userName, userEmail);
}
...

MainPage

<html>
    <body>
        Hello <%=UserModel.UserName %>!
    </body>
</html>
...

I have a feeling that this code has the potential to share session data between 2 users, although we have yet to realize it, even with about 5 simultaneous logins.

Do not use Hungarian notation for constants, keep it in PascalCase. If you like to differentiate the names from the properties you can name in form of {PropertyName}Key, like UserIdKey.
Do prefer .net aliases over the full type name, like Int32 -> int, String -> string.
Avoid repetition of word, like User.UserId and SessionHelper.GetSession.... Unless you are worried about mixed up with the foreign key in your DTO, but it is not the case here.
UserModel (if it is User, that is fine)(you weren't really consistent with the code) should be renamed to CurrentUser :

// which one makes more sense ?
UserModel.Name = userName;
CurrentUser.Name = userName;

HttpContext.Current.Session shows up quite frequently in SessionHelper, you could use a property to shorten it :

private static HttpSessionState CurrentSession
{
    get { return HttpContext.Current.Session; }
}

The SessionHelper class :

public class SessionHelper
{
    private static HttpSessionState Session
    {
        get { return HttpContext.Current.Session; }
    }

    public void SaveValue(string key, string value)
    {
        Session.Add(key, value);
    }
    public static string GetStringValue(string key)
    {
        if (Session == null || Session[key] == null)
            return null;

        return HttpContext.Current.Session[key].ToString();
    }
    public static int GetIntValue(string key)
    {
        if (Session == null || Session[key] == null)
            return 0;

        int result;
        return int.TryParse(Session[key].ToString(), out result) ? result : 0;
    }
}

If you can get your hand on C# 6, consider this version :

public class SessionHelper_c6
{
    private static HttpSessionState Session => HttpContext.Current.Session;

    public void SaveValue(string key, string value)
    {
        Session.Add(key, value);
    }
    public static string GetStringValue(string key)
    {
        return Session?[key]?.ToString() ?? null;
    }
    public static int GetIntValue(string key)
    {
        var value = Session?[key]?.ToString() ?? null;
        int result;
        return value != null && int.TryParse(value, out result) ? result : 0;
    }
}


Returns Google Sheet cells from a named range I've got this function that's called in several places in some of my Google Spreadsheets. It accepts a Range object, a string to search for, and an optional boolean to determine whether to make the search case-sensitive or not, and defaults to true.

function getCellsWithValueInRange(SearchRange, search_string, case_sensitive) {
    var case_sensitive = (case_sensitive !== undefined && case_sensitive === false) ? false : true;
    var result = [];
    var RangeValues = { values: SearchRange.getValues(),
                         width: SearchRange.getWidth(),
                        height: SearchRange.getHeight(),
                         start: { column: SearchRange.getColumn(),
                                     row: SearchRange.getRow() }};
    for (var x = 0; x < RangeValues.width; x++) {
        for (var y = 0; y < RangeValues.height; y++) {
            if (!case_sensitive) {
                // Lower case the search term and potential result, but ONLY if either terms have the toLowerCase method (like date objects).
                search_string = search_string.toLowerCase ? search_string.toLowerCase() : search_string;
                RangeValues.values[y][x] = RangeValues.values[y][x].toLowerCase ? RangeValues.values[y][x].toLowerCase() : RangeValues.values[y][x];
            }
        if (search_string === RangeValues.values[y][x]) {
            result.push(SearchRange.getCell(y + RangeValues.start.row, x + RangeValues.start.column));
        }
      }
    }
    return result;
}

Right now, this code works, but I'm not terribly happy with for several reasons:

It uses multiple nested loops, which feel and look horrible (debugging this was a pain).
I have doubts on its performance. Right now I've attempted to follow the best practices by batching retrieval (the first draft of this code just directly referenced the specific Ranges via getCell in the nested loops, but performance was so bad that any range that was over 600 rows just simply gave up, hence the use of getValues() instead. I haven't looked at the performance of this version, but at least it will accept large ranges (approximately 12 columns × 1300+ rows) without failing because it exceeded the maximum execution time.
I have no idea if there are any search values that would cause this function to fail. I mean, it already throws an error if the Cell Range is an object (i.e. when it's a date). Where else would it fail? Can I protect myself from other instances?

In any case, are there better, more maintanable and performant ways to code this? I've kind of accepted that it won't exactly be the fastest code (it literally has to go through any kind of range, no matter it's state), but any kind of improvement will do.

I'm kind of stymied, mostly because after searching high and low, I've discovered that it's kind of hard to get indexes for nested Arrays in JavaScript (and, by extension, Google Apps Script), so my initial attempts to make this more functional by using map or filter (I was toying with using Underscore for Google Apps Script) just hit a brick wall.

I didn't run unit tests for this on different search inputs, but it should give an idea of how you can bundle index information into the results for consumption with array class methods. Because you are working in a fairly robust environment (Google Sheets), you can be confident that the search array you are working with has a guaranteed nesting level (which limits a lot of possible issues).

The general idea is to replace the raw value with an annotated object, after which you can remove the row & column structure and work only with the value. (It's possible you could just use the reduce | forEach and push Array methods to go from the values array to the flattened array of annotated values.)

function annotateAndCase_(input2DArr, lower) {
  // Map the 0-base row and column indices onto the value.
  return input2DArr.map(function (row, rowIndex) {
    return row.map(function (val, colIndex) {
      var v = (lower && typeof val.toLowerCase === 'function') ? val.toLowerCase() : val;
      return {r: rowIndex, c: colIndex, v: v};
    });
  });
}

function getCellsWithValueInRange(SearchRange, search_string, case_sensitive) {
  // Pass exactly `false` to disable case sensitivity. Other falsy (0, "", null, undefined) -> `true`
  case_sensitive = case_sensitive !== false;
  if (case_sensitive) {
    search_string = search_string.toLowerCase();
  }
  const startRow = SearchRange.getRow(),
        startCol = SearchRange.getColumn();

  const indexedValues = annotateAndCase_(SearchRange.getValues(), !case_sensitive);

  // 2D array with max nesting level = 1 -> https://stackoverflow.com/a/10865042
  const flattened = Array.prototype.concat.apply([], indexedValues);

  const results = flattened.filter(function (cell) {
    return cell.v === search_string;
  }).map(function (match) {
    // Return a string R1C1 notation of each matched value.
    return ["R", match.r + startRow, "C", match.c + startCol].join("");
  });

  // Use a RangeList to optimize obtaining a large number of possibly-disjoint ranges.
  return results.length ? SearchRange.getSheet().getRangeList(results).getRanges() : [];
}

As mentioned in comments by @Dimu, you should reconsider why you need the individual cell references (perhaps just the collection as a RangeList is sufficient, e.g. for treating them in the same manner).

It's a similarly good idea to separate the functionality of annotating from this specific function that searches for values, since that allows reusing code, e.g. annotated a range object, use various filter predicates, various transformative maps, various final consumers, and so on. Probably you should further separate the casing from the annotation step - only certain filter predicates for the annotated array would care.


Print input lines longer than 80 characters in C Exercise 1-17 from the book The C Programming Language: 2nd Edition, K&R

Full description: Write a program to print all input lines that are longer than 80 characters.

#include <stdio.h>

#define MAX_ARRAYCHARACTERS 1000
#define MIN_CHARACTERS 80

int main(void){

    int currchar, // current character being read
        currlinelen = 0; // how many characters in a line

    char characters[MAX_ARRAYCHARACTERS]; // holds all the characters

    while( ( currchar = getchar() ) != EOF ){
        characters[currlinelen] = currchar;
        ++currlinelen;

        if(currchar == '\n'){ // if there is a new line

            if(currlinelen >= MIN_CHARACTERS){ // if the total length of characters of this line is higher than 80
                puts("LINE WITH 80+ CHARACTERS: ");
                for(int thischar = 0; thischar < currlinelen; ++thischar) // print all the characters that were in that line
                    putchar(characters[thischar]);
            }

            currlinelen = 0;
        }
    }

    return 0;
}

That's too complicated a solution.

It also not only imposes an arbitrary limit on the maximum possible input line length, it fails to check that the input doesn't exceed this limit.

80 is the only significant number in this program.
There's no need to worry about anything more than that.

Simply get characters into an 80 character buffer.

If you get '\n' before it fills, empty the buffer.

Otherwise, print the full buffer and get and print characters until the next '\n'.


Simple image rotation script I am writing a simple image rotation script. Users press either the prev or next button to view the previous or next images. It works well, but the code looks over the top. Is there a way I can reduce the amount of code used?

index.php

<input type="button" name="prev" id="prev" value="prev" />
<input type="button" name="next" id="next" value="next" />

jQuery

$('#next').click(function(){
    if($('#pic1').hasClass('top-pic')){
        $('#pic1').removeClass('top-pic');
        $('#pic2').addClass('top-pic');
    }else if($('#pic2').hasClass('top-pic')){
        $('#pic2').removeClass('top-pic');
        $('#pic3').addClass('top-pic');
    }else if($('#pic3').hasClass('top-pic')){
        $('#pic3').removeClass('top-pic');
        $('#pic4').addClass('top-pic');
    }else if($('#pic4').hasClass('top-pic')){
        $('#pic4').removeClass('top-pic');
        $('#pic1').addClass('top-pic');
    }

});

$('#prev').click(function(){
    if($('#pic1').hasClass('top-pic')){
        $('#pic1').removeClass('top-pic');
        $('#pic4').addClass('top-pic');
    }else if($('#pic4').hasClass('top-pic')){
        $('#pic4').removeClass('top-pic');
        $('#pic3').addClass('top-pic');
    }else if($('#pic3').hasClass('top-pic')){
        $('#pic3').removeClass('top-pic');
        $('#pic2').addClass('top-pic');
    }else if($('#pic2').hasClass('top-pic')){
        $('#pic2').removeClass('top-pic');
        $('#pic1').addClass('top-pic');
    }

});

You can use jQuery's .next() and .prev()

function nextPic() {
  var current = $("#container .top-pic");
  var next = current.next("img");
  if( next.length === 0 ) {
    next = $("#container img:first");
  }
  current.removeClass("top-pic");
  next.addClass("top-pic");
}

function prevPic() {
  var current = $("#container .top-pic");
  var prev = current.prev("img");
  if( prev.length === 0 ) {
    prev = $("#container img:last");
  }
  current.removeClass("top-pic");
  prev.addClass("top-pic");
}

$('#next').click(nextPic);
$('#prev').click(prevPic);

Or, avoiding the duplication:

function switchTopPic(direction) {
  var map = { prev: 'last', next: 'first' },
      current = $("#container .top-pic"),
      target = current[direction]("img");
  if( target.length === 0 ) {
    target = $("#container img")[map[direction]]();
  }
  current.removeClass("top-pic");
  target.addClass("top-pic");
}

$('#next').click(function () { switchTopPic("next") });
$('#prev').click(function () { switchTopPic("prev") });

Or given the markup in the question, the last little bit could just be:

$('#next, #prev').click(function () { switchTopPic(this.id) });

Either way, it should handle an arbitrary number of images (including none at all), and it doesn't require a top-pic to be set ahead of time. All state information is kept in the markup, and does not require the image IDs to be sequential (they don't even need an ID at all).

Edit: Here's a jsfiddle


Selection Sort implementation using modern C++ I have written this code for Selection sort. I want to use more C++11, C++14 or C++17. Please sugest some better alternatives.

#include <iostream>
#include <vector>
#include <algorithm>
#include <iterator>

template <typename InputItr>
void selectionSort(InputItr first, InputItr last)
{
  while(first != last)
  {
    InputItr min = std::min_element(first, last);
    if(*min < *first)
    {
      std::swap(*min, *first);
    }
    ++first;
  }
}

template <typename Itr>
void print(Itr first, Itr last)
{
  while(first != last)
  {
    std::cout << *first <<" ";
    ++first;
  }
  std::cout << "\n";
}

int main()
{
  std::vector<int> v({5, 3, 12, 2, 8});
  std::cout << "Original Array :";
  print(v.begin(), v.end());
  selectionSort(v.begin(), v.end());
  std::cout <<"Sorted Array :";
  print(v.begin(), v.end());
  std::cout << '\n';

  std::vector<char> c({'t', 'q', 'a', 'r', 'p'});
  std::cout << "Original Array :";
  print(c.begin(), c.end());
  selectionSort(c.begin(), c.end());
  std::cout <<"Sorted Array :";
  print(c.begin(), c.end());
  std::cout << '\n';

  std::vector<std::string> str({"code", "live", "love", "sing", "create"});
  std::cout << "Original Array :";
  print(str.begin(), str.end());
  selectionSort(str.begin(), str.end());
  std::cout <<"Sorted Array :";
  print(str.begin(), str.end());
  std::cout << '\n';

}

InputItr doesn't appropriately describe the iterator requirement of your selection sort, which multi-passes the range.  Consider naming the type after forward iterator.
Allow for custom overloads of swap by calling swap in an unqualified context.

    using std::swap;
    swap(*min, *first);

which is just std::iter_swap

    std::iter_swap(min, first);

You don't need to conditionally check if min is smaller than first.  Just swap.

  for (; first != last; ++first) {
    auto min = std::min_element(first, last);
    std::iter_swap(min, first);
  }

Your print function is just a reimplementation of std::copy.
Make sure you include everything (missing <string>).
Assert liberally. See the CPPCoreGuidelines.


C++ Random Phone Number Generator I was tinkering around with C++ after about 3 days of learning and decided to make a random phone number generator.

This is the code I came up with

#include <iostream>
#include <string>
#include <ctime>
#include <cstdlib>

using namespace std;
int getrandomdigit();

int main()
{
    srand(time(NULL));
    string input;
    int digit1_1;
    int digit1_2;
    int digit1_3;

    int digit2_1;
    int digit2_2;
    int digit2_3;
    int digit2_4;

    cout << "Enter three digits(area code): ";
    getline(cin, input);
    cout << "\n";
    cout << "\tAvailable numbers in your area.";
    cout << "\n\t******************************";

    cout << "\n\n";

    for(int i = 0; i < 10; ++i)
        cout << "\nPhone number: " << input << "-" << getrandomdigit() << getrandomdigit()
        << getrandomdigit() << "-" << getrandomdigit()
        << getrandomdigit() << getrandomdigit() << getrandomdigit();

    cout<<"\n";
    return 0;

}

int getrandomdigit()
{
    return rand() % 10;
}

So I believe I understand the logic of how the random number generation works. But before I added the function getrandomdigit(), I was using the manually defined "digit" vars in an attempt to generate pseudorandom numbers. So my question remains, why does it occur when you use the integers that the numbers AREN'T randomly generated. For instance, if you were to enter an area code, it would just print the same 2 prefixes 10 times, but when you use the function, it gives you a completely random var every time the counter is run until 10.

Here's a visual example of using the digit vars

Enter three digits(area code): 203

        Available numbers in your area.
        ******************************

Phone number: 203-523-3023
Phone number: 203-523-3023
Phone number: 203-523-3023
Phone number: 203-523-3023
Phone number: 203-523-3023
Phone number: 203-523-3023
Phone number: 203-523-3023
Phone number: 203-523-3023
Phone number: 203-523-3023
Phone number: 203-523-3023

Then here is an example of using the getrandomdigit function

Enter three digits(area code): 203

        Available numbers in your area.
        ******************************

Phone number: 203-696-3428
Phone number: 203-832-2940
Phone number: 203-293-9390
Phone number: 203-483-3935
Phone number: 203-616-6955
Phone number: 203-089-8674
Phone number: 203-953-6456
Phone number: 203-516-2123
Phone number: 203-800-7836
Phone number: 203-624-7130

Here are some observations that may help you improve your code.

Don't abuse using namespace std

Putting using namespace std at the top of every program is a bad habit that you'd do well to avoid.

Eliminate unused variables

Unused variables are a sign of poor code quality, so eliminating them should be a priority.  In this code, none of the digit_ variables are ever actually used.  My compiler also tells me that.  Your compiler is probably also smart enough to tell you that, if you ask it to do so.

Consider using a better random number generator

Because you're using a compiler that supports at least C++11, consider using a better random number generator.  In particular, instead of rand, you might want to look at std::uniform_real_distribution and friends in the <random> header.

Use string concatenation

The main function includes these lines:

std::cout << "\n";
std::cout << "\tAvailable numbers in your area.";
std::cout << "\n\t******************************";
std::cout << "\n\n";

Each of those is a separate call to operator<< but they don't need to be.  Another way to write that would be like this:

std::cout << "\n"
            "\tAvailable numbers in your area."
            "\n\t******************************"
            "\n\n";

This reduces the entire menu to a single call to operator<< because consecutive strings in C++ (and in C, for that matter) are automatically concatenated into a single string by the compiler.

Consider improving names

The variable input is not very descriptive.  Perhaps areaCode would be a better name.

Add error checking

The program appears to assume that the user will enter a valid 3-digit area code, but no provision is made to assure this.  A robust program always checks user input and provides error checking and handling.

Use object orientation

Because you're writing in C++, it would make sense to have a class such as PhoneNumber to encapsulate the details of your implementation.  You may not yet have learned about objects or classes, but they're one of the main strengths of C++ and something you should learn soon if you haven't already.  Use objects where they make sense.

Understand the problem domain

In the US and Canada (which, by the formatting of the phone number, seems to be where this is intended to be used), phone numbers are created according to the North American Numbering Plan.  In that plan, the second grouping of numbers is called that Central office code.  That three-digit number must start with a number in the range of 2 through 9 and cannot start with a 0 or 1.  Your current program does not obey this scheme and generates invalid numbers.


Python list "commander" (modify a list with user commands) I made a program that can modify a list using commands that the user enters.

The commands that are supported are (used from actual program):

/add [value1] [value2] [valueN]... - Add a value to the list
/rvalue [value] - Removes a value from the list
/rindex [index] - Removes the value at the specified index from the list
/print - Prints the list
/help - Lists every available command

Here is the code:

#Program to modify lists

value_list = []

def cmd_help():
    print("/add [value1] [value2] [valueN]... - Add a value to the list")
    print("/rvalue [value] - Removes a value from the list")
    print("/rindex [index] - Removes the value at the specified index from the list")
    print("/print - Prints the list")
    print("/help - Lists every available command")

def get_command_params():
    commands = {
        "/add": lambda *value : value_list.extend(list(value)),
        "/rvalue": lambda value : value_list.remove(value),
        "/rindex": lambda value : value_list.pop(int(value)),
        "/print": lambda : print(value_list),
        "/help": cmd_help
    }

    while True:
        input_cmd = input("Enter a command: ")

        #Separate arguments
        command = ""
        current_args_index = -1
        args = []
        last_char = ""

        for char in input_cmd:
            #Update separator
            if char == "/" or char == " ":
                last_char = char

            if last_char == "/":
                command += char
            elif last_char == "append":
                args[current_args_index] += char
            elif last_char == " ":
                args.append("")
                #To append individual characters to same index
                last_char = "append"
                ++current_args_index

        cmd = commands.get(command)
        if cmd == None:
            print("Invalid command!")
            continue

        return cmd, args

def main():
    cmd_help()

    #Wait for command
    while True:
        command, args = get_command_params()

        #Execute command with arguments
        try:
            command(*args)

            #Don't print success if command is print/help
            if args != []:
                print("Success!")
        except IndexError:
            print("Index out of range!")
        except ValueError:
            print("Element not found!")
        except TypeError:
            print("Invalid arguments! Type /help for help")

if __name__ == "__main__":
    main()

value_list = []

Global variables are almost never necessary.  If you are tempted to use a global, you have two choices:

Give the variable as an argument and reassign it to what the function returns (or just give it if the function mutates it).
Define a class and use instance attributes.

In this case, value_list is used by far too many things and with lambda functions and everything.  Therefore, I would suggest that you define a Commands class.

"/add": lambda *value : value_list.extend(list(value))

You don't need to convert value to a list.  That is just unnecessary processing time.  A list can be extended by a list, a tuple, a string, a generator, ... whatever iterable you want.

"/rindex": lambda value : value_list.pop(int(value)),

I wouldn't convert it to an integer within a lambda function.  I would create a whole function that uses a try block in case the user doesn't type a valid number.  That is also helpful if the user does type a number, but types a number that is too big or too small.

elif last_char == "append"
...
    last_char = "append"

Don't use magic numbers (or magic values).  You should define a constant called APPEND that can really be assigned to any object except a single-character string.

++current_args_index

Python doesn't use ++.  If x is 4, then +x is 4 and ++x is still 4.  If x is -4, then +x is -4 and ++x is still -4.  In other words, it does absolutely nothing.  Python isn't like C where that increments current_args_index.  If you want to increment it, do current_args_index += 1

cmd = ...
if cmd == None:
    ...

I have two problems.  First, if you treat None as a special case, you can simply use a try block.  Second, you should say is None, not == None because a class could define its __eq__ method to say that any instance is equal to None, but only None really is None.

Your main() function and how you use it looks terrific!  You take care of each error separately, and you don't call main() unless your file is being run as a program.  Keep it up!


Displaying a new image document in Picturebox I'm using WinForms and have a picturebox in my form. This program looks in "C:\image\" directory to find a specified image document, in my case a tif file. There is always only one picture in "C:\image\" directory.

After it locates the file, the program displays the image document in the picturebox.

When I was running this, I saw that the CPU usage was high. My goal is to make performance better or find out if there is a better way of coding this.

I have to manually go into my C:\image\ directory and delete the current image document then place a new image document in there so my picturebox will show the new image document.

int picWidth, picHeight;

private void Form1_Load(object sender, EventArgs e)
{
    timer1_Tick(sender, e);
}

private void File_Length()
{
    try
    {

        string path = @"C:\image\";
        string[] filename = Directory.GetFiles(path, "*.tif"); //gets a specific image doc.

        if (filename.Length > 0)
        {
            FileInfo fi = new FileInfo(filename[0]);
            byte[] buff = new byte[fi.Length];
            using (FileStream fs = File.OpenRead(filename[0]))
            {
                fs.Read(buff, 0, (int)fi.Length);
            }
            MemoryStream ms = new MemoryStream(buff);
            Bitmap img1 = new Bitmap(ms);
            //opened = true; // the files was opened.
            pictureBox1.Image = img1;

            pictureBox1.Width = img1.Width;
            pictureBox1.Height = img1.Height;
            picWidth = pictureBox1.Width;
            picHeight = pictureBox1.Height;
        }
    }
    catch(Exception)
    {

    }
}

public void InitTimer()
{
    timer1 = new Timer();
    timer1.Tick += new EventHandler(timer1_Tick); //calls method
    timer1.Interval = 2000; // in miliseconds (1 second = 1000 millisecond)
    timer1.Start(); //starts timer
}

private void timer1_Tick(object sender, EventArgs e)
{
    File_Length(); //checking the file length every 2000 miliseconds
}

Let's start here:

string[] filename = Directory.GetFiles(path, "*.tif");

If you know there can be only one file then you can try to get it right away with SingleOrDefault

var imageFileName = Directory.GetFiles(path, "*.tif").SingleOrDefault();

the condition will then become

var fileExists = !string.IsNullOrEmpty(imageFileName);
if (!fileExists)
{
   return;
}

Further you don't need all this code

FileInfo fi = new FileInfo(filename[0]);
byte[] buff = new byte[fi.Length];
using (FileStream fs = File.OpenRead(filename[0]))
{
  fs.Read(buff, 0, (int)fi.Length);
}
MemoryStream ms = new MemoryStream(buff);
Bitmap img1 = new Bitmap(ms);
//opened = true; // the files was opened.
pictureBox1.Image = img1;

instead you can just use

pictureBox1.Image = Image.FromFile(imageFileName);

besides there is a memory leak, you need to dispose the old image so before assigning a new one:

if (pictureBox.Image != null)
{
     pictureBox.Image.Dispose();
     pictureBox.Image = null;
}
pictureBox1.Image = Image.FromFile(imageFileName);

About the timer...

time1 = new Timer();
time1 .Tick += new EventHandler(timer1_Tick); //calls method
time1 .Interval = 2000; // in miliseconds (1 second = 1000 millisecond)
time1 .Start(); //starts timer

You have too many obvious comments here. There is no need to comment start as start ;-) One of them can even be replaced by a helper variable that makes the comment unnecessary. Meaningful variables are always better then comments.

const int oneSecondInMilisecods = 1000;

time1 = new Timer();
time1.Tick += timer1_Tick;
time1.Interval = 2 * oneSecondInMilisecods;
time1.Start();

Notice that you can also use a short event handler assignment.

Other suggestions:

You should use full names instead of abbreviations like fi or buff, imageFileInfo and imageBytes sounds much better and you don't have to think first what those variables are for
Collections should have plural form so string[] filename should be string[] imageFileNames
File_Length is not a good name becuause it doesn't tell us anything about what the method does and in your case it does a completely different thing. LoadImage would be much better here, we also don't put _ in method names unless they are event handlers or tests.
I would also change the name of the timer1 to loadImageTimer
You didn't put all the image loading code inside the time1_Tick event handler which is good ;-)


LeetCode Minimum Path Sum algorithm I am attempting to solve the Minimum Path Sum algorithm problem and I have a working solution, however there was an unwritten requirement that the algorithm not exceed an unspecified amount of time for any given matrix of size (m x n).

  Given a m x n grid filled with non-negative numbers, find a path from top left to bottom right which minimizes the sum of all numbers along its path.

  Note: You can only move either down or right at any point in time.

I know that this is one of these problems perfectly suited to Dynamic Programming, however my brain turns to mush when I start reading up on it.  This is supposed to be possible with two different state arrays capturing sums, but I am not comprehending how to do this accurate such that I am sure I am going the right direction.

public class Solution {

    int width = 0;
    int height = 0;
    HashMap<Point, Integer> previousStates = new HashMap<Point, Integer>();

    public int minPathSum(int[][] grid) {
        previousStates = new HashMap<Point, Integer>();
        height = grid.length;
        width = grid[0].length;
        return func(grid, new Point(0, 0));
    }

    int func(int[][] grid, Point p) {
        if (p.x == (width - 1) && p.y == (height - 1)) {
            return grid[p.y][p.x];
        }

        Point rightMostPoint = new Point(p.x + 1, p.y);
        Point downMostPoint = new Point(p.x, p.y + 1);
        boolean existingRightPreviousState = previousStates.containsKey(rightMostPoint);
        boolean existingDownPreviousState = previousStates.containsKey(downMostPoint);
        int rightLowestCost = (existingRightPreviousState) ? previousStates.get(rightMostPoint) : Integer.MAX_VALUE;
        int downLowestCost = (existingDownPreviousState) ? previousStates.get(downMostPoint) : Integer.MAX_VALUE;
        if (rightMostPoint.x < width && !existingRightPreviousState) {
            rightLowestCost = func(grid, rightMostPoint) + grid[p.y][p.x];
            previousStates.put(rightMostPoint, rightLowestCost);
        }
        if (downMostPoint.y < height && !existingDownPreviousState) {
            downLowestCost = func(grid, downMostPoint) + grid[p.y][p.x];
            previousStates.put(downMostPoint, downLowestCost);
        }

        return (rightLowestCost <= downLowestCost) ? rightLowestCost : downLowestCost;

    }
}

class Point {
    int x; int y;
    public Point(int x, int y) { this.x = x; this.y = y; }
    public boolean equals(Object obj) {
        if (obj == null || !(obj instanceof Point))
            return false;
        Point p = (Point)obj;
        return this.x == p.x && this.y == p.y;
    }
    public int hashCode() {
        return this.x ^ this.y;
    }
}

I start at the top left and recursively work either down or right based on which one returns the lowest sum.  For a given cell in the matrix if I have already encountered it then I already know which Point has the lowest cost in relation to that so I don't have to re-evaluate it.  In a way I am capturing state and using it but it still doesn't seem to meet the performance requirements of this problem.  I am guessing the unspoken requirement is \$O(n)\$ complexity and I am unsure in my recursive approach if that is possible.

Do you have any suggestions for improvement here so that I can reduce the operational complexity of this algorithm or at least offer me a better explanation of how dynamic programming can be used to solve this?

Your approach is technically correct, but very complex. Your call stack gets very big and you use very much memory by creating two objects for each point. But you don't need to.

What are you doing with your recursive calls? You go go from start to end and put every point on the call stack. But you have every point in the matrix. You don't need to create a copy.

You do a depth first search, but you should do a breadth first search. Think about how you can manipulate the grid on the way down, so the solution could be found in the grid.


Fibonacci Sequence in BrainF*** I have been trying to write a BrainF*** program that prints the Fibonacci sequence numbers repeatedly. I was wondering whether this is the most efficient way to do it. I basically repeatedly duplicate two cells into a third cell, and shift one cell forward and repeat.

+++++[->----[---->+<]>++.-[++++>---<]>.++.---------.+++.[++>---<]>--.++[->+++<]>.+++++++++..---.+++++++.+[-->+++++<]>-.<]

Portability

Your first loop immediately subtracts 4 from a cell whose value is 0.  It seems like your program is designed to run using the wraparound dialect of Brainfuck, so it would be a good idea to note that in a comment.  On the other hand, writing a Fibonacci sequence generator using the 8-bit dialect isn't that useful, because you would overflow after 0, 1, 1, 2, 3, 5, 8, 13, 21, 34, 55, 89, 144, 233.

However, I'm puzzled by why you would want to do any subtraction at all.  After all, the Fibonacci sequence involves addition, not subtraction.

Readability

There's no need to cram everything on one line.  Best practice would be to insert line breaks after . commands and use indentation to show off the loop structure.  Add some spaces as well, to let your code breathe.

Suggested solution

++++++++++ [->
   +++>+++++++>++++++++++>+++++++++++>++++++++++++ <<<<<
]
>> +++ .
< ++ .
>>> ++ .
< +++++  .
> ++++ .
> + .
<<<< .
>>> .
< - .
--- .
<< .
>> + .
> ----- .
.
--- .
<<< + .


Extract higher value of pair from JavaScript array Say we have these data:

var mat = [
  {
    "id": "A-B",
    "x": "1",
    "y": "0",
    "weight": "100"
  },
  {
    "id": "A-C",
    "x": "2",
    "y": "0",
    "weight": "77"
  },
  {
    "id": "A-D",
    "x": "3",
    "y": "0",
    "weight": "95"
  },
  {
    "id": "A-E",
    "x": "4",
    "y": "0",
    "weight": "18"
  },
{
    "id": "B-C",
    "x": "1",
    "y": "2",
    "weight": "3"
  },
 {
    "id": "D-F",
    "x": "5",
    "y": "3",
    "weight": "18"
  },
{
    "id": "B-A",
    "x": "0",
    "y": "1",
    "weight": "10"
  },
  {
    "id": "C-A",
    "x": "0",
    "y": "2",
    "weight": "7"
  },
{
    "id": "C-B",
    "x": "2",
    "y": "1",
    "weight": "1"
  },
  {
    "id": "D-A",
    "x": "0",
    "y": "3",
    "weight": "5"
  },
  {
    "id": "E-A",
    "x": "0",
    "y": "4",
    "weight": "18"
  },
 {
    "id": "F-D",
    "x": "3",
    "y": "5",
    "weight": "1"
  }
]

My task is to compare each object with its counterpart — i.e. compare A-B with B-A etc.  Note that for each pair, the x value of one is the y value of the other, and the y of one is the x of the other.  I want to only keep elements that have the higher weight of their pair.  If a pair has the same value of weight - as you see with A-E and E-A then both are discarded.

I am  very unfamiliar with JavaScript.  This is how I achieved this, but I have to feel like there is a better (i.e. less code, faster) way of doing it.  Also apologies if my javascript terminology is wrong.

// First I stored variables as vectors
var x = mat.map(function(x) {return x.x;});
var y = mat.map(function(x) {return x.y;});
var weights = mat.map(function(x) {return x.weight;});

// create a sequence of indices the length of the original array
var seq = [];
for (var i = 0; i < x.length; ++i) seq.push(i)

The strategy is to find the indices where each value of x is matched in y and vise-versa.  I use the helper function below to find which index is true for both for all:

// This is a Function to find matches
Array.prototype.diff = function(arr2) {
    var ret = [];
    this.sort();
    arr2.sort();
    for(var i = 0; i < this.length; i += 1) {
        if(arr2.indexOf( this[i] ) > -1){
            ret.push( this[i] );
        }
    }
    return ret;
};

Here, I go step by step to find the index that matches (i.e. is the opposite id):

// create a sequence
var seq = [];
for (var i = 0; i < x.length; ++i) seq.push(i)

var seqindices = [];
for(var j=0; j < seq.length; j++) {

var indicesy = [];
for(var i=0; i<x.length;i++) {
    if (y[i] === x[j]) indicesy.push(i);
}

var indicesx = [];
for(var i=0; i<x.length;i++) {
    if (x[i] === y[j]) indicesx.push(i);
}

seqindices.push(indicesx.diff(indicesy));
}

// seqindices
// 6,7,9,10,8,11,0,1,4,2,3,5

Now I compare the weights of each against its matching index and only keep weights if greater, remove if equal to or lower.

var matres = [];
for(var i=0; i<seqindices.length;i++) {
    if (weights[i] > weights[seqindices[i]]) matres.push(mat[i]);
}

This gives the following result:

[
  {
    "id": "A-B",
    "x": "1",
    "y": "0",
    "weight": "100"
  },
  {
    "id": "A-C",
    "x": "2",
    "y": "0",
    "weight": "77"
  },
  {
    "id": "A-D",
    "x": "3",
    "y": "0",
    "weight": "95"
  },
  {
    "id": "B-C",
    "x": "1",
    "y": "2",
    "weight": "3"
  },
  {
    "id": "D-F",
    "x": "5",
    "y": "3",
    "weight": "18"
  }
]

I have to believe I have gone a really long way round to get this result.  Hopefully someone can advise a better way.

Rewrite
Here's a functional approach, with notes inline as comments.
Note that the internal variables are all essentially simple "helper" functions or calculations, which allow us to write the entire algorithm that you're seeking as a single statement (the final return).
The basic idea is to first group together the pairs of "twin" elements.  Once we have that, we just go through the pairs and select the one with the highest weight.
function bestOfPairs(input) {
  var pairId   = elm    => elm.id.split('-').sort().join(''),        // creates an id that's the same for both elements of a "pair"
      byWeight = (a, b) => parseInt(b.weight) - parseInt(a.weight);  // reverse sort by weight: best candidate will be first in array
      grouped  = input.reduce((m, x) =>
        (m[pairId(x)] = (m[pairId(x)] || []).concat(x)) && m         // groups together elements as pairs, based on their id
      , {});

  return Object.keys(grouped).map(id => {                            // each "id" here is the "pairs" id, that is, the id being used to create the groups
    var sorted  = grouped[id].sort(byWeight),
        invalid = sorted[1] == undefined ||
                  sorted[0].weight == sorted[1].weight;
    return invalid ? null : sorted[0];                               // returns the element with higher weight, or null if they're equal, or there's only one
  })
  .filter(x => x !== null);                                          // this removes elments with the same weight
}

Output
[ { id: 'A-B', x: '1', y: '0', weight: '100' },
  { id: 'A-C', x: '2', y: '0', weight: '77' },
  { id: 'A-D', x: '3', y: '0', weight: '95' },
  { id: 'B-C', x: '1', y: '2', weight: '3' },
  { id: 'D-F', x: '5', y: '3', weight: '18' } ]

Finally, note that I'm using ES6 syntax, which is supported by modern browsers, but if you needed support for older browsers you'd need to transpile it using Babel.


Copy a directory structure to another, but only copying specific files script.py:

#!/usr/bin/python

import os

srcDir = os.getcwd()

dirName = 'target_directory'

dstDir = os.path.abspath(dirName)

def ignore_list(path, files):

    filesToIgnore = []

    for fileName in files:

        fullFileName = os.path.join(os.path.normpath(path), fileName)

        if (not os.path.isdir(fullFileName)
            and not fileName.endswith('pyc')
            and not fileName.endswith('ui')
            and not fileName.endswith('txt')
            and not fileName == '__main__.py'
            and not fileName == 'dcpp.bat'):

            filesToIgnore.append(fileName)

    return filesToIgnore

# start of script

shutil.copytree(srcDir, dstDir, ignore=ignore_list)

As shutil.copytree() has no option where I can give names for required files to copy like "ignore," I have modified the argument of ignore to give "required files to copy."

Review my code.

Looks ok for a specific task, but the code is not reusable at all. It would be more useful to create a function ignore_except that could be used like this to perform the same task:

shutil.copytree(srcDir, dstDir,
                ignore=ignore_except('*.pyc', '*.ui', '*.txt', '__main__.py', 'dcpp.bat'))

The source code of shutil.ignore_patterns would be a good starting point for such function.


Dynamically adding rows to an accessible HTML form Given the following HTML form (fragment):

<fieldset id="timesheet-rows">
    <legend>Add Entries</legend>

      Project
      <select id="project-0" name="project-0" required>
        <option value="" />
      </select>

      Department
      <select id="department-0" name="department-0" required>
        <option value="" />
      </select>

      Task:
      <select id="task-0" name="task-0" required>
        <option value="" />
      </select>

      Hours:
      <input type="number" step="0.25" id="hours-0" name="hours-0" width="1" placeholder="2.0" required />

      Comment:
      <input type="text" id="comment-0" name="comment-0" width="50" />

    <input type="button" id="add-row" name="add-row" value="Add row" />

  </fieldset>

I have implemented the following jQuery (fragment) to clone each 'timesheet row'
 along with the  tags, which are not caught by $(':input'):

$(document).ready(function() {

  var current_id = 0;
  $('#add-row').click(function(){
    next_element($('#timesheetrow-0'));
  })

  function next_element(element){
    var new_element = element.clone(),
        id = current_id + 1;
    current_id = id;
    new_element.attr("id",element.attr("id").split("-")[0]+"-"+id);

    // Ajuster les `id` et `name` dans les <input />s et <select />s
    $(':input', new_element).each(function(){
      var field_id = $(this).attr("id"),
          field_name = $(this).attr("name");
      $(this).attr("id", field_id.split("-")[0]+"-"+id );
      $(this).attr("name", field_name.split("-")[0]+"-"+id );
    });

    // Ajuster le for="" dans les s
    $('label', new_element).each(function(){
      field_for = $(this).attr("for");
      $(this).attr("for", field_for.split("-")[0]+"-"+id );
    });
    new_element.appendTo($("#timesheet-rows"));
  };
});

(Above jQuery was inspired by http://jsfiddle.net/32RgL/ ).

Is there a more elegant or complete way of doing this?

Most of this code is due to the fact that you're increasing the numbers in your attributes. I think that's a mistake. Your rows should all have identical name attributes, stored in an array (e.g. name="task[]").

The only problem would then be the identical IDs. Frankly, you shouldn't be using IDs in the first place. The only advantage of using IDs in this case is to associate the labels to the form fields (so that clicking the label focuses on the form field). This could easily be accomplished by wrapping the label around the form element.

So, to summarize, here are some points to consider:

Get rid of the IDs
Wrap the labels around the form elements to associate them with one another
Get rid of the numbers from the names
Use array like notation (e.g. name="task[]") for your names

Once you've incorporated all this, you can simply clone the row when needed.

HTML:

<fieldset id="timesheet-rows">
    <legend>Add Entries</legend>

    <div class="timesheet-row">

        <label>Project:
            <select name="project[]" required>
                <option value=""></option>
            </select>
        </label>

        <label>Department:
            <select name="department[]" required>
                <option value=""></option>
            </select>
        </label>

        <label>Task:
            <select name="task[]" required>
                <option value=""></option>
            </select>
        </label>

        <label>Hours:
            <input type="number" step="0.25" name="hours[]" width="1" placeholder="2.0" required />
        </label>

        <label>Comment:
            <input type="text" name="comment[]" width="50" />
        </label>

    </div>

    <input type="button" id="add-row" name="add-row" value="Add row" />
</fieldset>

Javascript:

jQuery(function($) {
    var $button = $('#add-row'),
        $row = $('.timesheet-row').clone();

    $button.click(function() {
        $row.clone().insertBefore( $button );
    });
});

Here's the fiddle: http://jsfiddle.net/wd5y9/


just implemented multiprocessing and queue demo, wonder if there are any improvements? i will give a list of urls, and want to make multi processes to fetch certain url's web content, and quit if all the urls are all fetched.

here is my implementation, and not sure if it's the right way to do the stuff:

#coding=utf8

import multiprocessing
from multiprocessing import JoinableQueue
import urllib2
import logging
import os

logging.basicConfig(level=logging.DEBUG)

URLS = [
        'http://stackoverflow.com/q/2243542/94962',
        'http://docs.python.org/library/logging.html',
        'http://www.python.org/dev/peps/pep-3101/',
        'http://news.ycombinator.com/',
        'http://www.evernote.com/about/learn_more/',
        'http://news.php.net/php.internals/55293',
        ]

POOL_SIZE = multiprocessing.cpu_count()

DEST_DIR = '/tmp/pytest/'

url_q = JoinableQueue()

class Worker(multiprocessing.Process):

    def run(self):
        while True:
            try:
                url = url_q.get()
                logging.info('%(process_name)s processing %(url)s' % {
                    'process_name': multiprocessing.current_process().name,
                    'url':url,
                    })
                web_cnt = urllib2.urlopen(url).read()
                url_filename = url[7:].replace('/', '-').strip('.html') + '.html'
                with open(os.path.join(DEST_DIR, url_filename), 'w') as f:
                    f.write(web_cnt)
                url_q.task_done()
            except Exception:
                logging.exception('error')

workers = []
for i in range(POOL_SIZE):
    worker = Worker()
    worker.name = 'worker%s'%i
    workers.append(worker)
    worker.start()

for url in URLS:
    url_q.put(url)

url_q.join()

print 'workers have done stuff'

for worker in workers:
    worker.terminate()

You should pass the url_q queue to the constructor instead of using it as a global variable.


Using large amounts of if-else statements for playing card numbers I have a method that uses a bunch of if-else statements, and I am thinking how I could simplify it.

public static CardNumber decode(String s) {
    if(s == null) {
        return null;
    } else if(s.equalsIgnoreCase(ACE.toString())) {
        return ACE;
    } else if(s.equalsIgnoreCase(TWO.toString())) {
        return TWO;
    } else if(s.equalsIgnoreCase(THREE.toString())) {
        return THREE;
    } else if(s.equalsIgnoreCase(FOUR.toString())) {
        return FOUR;
    } else if(s.equalsIgnoreCase(FIVE.toString())) {
        return FIVE;
    } else if(s.equalsIgnoreCase(SIX.toString())) {
        return SIX;
    } else if(s.equalsIgnoreCase(SEVEN.toString())) {
        return SEVEN;
    } else if(s.equalsIgnoreCase(EIGHT.toString())) {
        return EIGHT;
    } else if(s.equalsIgnoreCase(NINE.toString())) {
        return NINE;
    } else if(s.equalsIgnoreCase(TEN.toString())) {
        return TEN;
    } else if(s.equalsIgnoreCase(JACK.toString())) {
        return JACK;
    } else if(s.equalsIgnoreCase(QUEEN.toString())) {
        return QUEEN;
    } else if(s.equalsIgnoreCase(KING.toString())) {
        return KING;
    } else {
        return null;
    }
}

The whole class is here:

public class CardNumber {
    private final String name;
    private final int value;

    public static final CardNumber ACE_AS_ONE = new CardNumber("A", 1);
    public static final CardNumber TWO = new CardNumber("2", 2);
    public static final CardNumber THREE = new CardNumber("3", 3);
    public static final CardNumber FOUR = new CardNumber("4", 4);
    public static final CardNumber FIVE = new CardNumber("5", 5);
    public static final CardNumber SIX = new CardNumber("6", 6);
    public static final CardNumber SEVEN = new CardNumber("7", 7);
    public static final CardNumber EIGHT = new CardNumber("8", 8);
    public static final CardNumber NINE = new CardNumber("9", 9);
    public static final CardNumber TEN = new CardNumber("10", 10);
    public static final CardNumber JACK = new CardNumber("J", 11);
    public static final CardNumber QUEEN = new CardNumber("Q", 12);
    public static final CardNumber KING = new CardNumber("K", 13);
    public static final CardNumber ACE = new CardNumber("A", 14);

    private CardNumber(String name, int value) {
        this.name = name;
        this.value = value;
    }

    @Override
    public String toString() {
        return name;
    }

    public static CardNumber decode(String s) {
        if(s == null) {
            return null;
        } else if(s.equalsIgnoreCase(ACE.toString())) {
            return ACE;
        } else if(s.equalsIgnoreCase(TWO.toString())) {
            return TWO;
        } else if(s.equalsIgnoreCase(THREE.toString())) {
            return THREE;
        } else if(s.equalsIgnoreCase(FOUR.toString())) {
            return FOUR;
        } else if(s.equalsIgnoreCase(FIVE.toString())) {
            return FIVE;
        } else if(s.equalsIgnoreCase(SIX.toString())) {
            return SIX;
        } else if(s.equalsIgnoreCase(SEVEN.toString())) {
            return SEVEN;
        } else if(s.equalsIgnoreCase(EIGHT.toString())) {
            return EIGHT;
        } else if(s.equalsIgnoreCase(NINE.toString())) {
            return NINE;
        } else if(s.equalsIgnoreCase(TEN.toString())) {
            return TEN;
        } else if(s.equalsIgnoreCase(JACK.toString())) {
            return JACK;
        } else if(s.equalsIgnoreCase(QUEEN.toString())) {
            return QUEEN;
        } else if(s.equalsIgnoreCase(KING.toString())) {
            return KING;
        } else {
            return null;
        }
    }

    public int getValue() {
        return value;
    }
}

Convert your constant values to an enum instead, for example:

enum Card {
    ACE_AS_ONE("A", 1),
    TWO("2", 2),
    THREE("3", 3),
    // and so on
    KING("K", 10);

    private final String name;
    private final int value;

    Card(String name, int value) {
        this.name = name;
        this.value = value;
    }

    public static Card decode(String name) {
        for (Card card : Card.values()) {
            if (card.name.equalsIgnoreCase(name)) {
                return card;
            }
        }
        return null;
    }
}

While at it, add some unit tests to verify, for example:

public class CardTest {
    @Test
    public void testDecodeNumeric() {
        assertEquals(Card.TWO, Card.decode("2"));
        assertEquals(Card.THREE, Card.decode("3"));
    }

    @Test
    public void testDecodeLabeled() {
        assertEquals(Card.KING, Card.decode("K"));
    }

    @Test
    public void testNonexistent() {
        assertNull(Card.decode("nonexistent"));
    }

    @Test
    public void testDecodeNull() {
        assertNull(Card.decode(null));
    }
}


Efficient storage and calling of varying objects This question deals with the ability to construct a structure that is able to be easily iterated through.

The problem:

I have four different "boxes":

Box A: Always has 1 apple and 1 pear
Box B: Always has 1 orange and 1 pear
Box C: Always has 1 peach
Box D: Can have varying amounts of apples and oranges (0 - 10+) but always has 1 pear

Each piece of fruit has a different taste value

I currently process this information by creating a queue of the different taste values and box types then process this information with a lots of if-elses in multiple functions.

Here is an example of one function that seeks to get a total sum of the taste values e.g.

void process_crate(vector<string> boxes, vector<double> taste_value, vector<vector<double> > amount){

    unsigned int count = 0; // holds index in taste_value
    double sum_taste = 0;  // holds taste value

    for(unsigned int i = 0; i < boxes.size(); i++){
        string active_box = boxes[i];

        if(active_box == "A"){ // An apple and a pear

            sum_taste += taste_box_A(taste_value[count],taste_value[count+1]); // user preference

            count += 2;
        }else if(active_box == "B"){ // An orange and a pear
            sum_taste += taste_box_B(taste_value[count], taste_value[count+1]); // user preference

            count += 2;
        }else if(active_box == "C"){ // A peach
            sum_taste += taste_box_C(taste_value[count], taste_value[count+1]); // user preference

            count += 1;
        }else{ // Box D varying apples and oranges, but always a pear

            unsigned int apples = amount[i][0];
            unsigned int oranges = amount[i][1];

            vector<double> apple_tastes(apples);
            vector<double> orange_tastes(oranges);

            double pear_taste = taste_value[count+apples + oranges];

            if(apples != 0){
                for(unsigned int j = 0; j<apples; j++){
                    apple_tastes[j] = taste_value[count+j];
                }
                count += apples;
            }

            if(oranges != 0){
                for(unsigned int j = 0; j<apples; j++){
                    orange_tastes[j] = taste_value[count+j];
                }
                count += oranges;
            }

            sum_taste += taste_box_D(apple_tastes, orange_tastes, pear_taste); // user preference
            count += 1;
        }

    }
}

I think use of IF-ELSE statements combined with lots of vectors to store the data could be greatly improved.

Any suggestions?

It's hard to know where to start with this, but here are some things that may help you improve your code.

When seeking a review, provide full code

The posted code is just a fragment with a lot of pieces missing.  For instance, all of the taste_box_x functions are missing and it's not really very obvious what they might be.  To get a better review, post a better question.

Pass complex types reference rather than by value

For complex types such as std::vector, it's usually better to pass by reference rather than by value to avoid forcing each function call to make copies.  If the passed object should not be modified by the routine, pass a const reference.

Use classes

Since this is C++, it would make sense to use classes.  For example, one could imagine that there is a base class box from which each of the box types might be derived.  User taste preference could be an std::unordered_set in which each fruit type is the key and the value would be that user's taste weighting value.  Fruits might be an enum class, etc.

Don't abuse using namespace std

Putting using namespace std at the top of every program is a bad habit that you'd do well to avoid.


Hash table with complex variable-length keys I'm trying to implement the game Snake in Javascript and getting the computer to play it by itself, and want to check whenever the snake loops (repeats the same set of moves twice) so I can kill it.

To do this, the first thing that came to mind is saving the state the entire game at every frame (the state being the position of the snake's head and body segments, as well as the position of the 'pallet' it's looking to eat) in some sort of dictionary, and checking every frame if the new state of the game exists in this dictionary.

I don't quite know what's the best/most efficient/most readable way to implement this is. Here's what I tried so far:

const Snake = {
  direction: new Vector2(0, 1), // Up
  pallet: Vector2.random(0, 100, 0, 100).floor(),
  head: Vector2.random(0, 100, 0, 100).floor(),
  body: [], // Array of Vector2's
  loopHistory: {}, // Records every state
  checkForLoop: function () {
    // Current state of the system:
    const arr = [this.direction.hash(), this.pallet.hash(), this.head.hash(), ...this.body.map(x => x.hash())];
    //True if a loop is found, false if the state is completely new:
    let found = true;
    let obj = this.loopHistory;
    for (let i = 0; i < arr.length; i++) {
      if (!obj.hasOwnProperty(arr[i])) {
        //This is a part of the state that has never been seen before (new pallet position, new body segment etc.)
        found = false;
        // Record it for the next time:
        obj[arr[i]] = {};
      }
      obj = obj[arr[i]];
    }
    return found;
  }
}

loopHistory and checkForLoop are the relevant bits. Here's the Vector2 class for reference:

 const Vector2 = function (x, y) {
   this.x = x;
   this.y = y;
 };

 Vector2.random = function (xmin, xmax, ymin, ymax) {
   return new Vector2(
     xmin + Math.random() * (xmax - xmin),
     ymin + Math.random() * (ymax - ymin)
   );
 };
 Vector2.prototype.hash = function () {
   return this.x + 73856093 * this.y;
 };
 Vector2.prototype.floor = function () {
   return new Vector2(Math.floor(this.x), Math.floor(this.y));
 };

The issue with this implementation is it generates a massive ever growing arbitrarily deep object (loopHistory) for every game being run. With each game there could be thousands of states before the snake actually loops to its death, and there could be up to one hundred games running at once. The solution feels inefficient and clumsy and difficult to read/maintain, plus it takes a lot of memory, but that seems inevitable. So I'm looking for the go-to solution for situation like this, if one exists, or just any better implementation.

Please help me kill the snake! Thanks in advance for answers.

Edit: I forgot to mention that the behavior of the snake is only dependent on the state of the system that we captured earlier (head position, body positions, pallet position) so if it finds itself in the exact same circumstance it will inevitably react the same way and repeat the same moves.

So I'm looking for the go-to solution for situation like this, if one exists

Big step, small step. Rather than run the snake once, you run it twice in parallel, but the two instances run at different speeds:

snake1 = ...;
snake2 = snake1.clone();
while (true) {
    snake1.step();
    snake2.step();
    snake2.step();
    if (snake1.equals(snake2)) {
       // Loop detected
    }
}

If the cycle is of length \$n\$ and you enter the cycle for the first time at \$t_0\$ then this will detect the cycle as long as there's a number \$t \ge t_0\$ for which \$t \equiv 2t \pmod n\$. But that reduces to \$t \equiv 0 \pmod n\$, so \$t\$ is the first multiple of \$n\$ greater than or equal to \$t_0\$, and it's guaranteed to find the loop.

Essentially you trade memory saving (no history required) for execution time (it takes \$3t\$ calls to step() rather than \$t_0 + n\$).

That renders rather unnecessary the following observation:

    const arr = [this.direction.hash(), this.pallet.hash(), this.head.hash(), ...this.body.map(x => x.hash())];

Most of those don't need to use a Vector2 with a hash: there are 4 possibilities for the direction, and the body can be represented by a series of directions. Using arrays of length 4 might be more efficient than object properties.


LeetCode: Validate Binary Search Tree C# I was struggling with this question for many times,
and I have solved it every time, because I understand the concept.
but I can never get to a good concise solution which is easy to follow.

In a coding interview I always choke when I see this question.

This is the best solution I was able to come up with.

Can the code be written even more simple and easier to follow?

using GraphsQuestions;
using Microsoft.VisualStudio.TestTools.UnitTesting;

namespace TreeQuestions
{
    /// <summary>
    /// https://leetcode.com/explore/interview/card/top-interview-questions-easy/94/trees/625/
    /// </summary>
    /// check the iterative version with a stack as well
    /// https://leetcode.com/explore/interview/card/top-interview-questions-easy/94/trees/625/discuss/32112/Learn-one-iterative-inorder-traversal-apply-it-to-multiple-tree-questions-(Java-Solution)
    [TestClass]
    public class IsValidBSTtest
    {
        //   2
        //  / \
        // 1   3
        [TestMethod]
        public void ValidTree()
        {
            TreeNode root = new TreeNode(2);
            root.left = new TreeNode(1);
            root.right = new TreeNode(3);
            IsValidBSTclass temp = new IsValidBSTclass();
            Assert.IsTrue(temp.IsValidBST(root));
        }

        [TestMethod]
        public void NotValidTree()
        {
            TreeNode root = new TreeNode(5);
            root.left = new TreeNode(1);
            root.right = new TreeNode(4);
            root.right.left = new TreeNode(3);
            root.right.right = new TreeNode(6);
            IsValidBSTclass temp = new IsValidBSTclass();
            Assert.IsFalse(temp.IsValidBST(root));
        }

        /*
         *      10
         *      / \
         *     5   15
         *         /\
         *        6  20
         */

        [TestMethod]
        public void NotValidTree2()
        {
            TreeNode root = new TreeNode(10);
            root.left = new TreeNode(5);
            root.right = new TreeNode(15);
            root.right.left = new TreeNode(6);
            root.right.right = new TreeNode(20);
            IsValidBSTclass temp = new IsValidBSTclass();
            Assert.IsFalse(temp.IsValidBST(root));
        }
    }

    public class IsValidBSTclass
    {
        public bool IsValidBST(TreeNode root)
        {
            return Helper(root, null, null);
        }
        public bool Helper(TreeNode root, TreeNode minNode, TreeNode maxNode)
        {
            if (root == null)
            {
                return true;
            }
            if (minNode != null && root.val <= minNode.val || maxNode != null && root.val >= maxNode.val)
            {
                return false;
            }

            return Helper(root.left, minNode, root) && Helper(root.right, root, maxNode);
        }
    }
}

For the most part I agree with your code.  However there are a couple of things:

IsValidBST I think is redundant.  Anyone running this method already knows it applies to a BST.  I think just IsValid would be better.

Helper should have a better name, I think an IsValid overload would work and it should be private.

It doesn't make much sense to me to make an instance of this class.  The methods don't rely on anything internal to the class.  I would suggest making the methods static.

It could look something like this:

public class IsValidBSTclass
{
    public static bool IsValid(TreeNode root)
    {
        return IsValid(root, null, null);
    }
    private static bool IsValid(TreeNode root, TreeNode minNode, TreeNode maxNode)
    {
        if (root == null)
        {
            return true;
        }
        if (minNode != null && root.val <= minNode.val || maxNode != null && root.val >= maxNode.val)
        {
            return false;
        }

        return IsValid(root.left, minNode, root) && IsValid(root.right, root, maxNode);
    }
}


Find elements occurring even number of times in an integer array I came across an interview question, which is like this:

  You have an array of integers, such that each integer is present an odd number of time, except 3 of them. Find the three numbers.

I tried implementing it using a HashMap<Integer, Integer>, by mapping each integer element to a number that would identify if that number occurs even number of times. I used the XOR logic for that. 5 ^ 5 = 0, and 5 ^ anyNumberOtherThan5 != 0.

Here's my code:

public static void findEvenOccurringNum(int[] arr) {
    Map<Integer, Integer> map = new HashMap<Integer, Integer>();

    for (int num: arr) {
        if (map.containsKey(num)) {
            map.put(num, map.get(num) ^ 1);
        } else {
            map.put(num, 1);
        }
    }

    for (Entry<Integer, Integer> entry: map.entrySet()) {
        if (entry.getValue() == 0) {
            System.out.println(entry.getKey());
        }
    }
}

What I don't like about this solution is the 2nd iteration. That would iterate over all the unique numbers in the array. Can there be any possible improvement here? May be a solution that avoids using a map? Or is it the best I can do?

Wiki Answer - discussion of scalability.

After some investigation I thought it would be interesting to publish some of the performance results I have seen.... there are some interesting observations.

I have taken two algorithms, both on this page, the 200_Success one, and the rolfl one. I have run both algorithms through two types of tests, and for each type, I ran it through data of different sizes. I measured the actual performance.

The two types of test are 'data complexity' related. The first test I call 'sparse', and there are very few duplicates of data. The second set of data is 'freq' which has many, many duplicates. The two data sets are constructed as (1 << 18 is 256K elements and runs in an 'about-right' time on my computer):

    int[] sparse = new int[1<<18];
    Random rand = new Random(1); // repeatable 'random' numbers
    for (int i = 0; i < sparse.length; i++) {
        sparse[i] = rand.nextInt(sparse.length);
    }

    int[] freq = new int[1<<18];
    for (int i = 0; i < freq.length; i++) {
        freq[i] = rand.nextInt(10);
    }

So, two different datasets.... and then, I use the data in 5 different sizes.... each size is half the size of the next size.... so, I have data of 16K, 32K, 64K, 128K, and 256K.

I then run each of the algorithms a number of times for each data size. The number of times depends on the size of the data.... The bigger the data is, the fewer times I run the loop, and, as the data size doubles, I halve the number of iterations.

iterations   256    128    64     32      16
data size  16384  32768 65536 131072  262144

In theory, in a perfectly scalable system, the data-size and iteration combinations should result in constant-time execution....

Then I timed the actual execution times, and here are the results (on a warmed up system).

RLsparse,130.632,164.163,225.850,271.158,289.436,322.981,343.293,360.966,390.827,412.959
RLfreq,60.455,66.364,53.215,76.341,82.661,85.065,88.740,87.201,89.798,87.361
2Ssparse,157.926,184.521,214.407,242.943,256.200,286.686,352.467,438.048,420.164,666.043
2Sfreq,152.263,155.817,158.332,160.107,158.856,157.523,159.182,158.238,158.978,157.142
datasize,512,1024,2048,4096,8192,16384,32768,65536,131072,262144
iterations,8192,4096,2048,1024,512,256,128,64,32,16

Plotting this as a graph, using the datasize as the X axis, it looks like:

The 'dip' in the performance of the 2Ssparse 'curve' is consistent... there must be some anomoly that happens in the data of that size that 'wins' for whatever reason.

The 'obvious' observations are:

both algorithms like lots of duplicate values.
for high-duplicate values the 200_Success algorithm is practically O(n) and it appears the HashMap is O(1) as stated.
for sparse-duplicate values the 200_Success scales worse than the rolfl version, and, although it started faster, for larger data sets it quickly degrades to be slower.
for sparse-duplicate values the rolfl version is faster (runs in 2/3 of the time), but is slightly degrading in performance.... I expect that for all reasonable data sizes it will be faster.....

EDIT:

After a request for 'medium' amounts of duplication (the random numbers are taken from a pool of 1024 instead of just 10):

RLsparse,124.211,147.412,216.475,273.934,293.235,330.671,346.458,372.003,399.730,407.985
RLfreq,42.496,41.446,53.732,58.305,66.281,65.889,64.953,66.078,73.901,71.291
RLmedium,122.813,179.644,218.106,253.908,272.583,259.016,232.671,226.121,220.121,207.180
2Ssparse,155.091,180.669,205.877,236.890,254.193,288.193,351.495,450.381,426.626,665.759
2Sfreq,172.426,173.460,173.988,174.462,175.049,174.477,175.384,172.826,174.230,175.976
2Smedium,162.969,210.496,232.372,235.070,229.555,222.412,219.141,219.358,218.502,221.412

And the corresponding plot is:

EDIT2:

The HashSet based implementation has been revised to include hints on th size of the HashSets. The revised performance results are:

RLsparse,113.848,156.549,195.834,264.010,280.561,314.784,332.941,360.607,387.728,401.440
RLfreq,50.688,45.955,61.013,59.733,74.000,68.274,69.208,70.487,76.905,74.185
RLmedium,116.118,177.327,223.468,258.029,275.395,263.365,234.839,225.775,221.798,210.071
2Ssparse,127.956,160.641,159.736,171.221,180.202,195.650,265.159,328.633,270.251,369.847
2Sfreq,154.762,154.865,157.780,157.265,158.705,159.562,160.342,161.276,160.924,163.296
2Smedium,128.366,152.183,187.613,198.995,270.550,198.450,200.526,202.244,205.382,207.368
datasize,512,1024,2048,4096,8192,16384,32768,65536,131072,262144
iterations,8192,4096,2048,1024,512,256,128,64,32,16

And the corresponding plot is (Note the scale is the same as other plots, max 700ms):

Here is the code I used to generate the above graph (the actual algorithms not included since they are above...).

This code is not neat, and please do not review it .... ;-) But feel free to make any changes you like to improve it... I know it is ugly... but it works.

package dupcnt;

import java.util.Arrays;
import java.util.HashSet;
import java.util.Random;
import java.util.Set;

public class CompareDups {

    public static void main(String[] args) {

        System.out.println("Building Data");

        int[] sparse = new int[1<<18];
        int[] freq = new int[sparse.length];
        int[] medium = new int[sparse.length];
        Random rand = new Random(1); // repeatable 'random' numbers
        for (int i = 0; i < sparse.length; i++) {
            sparse[i] = rand.nextInt(sparse.length);
        }
        for (int i = 0; i < freq.length; i++) {
            freq[i] = rand.nextInt(10);
        }
        for (int i = 0; i < medium.length; i++) {
            medium[i] = rand.nextInt(1024);
        }

        System.out.println("Built data " + sparse.length);

        final int dpoints = 10;
        int[] iterations = new int[dpoints];
        for (int i = 0; i < iterations.length; i++) {
            iterations[i] = 1 << dpoints + 3 - i;
        }

        for (int i = 0; i < 10; i++) {
            long[][] data = new long[8][dpoints];
            for (int dp = 0; dp < dpoints; dp++) {
                int[] input = null;
                input = Arrays.copyOf(sparse, sparse.length >>> dpoints - 1 - dp);
                data[0][dp] = processRL(input, iterations[dp]);
                data[3][dp] = process2S(input, iterations[dp]);
                input = Arrays.copyOf(freq, freq.length >>> dpoints - 1 - dp);
                data[1][dp] = processRL(input, iterations[dp]);
                data[4][dp] = process2S(input, iterations[dp]);
                input = Arrays.copyOf(medium, medium.length >>> dpoints - 1 - dp);
                data[2][dp] = processRL(input, iterations[dp]);
                data[5][dp] = process2S(input, iterations[dp]);
                data[6][dp] = input.length;
                data[7][dp] = iterations[dp];
            }
            String[] series = {"RLsparse", "RLfreq", "RLmedium", "2Ssparse", "2Sfreq", "2Smedium", "datasize", "iterations"};
            for (int s = 0; s < series.length; s++) {
                StringBuilder sb = new StringBuilder();
                sb.append(series[s]);
                for (long l : data[s]) {
                    if (s < 6) {
                        sb.append(",").append(String.format("%.3f", l/1000000.0));
                    } else {
                        sb.append(",").append(String.format("%d", l));
                    }
                }
                System.out.println(sb.toString());
            }
         }
    }

    private static long processRL(final int[] input, final int iterations) {
        System.gc();
        int cnt = 0;
        long nanos = System.nanoTime();
        for (int i = 0; i < iterations; i++) {
            cnt += findDuplicates(input).length;
        }
        if (cnt < 0) {
            throw new IllegalStateException("This is just to keep things from being optimized out.");
        }
        return System.nanoTime() - nanos;
    }

    private static long process2S(final int[] input, final int iterations) {
        System.gc();
        int cnt = 0;
        long nanos = System.nanoTime();
        for (int i = 0; i < iterations; i++) {
            cnt += findEvenOccurrences(input).size();
        }
        if (cnt < 0) {
            throw new IllegalStateException("This is just to keep things from being optimized out.");
        }
        return System.nanoTime() - nanos;
    }

}


How can I get this R code to pass the "1 minute" test for this Project Euler question? I am attempting to learn R programming by going through the questions in Project Euler.

The code below is my solution to problem 5 which asks for the smallest number that is evenly divisible by 1 through 20.

The code works but takes a very very long time.  Project Euler states that all of their problems should take a modern computer < 1 minute.

I know R is not as efficient as other languages but would it be possible to make my solution more efficient?

divis <- function(){
  count = 11
  tst <- 1:20 %% count # create vector of remainders
  while(sum(tst) != 0){ # test if all remainders are 0
    count <- count + 1
    tst <- count %% 1:20
    print(count)
    print(tst)
    }
  }
}

You should change the algorithm in order to make your program run faster. The while loop is performed over 200 million times (printing out information in every iteration) until it reaches the desired number.

If you calculate the answer using LCM (taking the least common multiple of all numbers between 1 and 20), the program should run instantly in pretty much any programming language.


Better code indentation I'm sure code indentation is a basic requirement for correct development. I have checked out may pages to learn more and results of my coding are below.

I like to share that here to receive opinions and suggestions.

<?php
include_once "../../config.php";

// Start
if (!isset($_SESSION)) session_start();
$level = 2;

if (!isset($_SESSION['user_id']) OR ($_SESSION['user_levell'] > $req_level)) {
    header("Location: $url/dashboard.php?error=1"); exit;
}

// Load permissions
$result = mysql_query("
    SELECT conf
    FROM adm_users
    WHERE user_id=".$_SESSION['user_id']
);
$row = mysql_fetch_array($result, MYSQL_BOTH);
    $access = $row['acess'];

if (($_SESSION['user_level'] != "1") && ($access_config != "1")) {
    header("Location: index.php?error=1"); exit;
}
// End

include_once "$url/includes/head.php";
include_once "$url/includes/menu.php";

// Load ser
$result = mysql_query("
    SELECT  *
    FROM    adm_ser
    WHERE   ser_id='$ser_id'
");

$row = mysql_fetch_array($result, MYSQL_BOTH);
    $ser_id = stripslashes($row['ser_id']);
    $ser_cod = stripslashes($row['ser_cod']);
    $ser_cd = stripslashes($row['ser_cnpj']);
    $ser_name = stripslashes($row['ser_name']);
    $ser_status_id = stripslashes($row['ser_status_id']);

// Dropdown Ser Status
$sql2 = mysql_query("
    SELECT *
    FROM adm_ser_status
    WHERE status_id=".$ser_status_id
);

while($row = mysql_fetch_array($sql2)) {
    $status_id_dropdown = $row['status_id'];
    $status_de_dropdown = $row['status_desc'];
}
?>

You don't need to indent your $ser_ value assignments.  Indentation should be left for within a class, method, loop, etc as you do for most of your other code.  I notice that you separate your MySQL statement, which can be done, but on such a simple query, really isn't needed as the only purpose for doing that is to help readability when it is a complicated query. ^^

On a side note, mysql_ functions are deprecated now, and you should switch to mysqli_ (or the MySQLi class), or (preferably) PDO.


Using one function to go from one element to another After I made a change to the html structure for a project, I found myself having to change many jquery selectors in many places.  To avoid this problem in the future I decided to make a simple function where I pass an element indicating where I am and a string indicating where I want to go.  This function should apply whatever logic is necessary to access the given endpoint.  I showed this code to someone and he didn't like it, he said I should use multiple functions or an object.  I thought that would make the code too much longer for little to no benefit, but I'm not really sure.  The code below is for this tool, which is a tree-based time-tracking app, so for example, "node" refers to a node in the tree.  The logic is not exhaustive as I only add code to the function on an as-needed basis.

// Access one particular element from another.
function from(start, end) {
  start = $(start)
  if(start.parent().hasClass('headline')) {
    var node = start.parent().parent()
    if(end == 'node')  return node
    else if(end == 'model')  return get_model(node.attr('model_id'))
  }
  if(end == 'headline')  return start.children('.headline')
  if(end == 'child_list')  return start.children('ol')
  if(end == 'each_child')  return start.find('> ol > .model')
  if(end == 'parent')  return start.parents('.model')
  if(end.indexOf('.') != 0)  end = '.' + end
  if(start.hasClass('headline'))  return $(start.children(end))
  return start.find('> .headline > ' + end)
}

The answer depends, for me, on many things. Some are:

is it a prototype or not? Looks like it is
are you in a rush? If so, do whatever works for you IF it's a prototype.
how big will the whole project be?

Here's a list of things that bother me:

Function name: from? Might make sense to you but it's not a very good name. It's too...generic. I'd take swissArmyKnifeFromElement over from any day - easy to remember, hard to confuse. If this method is somehow part of an object (like your Tree), then it might be OK for a private function.
The arguments might mean something else too - start and end suggest a range. It's potentially misleading. end can be many things (two, so far): a rule, like 'node' or 'child_list' or a class name. Again, misleading - it will only bring pain.

function findTreeElements(root, rule, params)
{
  //implementation
}

//you could call it like this
findTreeElements(a,'class',{'name':'someClassName'});
findTreeElements(a,'headline');

The more your project grows, the more complex this function will become - in ways that you cannot foresee :) Maybe that's why your friend was talking about objects and multiple functions. jQuery uses objects everywhere and I think that you should do that too:

define a tree/list object
define a task/node/whatever object
have the tree keep a list of tasks/nodes/etc
define the methods for manipulating the tree/node/whatever inside the associated objects

something like (totally random, possibly related, example):

var list = tree.findNode(someNodeNameOrIndex).tasks();

It's up to Tree object to manage its list of nodes and implement the findNode method (that parses the DOM whatever way you like).

It's up to the Node object (returned by Tree's findNode) to manage the list of tasks inside that node (or something similar) and implement the tasks function which would work in a similar way to your current find(elem, 'child_list').

Look at the code written by 'better' javascript developers and learn from them. There are many good projects out there that can teach you a lot. Look for decent sized projects, not 'plugins' or basic snippets; browser extensions are also good candidates.


Find max number of items that can fit in a given max-width Given a list of elements, each with own width, and a max-width.
I need to find how many elements can be fit inside that max-width, keeping the order of the elements.

Some examples:

elements: { width: 10 }, { width: 20 }
max-width: 30
result count should be 2, because both elements can be fit in this max-width
elements: { width: 10 }, { width: 20 }
max-width: 5
result count should be 0, because first element cannot be fit in this max-width
elements: { width: 10 }, { width: 20 }, { width: 5 }
max-width: 20
result count should be 1, because adding the second element exceed the max-width.

This piece of javascript code solve it, but I feel it is not readable enough:

function getNumberOfElementsInWidth(elements, maxWidth) {
  let accWidth = 0;
  let count = 0;
  for (let i = 0; i<elements.length; i++) {
    if (accWidth + elements[i].width <= maxWidth) {
      accWidth = accWidth + elements[i].width;
      count++;
    } else {
      break;
    }
  }
  return count;
}

function doSample() {
  const elements = [ { width: 10 }, { width: 12 }, { width: 15 } ];
  const maxWidth = 100;

  console.log(getNumberOfElementsInWidth(elements, maxWidth));
}

doSample();

I would appreciate your feedback on how to make the code for getNumberOfElementsInWidth function clearer.

I find your code clear enough, but you could improve it a little bit, so you only do the add operation once:

function getNumberOfElementsInWidth(elements, maxWidth) {
  let accWidth = 0;
  let count = 0;
  for (let i = 0; i < elements.length; i++) {
    accWidth += elements[i].width
    if (accWidth <= maxWidth) {
      count++;
    } else {
      break;
    }
  }
  return count;
}

Further you could reverse the conditional statement:

function getNumberOfElementsInWidth(elements, maxWidth) {
  let accWidth = 0;
  let count = 0;
  for (let i = 0; i < elements.length; i++) {
    accWidth += elements[i].width
    if (accWidth > maxWidth) {
      break;
    }

    count++;
  }
  return count;
}

In a more condensed version you could make the count variable also be the index:

function getNumberOfElementsInWidth(elements, maxWidth) {
  let accWidth = 0;

  for (var count = 0; count < elements.length; count++) {
    if ((accWidth += elements[count].width) > maxWidth) {
      break;
    }
  }
  return count;
}

Be aware that count must be declared as var count = 0 not let count = 0


Building a nested tree structure from an ordered list I have an ordered list in this format. The data is ordered by path so that the list will never contain a child before the parent in the list.

id, path
1, null
2, null
3, 1
4, 1-3
5, 2
6, 2-5

Each entry contains the path it needs to place itself in the tree structure.

The catch is that the list doesn't always start with nulls sometimes the list starts in the middle of a path.

id, path
10, 2-5-6
11, 2-5-6-10
12, 2-5-6-10
13, 2-5-6-10-11
14, 2-5-6-10-11-13
15, 2-5-6-10-12

In this case I need to treat the first record in this list as the root and build out everything else as normal without permanently editing the paths of any items in the list.

    function nestComments(comments) {
    var nested = {
        "Comments": []
    };
    if (!comments) return nested;

    var normalize,
        path,
        level;

    for (var i = 0; i < comments.length; i++) {

        var comment = comments[i],
            tree = nested,
            isValid = true;

        if (comment.path) {
            path = comment.path.split('-');

            //permalinked comments don't have null paths
            //keep track of the permalink the path
            //adjust children paths to treat permalink path like the root

            //if the first comment has a path then we need to normalize the path
            if (i === 0) {
                normalize = path.length;
            }

            if (normalize) {
                //normalize the path based on the peramlink's path
                path.splice(0, normalize);
            }

            for (var j = 0; j < path.length; j++) {
                var step = path[j];

                var index = tree.Comments.findIndex(x => x.comment_id.toString() === step);
                if (index < 0) {
                    isValid = false;
                    break;
                }
                tree = tree.Comments[index];

                tree.Comments = tree.Comments || [];
            }

        }

        if (!path || path.length === 0) {
            level = 0;
        } else {
            level = path.findIndex(x => x === path[path.length - 1]);
        }

        if (!trimComments(level, tree) || !isValid) {
            continue;
        }

        tree.Comments.push(comment);

    }
    return nested;
}

function trimComments(level, tree) {
    var result;
    switch (level) {
        case 0:
            result = true; // limit based on the sql query
            break;
        case 1:
            result = tree.Comments.length < 5;
            break;
        case 2:
            result = tree.Comments.length < 5;
            break;
        case 3:
            result = tree.Comments.length < 3;
            break;
        case 4:
            result = tree.Comments.length < 2;
            break;
        case 5:
            result = tree.Comments.length < 1;
            break;
        case 6:
            result = tree.Comments.length < 1;
            break;
        default:
            result = false;
            break;
    }
    return result;
}

var comments = nestComments([{comment_id: 4, path:"1-2-3"},{comment_id: 5, path:"1-2-3-4"},{comment_id: 6,path:"1-2-3-4"}, {comment_id: 7, path:"1-2-3-4-6"},{comment_id: 8, path:"1-2-3-4-6"}, {comment_id: 9, path:"1-2-3-4-6-7"}, {comment_id: 10, path:"1-2-3-4-6-7-9"}, {comment_id: 11, path:"1-2-3-4-6-8"}]);

console.log(comments);

Please help me improve this code.

First, nestComments appears very long. Functions that usually do too much are also functions that become harder to test. Also, code that is is visually long is usually hard to understand. Consider splitting it off into smaller, manageable functions.

Now one thing that makes code slow is when you nest loops. In this case, you have 3 levels, 2 for loops and findIndex which is essentially a loop. Nesting loops can cause operations to explode exponentially. What you'd want to do is keep it to a minimum, since some operations really require nested loops.

Next would be trimComments. The problem of having a switch (or any code that does comparison against a hard set of items) is that you have to play catch-up with the things you need to compare. You will constantly dive into code to add things you never wrote in the first place.

Also, your trimComments is just sending in a value and expecting a result that's pretty 1:1, reminiscent of a key-value pair. This can easily be done with regular objects instead of a switch statement. You can find the input from the keys, and get its corresponding value.

Looking for how trimComments is used, I see that you use it to determine if a comment is included. Ok, cool, you use it to skip a push. But the problem is that you've already done processing, then just suddenly skip it from being included. That's a waste of operations. Also, you can preprocess your paths so that you only include the intended maximum number of comment paths, and operate on that instead.

Here's my take on it:

Normalize your input by working on full paths.
Trim down the paths based on maximums.
Operate on the remaining paths.

And the code would be broken down into:

const commentsPerLevel = {
  '1': 5
  '2': 5
  '3': 3
  '4': 2
  '5': 1
  '6': 1
}

function getMaxForLevel(level){
  // returns value from commentsPerLevel or a default value
  return commentsPerLevel[level] || 0;
}

function getCommentPaths(comments){
  // returns full paths from input
  return comments.map(comment => `0-${comment.path}-${comment.comment_id}`);
}

function trimPaths(paths){
  // returns paths trimmed based on maximums from commentsPerLevel
}

function buildCommentTree(paths){
  // build comment tree based on paths
}

function nestComments(comments){
  const paths        = getCommentPaths(comments);
  const trimmedPaths = trimPaths(paths);
  const commentTree  = buildCommentTree(trimmedPaths);
  return commentTree;
}

I left out implementation for some, but the idea is to break down your operations into functions so they won't be that hard to maintain.


Complex sorting of nested data I am retrieving data from Postgres (jsonb type) and I need to return an OrderedDict that has a predictable order for human and machine consumption.  There are some common(ish) keys that should be used to direct precedence of values of common types (based on a predefined order and if sort_order is defined).  Otherwise, sort order should fall back to key based lexicographic ordering.

The general intent is to have a predictable, 'sane', represenation of composite dicts.

The basic algorithm is:

Dicts come before lists
Values that are NOT iterables or mapping take precedence over objects that are.
Values of the same type whose keys are not in sort_order are considered equal and should be sorted lexicographically.
Obj A takes precedence over Obj B if type(A[0]) == type(B)  and [0] in sort_order not B[0] in sort_order
if all([type(A[1]) == type(B[1]), A[0] in sort_order, B[0] in sort_order]) then the index position of the object key is the precedence determinant.

I have attempted several implementations, but I have not been able to come up with anything that I would consider pythonic/elegant.

Here is the latest incarnation:

# -*- coding: utf-8 -*-

import json
from collections import OrderedDict

def dict_sort(obj, sort_order=None):
    def seq(s, o=None, v=None):
        return str(s) + str(o) + str(v) if o is not None else str(s)

    order_seq = None
    if sort_order is not None and obj[0] in sort_order:
        order_seq = [i for i, v in enumerate(sort_order) if v == obj[0]][0]

    if isinstance(obj[1], dict):
        return seq(2, order_seq, obj[0]) if order_seq else seq(3)
    elif isinstance(obj[1], list):
        return seq(4, order_seq, obj[0]) if order_seq else seq(5)
    else:
        return seq(0, order_seq, obj[0]) if order_seq else seq(1)

def comp_sort(obj, sort_order=None):
    data = OrderedDict()
    if isinstance(obj, dict):
        for key, value in sorted(obj.items(), key=lambda d: dict_sort(d, sort_order)):
            if isinstance(value, dict) or isinstance(value, list):
                data[key] = comp_sort(value, sort_order)
            else:
                data[key] = value
    elif isinstance(obj, list):
        try:
            return sorted(obj)
        except:
            items = []
            for value in obj:
                if isinstance(value, dict) or isinstance(value, list):
                    items.append(comp_sort(value, sort_order))
                else:
                    items.append(value)
            return items
    return data

# thx herk

Here is a sample data set.

You can use a list comprehension to avoid verbosity:

        items = []
        for value in obj:
            if isinstance(value, dict) or isinstance(value, list):
                items.append(comp_sort(value, sort_order))
            else:
                items.append(value)
        return items

Becomes:

is_dict_or_list = isinstance(value, dict) or isinstance(value, list)
return [comp_sort(value, sort_order) if is_dict_or_list else value
               for value in obj ]

Remember to specify:

except ExpectedError:

And not

except:

The latter will swallow anything and harden debugging.