Simple Rock, Paper, Scissors in Python I have looked at many different approaches to this game online, but as someone who isn't very experienced with Python, I may not be aware of what others are doing right.

import random

def RPS():
    print("You are playing Rock Paper Scisscors")
    comp_possible  = 1,2,3
    score = [0,0]
    flag = 0
    while True:
        print("Enter your choice:")
        while True:
            choice = input('->')
            if choice == 'r' or choice == 'R' or choice == 'Rock' or choice == 'rock' or choice == '1':
                choice_identifier  = 1
                break
            elif choice == 'S' or choice == 's' or choice == 'Scissors' or choice == 'sciccors' or choice == '2':
                choice_identifier =  2
                break
            elif choice == 'P' or choice == 'p' or choice == 'Paper' or choice == 'paper' or choice == '3':
                choice_identifier =  3
                break
            else:
                print('That\'s not an option in this game :)')
                print('Try again:')
                continue

        comp_choice = random.choice(comp_possible)
        if choice_identifier == comp_choice:
            print('It\'s a draw!')
            score[0] = score[0] + 1
            score[1] = score[1] + 1
        elif (choice_identifier == 1 and comp_choice == 2) or (choice_identifier == 2 and comp_choice == 3) or (choice_identifier == 3 and comp_choice == 1):
            print('You win!')
            score[0] = score[0] + 1
        else:
            print('You lose...')
            score[1] = score[1] + 1

        while True:
            answer = input('Play another round?')
            if answer == 'y' or answer == 'Y' or answer == 'yes' or answer == 'Yes' or answer == 'ye' or answer == 'Ye' or answer == 'sure' or answer == 'Sure':
                print(' Current score: You - ',score[0],' Computer - ',  score[1])
                flag = 0
                break
            elif answer == 'n' or answer == 'N' or answer == 'no' or answer == 'No' or answer == 'nah' or answer == 'Nah':
                print('Thanks for playing! Final score: You - ',score[0],' Computer - ',  score[1])
                flag = 1
                break
            else:
                print('Yay or nay...')
                continue
        if flag == 0:
            continue
        else:
            break

RPS()

What things in my code are, for example, horribly inefficient or are bad practices?

The game "Rock Paper Scissors" can be specified in terms of states.
Specification
The game is played by two players, playerA and playerB. Each player selects from among a set of three options {null, rock, paper, scissors}. null is used to represent the state before a player has chosen. Using an ordered pair (playerA_choice, playerB_choice) creates the possible game states:
 (null,    rock)
 (null,    paper)
 (null,    scissors)
 (rock,    null)
 (rock,    rock)
 (rock,    paper)
 (rock,    scissors)
 (paper,   null)
 (paper,   rock)
 (paper,   paper)
 (paper,   scissors)
 (scissors, null)
 (scissors, rock)
 (scissors, paper)
 (scissors, scissors)

There are three final states and three transition functions to them:
playerA_wins = (rock, scissors) | (paper, rock) | (scissors, paper)
playerB_wins = (rock, paper) | (paper, scissors) | (scissors, rock)
draw = (rock, rock) | (paper, paper) | (scissors, scissors)

The start state is:
 (null, null)

The states:
 (null,    rock)
 (null,    paper)
 (null,    scissors)
 (rock,    null)
 (paper,   null)
 (scissors, null)

are blocking and require further input from one of the players.
Implementing The Specification
This is a sketch of the game:
# Some Useful Names
null = "null"
rock = "rock"
paper = "paper"
scissors = "scissors"

# A Thesaurus (implemented as a dictionary)
synonyms = {"rock": rock,
            "paper": paper,
            "scissors": scissors,
            "stone": rock,
            "vellum": paper,
            "shears": scissors}

# Final States
game_is_draw = "Game is a Draw"

playerA_wins = "Player A Wins"

playerB_wins =  "Player B Wins"

# Initial State
both_players_must_choose = "Both Players Must Choose"

# Transition States
playerA_must_choose = "Player A Must Choose"

playerB_must_choose = "Player B Must Choose"

# Transition Table (implemented as a dictionary)
transitions = {(null,    null): both_players_must_choose,
               (null,    rock): playerA_must_choose,
               (null,    paper): playerA_must_choose,
               (null,    scissors): playerA_must_choose,
               (rock,    null): playerB_must_choose,
               (rock,    rock): game_is_draw,
               (rock,    paper): playerB_wins,
               (rock,    scissors): playerA_wins,
               (paper,   null): playerB_must_choose,
               (paper,   rock): playerA_wins,
               (paper,   paper): game_is_draw,
               (paper,   scissors): playerB_wins,
               (scissors, null): playerB_must_choose,
               (scissors, rock): playerB_wins,
               (scissors, paper): playerA_wins,
               (scissors, scissors): game_is_draw}

# Simulate Initialization
playerA_choice = null
playerB_choice = null

# Simulate Players Choosing
playerA_choice = synonyms["stone"]
playerB_choice = synonyms["shears"]

# Main Logic
state = (playerA_choice, playerB_choice)
print(outcomes[state])

Details to handle input output should be at a higher layer of abstraction. It shouldn't matter to the game engine if the game is between a human and a computer, two humans, or two computers. It shouldn't matter if it is being played using a laptop or over the internet.
Data Structures
A good rule of thumb is to replace complex logic with a data structure, and
if choice == 'r' or choice == 'R' or choice == 'Rock' or choice == 'rock' or choice == '1':

is the sort of code that is hard to understand and hard to maintain. Perhaps to the point where it is better to forgo the user friendly approach?  Nay! A thesaurus is a good place to look for synonyms. Although, Python lacks Thesauri, a dictionary will probably do. Now the game can be sold at Ye Local Renaissance Faire as Stone, Vellum, Shears!
Indeed, a dictionary is a good way to map each possible game state to the next state. The code does in lieu of directly implementing the logic the specification uses to describe the final states.
The reason is maintainability. The mobile version of the game will offer an upgrade to Rock, Paper, Scissors, Spock, Lizard currently in development. Once we get around to updating the dictionaries synonyms and transitions the game will be done, profits will role in, and we will never have to work again unless sleeping on a stack of money counts as work.
But Really, Why All the Ceremony?
One of the real values that comes from using dictionaries is that a dictionary can be used to dispatch functions.
# Abbreviated Rock Paper Scissors

# Some Useful Names
rock = "rock"
paper = "paper"

def playerA_wins():
    print("Player A Wins")

def playerB_wins():
    print("Player B Wins")

transitions = {(paper, rock): playerA_wins,
               (rock, paper): playerB_wins}

transitions[(paper, rock)]()
transitions[(rock, paper)]()

Getting user input can be dispatched similarly by the dictionary. However, the initial (null, null) state, threading might be justified so that playerA_choice() doesn't block playerB_choice() or vice versa...that's IO for 'ya.
Recommendations

Better variable names. Write about rocks and paper and scissors: rock = 1 makes the code more readable. Readability particularly helps the person writing the code.
Make the code more modular. Put all the initialization together. Put all the user interface someplace else. Separate out the string matching. Only put high level abstractions in the main loop.
Consider specifying the problem before writing code for a solution. At the core Rock, Paper, Scissors does not deal with synonyms. Writing a specification makes it clear that synonyms are a feature and encourages keeping their logic outside the main loop. This makes the code more readable.


Hungry ant AI: any food here? I'm working on a little AI to simulate ants (very basic). The full code can be found on Github. This function (in Ants.py) takes a list of Tuples containing coordinates and tells whether or not the ant is over one of them.

def Got_Food(self,all_foods):
    foods = []
    x,y = self.location[0],self.location[1]
    c_x,c_y = self.size[0],self.size[1]
    for food in all_foods:
          if food[1] >= y-c_y and food[1] <= y+c_y \
          and food[0] >= c_x and food[0] <=x+c_x:
            if food[1] > y and food[1] <y+c_y\
            and food[0] > x and food[0] <x+c_x:
                foods.append((food[0],food[1]))
            elif food[1] < y and food[1] >y-c_y\
            and food[0]  > x and food[0] >x-c_y:
                foods.append((food[0],food[1]))
          if food[0] >= x-c_x and food[0] <= x+c_x \
          and food[1] >= y and food[1] <=y+c_y:
                foods.append((food[0],food[1]))
    if len(foods) == 0:
        return None
    return foods

This function works exactly as I want, but it is probably one of the ugliest I've ever seen or written. Does anyone know a way around all these nested loops?

Disclaimer: Current version of the code seems to be very weirdly indented. Some comments might be wrong because of some misinterpretation of your code.

Style

Python has a style guide called PEP 8. It is usually a good idea to try to stick to it unless you have good reasons not to. In any case, it is definitly worth the read. You'll find various tools to check compliancy. In your case, the main issues would be : naming (snake_case is recommended for function names), spacing (whitespace around operators), indentation (4 spaces)

Design

Your function returns either a list of food or None is there is no food. This seems to be more complicated than it should be: it could return a list of food (empty is there is no food). It makes your function easier to use, easier to explain and more concise: you can simply remove the if len(foods) == 0: condition.

From the Zen of Python:

  Special cases aren't special enough to break the rules.

In any case, your function deserves some documentation.

Tuple unpacking

One of my favorite feature of Python is iterable unpacking. You can do something like:

a, b, c = my_list_with_3_elements

and have easy access to the different elements (and an exception if the list has the wrong number of elements).

In your case, assuming location and size have the wrong length, you could simply write:

 x, y = self.location
 c_x, c_y = self.size

This is very convenient to avoid long expressions with error-prones bracket access to a particular index.

Going further, you could do the same with food[0] and food[1].

Now, (assuming I haven't broken too many things) looks like:

def got_food(self, all_foods):
   foods = []
   x, y = self.location
   c_x, c_y = self.size
   for f_x, f_y in all_foods:
      if f_y >= y - c_y and f_y <= y + c_y and f_x >= c_x and f_x <= x + c_x:
          if f_y > y and f_y < y + c_y and f_x > x and f_x < x + c_x:
              foods.append((f_x, f_y))
          elif f_y < y and f_y > y - c_y and f_x  > x and f_x > x - c_y:
              foods.append((f_x, f_y))
      if f_x >= x - c_x and f_x <= x + c_x and f_y >= y and f_y <= y + c_y:
            foods.append((f_x, f_y))
  return foods

Chained comparisons

From the doc:

  Comparisons can be chained arbitrarily, e.g., x < y <= z is equivalent to x < y and y <= z, except that y is evaluated only once (but in both cases z is not evaluated at all when x < y is found to be false).

In your case, many things can be rewritten:

def got_food(self, all_foods):
   foods = []
   x, y = self.location
   c_x, c_y = self.size
   for f_x, f_y in all_foods:
      if y - c_y <= f_y <= y + c_y and c_x <= f_x <= x + c_x:
          if y < f_y < y + c_y and x < f_x < x + c_x:
              foods.append((f_x, f_y))
          elif y > f_y > y - c_y and f_x > x and f_x > x - c_y:
              foods.append((f_x, f_y))
      if x - c_x <= f_x <= x + c_x and y <= f_y <= y + c_y:
            foods.append((f_x, f_y))
  return foods

I do not understand enough of the intent to go any further (also I suspect the condition f_x > x is wrong).


Codewars: Sum of Pairs I've been working on this task (see screenshot).

Here's my answer to it so far:

var sum_pairs=function(ints, s){
  //your code here

  var ptr_1 = 0,
      ptr_2 = 0,
      i = 0,
      j = 0,
      len = ints.length,
      j_min = len,
      arr_sum = [];

  for (i; i < len-1; i++){
    for (j = i+1; j < len; j++){
      if(ints[i] + ints[j] === s){
        if (j < j_min){
          j_min = j;
          arr_sum = [parseInt(ints[i]), parseInt(ints[j])]
        }
      }
    }
  }

return ( (arr_sum.length === 0)? undefined : arr_sum );

}

However, while the tests all pass, it throws an STDERR about 'optimising the code':

How might I be able to optimise my code further?

Straightforward enumeration algorithms utilizing 2-layered loops will always take a lot of time on extremely large arrays where a possibility exists that the first match is achieved by elements near the start/end of the array (e.g. indices 515 and 9123456 of a 10 million elements array), which would lead to the same excessive number of iterations as the original code performed.

Here's an updated code that performs much faster on the average. The idea is to build a map of unique values and their minimal indices in the source array, and simultaneously check if there's an entry for the complementary number (addend) already.

var sum_pairs = function(ints, s) {
    var map = {},
        pair, pairMaxIndex = ints.length - 1;

    for (var i = 0; i <= pairMaxIndex; i++) {
        var a = ints[i];
        var b = s - a;
        var j = map[b];
        if (j !== undefined && i <= pairMaxIndex && j <= pairMaxIndex) {
            pairMaxIndex = i > j ? i : j;
            pair = i < j ? [a, b] : [b, a];
        }
        var tmp = map[a];
        if (tmp === undefined || i < tmp) {
            map[a] = i;
        }
    }
    return pair;
};

  10 million randoms in -100..100 range: 1ms - 1000ms
  10 million randoms in -1,000,000..1,000,000 range: 1ms - 5000ms
  10 million randoms in -10,000,000..10,000,000 range: 1ms - crash
  1 million randoms in -1,000,000,000..1,000,000,000 range: 1ms - 2000ms

The worst time in each case was caused by artificially crafting an array described above.
The crash in the 3rd test occured because the number of unique values neared 1G and the object exceeded 32-bit Javascript engine's memory limit of 2 GB. All tests were run on i7 CPU.

The code above incorporates the following optimizations as well:

Flambino's answer shows there's no need to continue enumeration if current index is bigger than the matching values' max index found already.
Jodrell's answer shows it's possible to do everything in just one loop, pointed out by Jonah.

And the tests:

console.clear();
var arr = [], max = 1000000;
for (var i = 0; i<10000000; i++) arr[i] = Math.random() * max * 2 - max |0;
console.log('array of', arr.length, 'elements is built');

console.time(1);
console.log(sum_pairs(arr, 1000));
console.timeEnd(1);


Bash script to batch rename files This is my homebrew bash script for batch renaming files extensions (my music library needs organizing):

#!/bin/bash

# rext.sh
# Syntax: rext.sh EXT1 EXT2
# Batch rename any file in current directory with extension of EXT1 to EXT2

EXT1=$1
EXT2=$2

for f in *.$EXT1
do
  fname=${f%.*}
  newfname="$fname.$EXT2"
  mv -v -i "$f" "$newfname"
done

It works beautifully with my music files, but is this code sufficient or can I still improve on it? Also I've seen examples using rename instead of mv, so is there any advantage in using rename?

A tiny mistake is not quoting $EXT1 in for f in *.$EXT1.
If it contains a space, the script won't work as expected.
I agree it's an unrealistic corner case,
but it's easy enough to protect against with proper quoting: for f in *."$EXT1".

The rename Perl script is famous for this purpose.
Since you asked about its advantages:

Stood the test of time
You don't have to write it
It has more features (full regex support)

The equivalent usage to simulate the behavior of your script would be:

rename -v 's/EXT1$/EXT2/' *.EXT1

A feature I like is the -n flag for dry run, to verify the renames that would be performed before actually performing them.


CommandLineToArgV Unescaping Trying to implement something like CommandLineToArgV:

template<typename InIter, typename OutIter>
inline InIter CmdLineToArgvWUnescape(InIter begin, InIter end, OutIter target)
{
    if (std::distance(begin, end) < 2 || *begin != L'"')
    {
        // ""s are required
        throw MalformedEscapedSequence();
    }
    ++begin; //Skip "
    std::size_t backslashCount = 0;
    for(; begin != end; ++begin)
    {
        switch(*begin)
        {
        case L'\\':
            backslashCount++;
            break;
        case L'"':
            if (backslashCount)
            {
                std::fill_n(target, backslashCount / 2, L'\\');
                *target++ = L'"';
                backslashCount = 0;
            }
            else
            {
                return ++begin;
            }
        default:
            if (backslashCount)
            {
                std::fill_n(target, backslashCount, L'\\');
                backslashCount = 0;
            }
            *target++ = *begin;
        }
    }
    throw MalformedEscapedSequence();
}

My one comment is add a comment on exactly what it is doing.

Even after studying I am not 100% sure what it does (I would need some unit tests to feel I understood it).

What I think you are trying to achieve:

Convert \" into "
Convert \\\" into \"


Grouping ZIP codes into ranges There is a clear pattern as to what the number values would be. And this has a starting interval and known start and end value. Though the end may change, it seems simpler to set the values and do some magical loop. Any better formats would be appreciated. The sets are what I would have gone with if I could have gotten the loop to work correctly.

<?php
$setStart =5000;
$setEnd = 95000;
$setInterval = 5000;

    //Enter your code here, enjoy!

    foreach($zips as $zipcode) {
    $zip = $zipcode['zip'];
    $zipInt = intval($zipcode['zip']);
    $org_ids = $zipcode['orgs'];
    $org_ids = explode(",",$org_ids);
    if (!empty($org_ids)) {
        if ($zipInt < 5000) {
            $zips00_array[$zip] = $org_ids;
        }
        if ($zipInt >= 5000 && $zipInt < 10000) {
            $zips05_array[$zip] = $org_ids;
        }
        if ($zipInt >= 10000 && $zipInt < 15000) {
            $zips10_array[$zip] = $org_ids;
        }
        if ($zipInt >= 15000 && $zipInt < 20000) {
            $zips15_array[$zip] = $org_ids;
        }
        if ($zipInt >= 20000 && $zipInt < 25000) {
            $zips20_array[$zip] = $org_ids;
        }
        if ($zipInt >= 25000 && $zipInt< 30000) {
            $zips25_array[$zip] = $org_ids;
        }
        if ($zipInt >= 30000 && $zipInt < 35000) {
            $zips30_array[$zip] = $org_ids;
        }
        if ($zipInt >= 35000 && $zipInt < 40000) {
            $zips35_array[$zip] = $org_ids;
        }
        if ($zipInt >= 40000 && $zipInt < 45000) {
            $zips40_array[$zip] = $org_ids;
        }
        if ($zipInt >= 45000 && $zipInt < 50000) {
            $zips45_array[$zip] = $org_ids;
        }
        if ($zipInt >= 50000 && $zipInt < 55000) {
            $zips50_array[$zip] = $org_ids;
        }
        if ($zipInt >= 55000 && $zipInt < 60000) {
            $zips55_array[$zip] = $org_ids;
        }
        if ($zipInt >= 60000 && $zipInt < 65000) {
            $zips60_array[$zip] = $org_ids;
        }
        if ($zipInt >= 65000 && $zipInt < 70000) {
            $zips65_array[$zip] = $org_ids;
        }
        if ($zipInt >= 70000 && $zipInt < 75000) {
            $zips70_array[$zip] = $org_ids;
        }
        if ($zipInt >= 75000 && $zipInt < 80000) {
            $zips75_array[$zip] = $org_ids;
        }
        if ($zipInt >= 80000 && $zipInt < 85000) {
            $zips80_array[$zip] = $org_ids;
        }
        if ($zipInt >= 85000 && $zipInt < 90000) {
            $zips85_array[$zip] = $org_ids;
        }
        if ($zipInt >= 90000 && $zipInt < 95000) {
            $zips90_array[$zip] = $org_ids;
        }
        if ($zipInt >= 95000) {
            $zips95_array[$zip] = $org_ids;
        }
    }
}

I think you could use a one-liner like:

$zip_arr_id = floor(($zipInt/1000)/5 )*5;

which could correspond to the $zips${$zip_arr_id}_array (unsure if this is exactly right) that you're saving the org ID's to. Look into variable variables in php.

Riffing on this idea could save you a lot of lines in your code.


Pipe based multiprocess filter This code is a multi-threaded version of filter that uses pipes to communicate between the workers and the main thread. I am looking for any ways to make this code cleaner. I'm also interested in if there is anything I can do to make this play nicely with infinite iterators.

from multiprocessing import Process, Pipe
from itertools import cycle, count

def distributer(f, data):
    def worker(pipe):
        while True:
            piece = pipe.recv()
            if f(piece):
                pipe.send(piece)
            pipe.send(False)

    pipes = []
    for _ in range(8):
        parent_conn, child_conn = Pipe()
        p = Process(target=worker, args=(child_conn,))
        pipes.append(parent_conn)
        p.start()
        parent_conn.send(next(data))

    for pipe in cycle(pipes):
        if pipe.poll(.0001):
            ans = pipe.recv()
            if ans:
                yield ans
            pipe.send(next(data))

if __name__ == '__main__':
    def g(x):
        return x%1000==0
    data = count(1)
    for ans in distributer(g, data):
        print(ans)

I recommend a signature of:

def distributer(f, data, num_workers=8):
...
    for _ in range(num_workers):

Perhaps f is the appropriate identifier. But it leaves me scratching my head a bit. I wouldn't mind a docstring explaining the function's responsibility.

    p = Process(target=worker, args=(child_conn,))

Ordinarily this would be perfect. But given that you're also manipulating pipes, it wouldn't hurt to assign to proc instead.

Your prose suggests that you're filtering, in this case picking out one value in a thousand. Perhaps the appropriate function name would have been filter_worker().

Phrasing it as "reduce" may have been clearer than "filter". For example, every thousandth row return sum of previous rows, that sort of thing, if the code is strictly for illustrative purposes.

In summary, as a reader I'm looking for a comment / docstring that explains what is interesting about the top level code doing busy work forever (I see no benchmark timings), and I'm interested in a docstring on worker() explaining the protocol it implements.


Am I managing my memory correctly? I read that if an object is made using the init it must be released, but if it's something like this  elementFormula = [[NSMutableString  stringWithString:@""] it is autoreleased. However, there are some other aspects of memory management that confuse me.

For example, something like [[NSMutableString  stringWithString:@""] retain]  If I retain an autoreleased object, do I have to release it myself, or is it still autoreleased? Also, if I declare a pointer to an object in the .h and assign it a value in the .m like so     completeFormula = [self synthesizeFormula]; without going through the init process, do I have to release completeFormula?

Those are my main concerns, but I have posted the entire code below if needed.

@interface EquationTextField : UIView <KeyInput> {

FormulaKeyboard *keyboard;
int consecutiveElementCount;
int subscriptLength;
int chargeIndex;
NSMutableString *elementFormula;
NSString *lastElementPressed;
NSString *charge;
NSString *state;
NSString *completeFormula;
}

init {

      NSArray *bundle = [[NSBundle mainBundle] loadNibNamed:@"FormulaKeyboard"     owner:self options:nil];

    for (id object in bundle) {
        if ([object isKindOfClass:[FormulaKeyboard class]])
            keyboard = (FormulaKeyboard *)object;
    }

    self.inputView = keyboard;
    keyboard.delegate = self;

    elementFormula = [[NSMutableString  stringWithString:@""] retain];
    charge = [[NSString alloc] initWithString:@""];
    state = [[NSString alloc] initWithString:@""];

}

 - (void)addElement:(NSString *)currentElement {

if (![currentElement isEqualToString:lastElementPressed])  {

    // when new element is pressed

    [elementFormula appendString:currentElement];
    lastElementPressed = currentElement;
    consecutiveElementCount = 1;

}

else if ([currentElement isEqualToString:lastElementPressed]) {

    // when element is pressed consecutively

    if (consecutiveElementCount == 1) {

        // since there is no '1' subscript, nothing needs to be deleted

        [elementFormula appendString:@"₂"];
        subscriptLength = 1;

    }

    if (consecutiveElementCount > 1) {

        // deletes last subscript and replaces it with new subscript

        NSArray *subscriptList = [[[NSArray alloc] initWithObjects:@"₂", @"₃", @"₄", @"₅", @"₆", @"₇", @"₈", @"₉", @"₁₀", @"₁₁", @"₁₂", nil] autorelease];

        NSRange range = NSMakeRange (([elementFormula length] - subscriptLength), subscriptLength);

        [elementFormula deleteCharactersInRange:range];

        NSString *tempSubscript = [subscriptList objectAtIndex:consecutiveElementCount - 1];
        [elementFormula appendString:tempSubscript];

        subscriptLength = [tempSubscript length];

    }

    if (consecutiveElementCount < 11)
        consecutiveElementCount ++;

}

completeFormula = [self synthesizeFormula];

 }

- (void)changeCharge:(NSString *)chargeIncrease {

NSArray *chargeList = [[[NSArray alloc] initWithObjects:@"⁴⁻", @"³⁻", @"²⁻", @"⁻", @"", @"⁺", @"²⁺", @"³⁺", @"⁴⁺", nil] autorelease];

if ([chargeIncrease isEqualToString:@"+"]) {

    chargeIndex += 1;
    charge = [chargeList objectAtIndex:chargeIndex];

}

else if ([chargeIncrease isEqualToString:@"-"]) {

    chargeIndex -= 1;
    charge = [chargeList objectAtIndex:chargeIndex];

}

completeFormula = [self synthesizeFormula];

}

- (void) changeState:(NSString *)stateName {

state = stateName;

completeFormula = [self synthesizeFormula];

}

- (NSString *)synthesizeFormula {

NSMutableString *synthesizedFormula = [[[NSMutableString alloc] initWithString:elementFormula] autorelease];
[synthesizedFormula appendString:charge];
[synthesizedFormula appendString:state];

return synthesizedFormula;
}

- (void)dealloc
{
[charge release];
[state release];
self.inputView = nil;
keyboard.delegate = nil;
[keyboard release];
[super dealloc];
}

It's not init that causes you to have to be responsible for releasing the memory its alloc. If you call alloc, retain or new (this is rarely used these days) then you are responsible for releasing the memory yourself.

So in the case where you had an autoreleased object and you retained it. Then yes you would then be responsible for subsequently releasing it.

The most common side affect where you can be responsible for releasing an object you didn't explicitly call alloc or retain on is with a property that has the retain attribute.

@property (retain) NSString *string;

if you then have the following code somewhere

self.string = [NSString stringWithString:@"This is an autoreleased string typically"];

calling the self.string piece will cause it to retain the value you are setting it with. So you would want to make sure and have the following to clean it up, probably in your dealloc.

self.string = nil;


class MUserNew | Add DRY, remove constants to mysql defaults Note, I choose not to use the Factory Pattern out of preference.

This is a ( M ) odel class in the MVC pattern.

Private functions return Booleans and public functions return strings which I then output to the browser from the Controller.  The class returns a string from the invoke call.

Here is the only call to the class:

Call

$model = new UserNew( new IUniversals() , new IDatabase(), new IText(), new IMessage() );
echo $model->invoke();

Class

class MUserNew extends MUser // set mysql defaults
{
    private $universal_object;
    private $text_object;
    private $message_object;
    private $input_fields = array( 'name', 'email', 'pass' );

    public function __construct( $universal_object, $text_object, $message_object  )
    {
        parent::__construct();
        $this->universal_object = $universal_object;
        $this->text_object = $text_object;
        $this->message_object = $message_object;
    }
    public function invoke()
    {
        $not_validated = $this->checkInput();
        if( $this->universal_object->get( 'load' ) && ( $not_validated !== true ) )
        {
            return $not_validated;
        }
        else
        {
            $email_is_unique = $this->checkEmail();
            if( ( $email_is_unique === true ) )
            {
                return $this->message_object->get( 'PASS' ) . $this->insertRecord() ;
            }
            else
            {
                return $email_is_unique;
            }
        }
    }
    private function checkInput()
    {
        if( !$this->text_object->checkEmpty() )
        {
            return $this->text_object->get( 'empty' );
        }
        foreach( $this->input_fields AS $pattern )
        {
            if( !$this->text_object->checkPattern( $pattern ) )
            {
                return $this->message_object->get( $pattern );
            }
        }
        return true;
    }
    private function checkEmail()
    {
        if( $this->database_object->_pdoQuery( 'single', 'signup_check', array($this->text_object->get('email') ) ) )
        {
            return $this->$message_object->get( 'taken' );
        }
        else
        {
            return true;
        }
    }
    private function insertRecord()
    {
        srand( time() );
        $hash = crypt( $this->text_object->get( 'pass' ), substr( strval( rand() ), -2 ) );
        $this->text_object->set('pass', $hash );
        $this->database_object->_pdoQuery( 'single', 'signup_insert', array( '', $this->text_object->get('name'), $this->text_object->get('email'), $this->text_object->get('pass'), 0, 0 ) );
        $this->database_object->_pdoQuery( 'none', 'bookmark_insert', array($result['id'], 'Facebook', 'http://www.facebook.com', 'facebook.com', 'social', 'http://static.ak.fbcdn.net/rsrc.php/yi/r/q9U99v3_saj.ico'));
        return $hash;
    }

}

Where do I begin?

Simplest first. Make sure you assign your constructor a state (public, private, or protected). Most likely public.

Now... Where are all these extra properties coming from? $ISharedObject is not even remotely similar to anything else, so it isn't a typo. And its not defined as a class property, so I can't even be sure this is supposed to be here. The class you provide does not extend a parent class, so it isn't from there. It just appeared in invoke() out of no where. The method it uses suggests that it is most likely a $MessageObject but I don't know.

It looks like you've either copy-pasted this all together, or had a collaboration with someone else and you've crossed your naming schemes. $MessageObject and $TextObject looks like they should be $message_object and $text_object respectively. They should be renamed as such or vice-versa. It is probably easier to rename where they are defined, less to change, but up to you.

You have a $not_validated and $validated variable, of which only the first was defined.

If checkEmail() only ever returns a boolean, like its name suggests, then there is no need for absolute equality operators "===". Just if( $email_is_unique ). If it returns something other than a boolean, then it is doing too much and needs to be separated into multiple functions.

Arrays and loops are your best friend for repititive code.

$patterns = array(
    'name',
    'email',
    'pass',
);

foreach( $patterns AS $pattern ) {
    if( !$this->TextObject->checkPattern( $pattern ) ) {
        return $this->MessageObject->get( $pattern );
    }
}
return true;

Change these first few things and I'll take another look, but right now this is very confusing.

UPDATE

"This has irked me for a long time...code that is DRY ( Don't Repeat Yourself )...is less efficient then code that has some repetition. Dry is a good concept unless taken to extreme...as far as looping goes...either way is fine I think....I'll add the loop in as a matter of preference even though a tad bit less efficient."

How do you figure this is less efficient?

$patterns = array( 'name', 'email', 'pass' );
foreach( $patterns AS $pattern )
{
    if( !$this->$text_object->checkPattern( $pattern ) )
    {
        return $this->$message_object->get( $pattern );
    }
}

Than this?

if( !$this->$text_object->checkPattern( 'name' ) )
{
    return $this->$message_object->get( 'name' );
}
if( !$this->$text_object->checkPattern( 'email' ) )
{
    return $this->$message_object->get( 'email' );
}
if( !$this->$text_object->checkPattern( 'pass' ) )
{
    return $this->$message_object->get( 'pass' );
}

The first reduces the amount of code written, which in turn reduces the amount of times you can make mistakes, which in turn reduces the amount of lines you'll have to wade through should you ever need to debug it. It also automates similar tasks for any parameters you wish to throw at it so that if that task should ever need to change, you'll only ever have to do so once. Or should you ever wish to add to it, you only need to add a parameter to an array. Where did you hear that DRY is inefficient? I admit abusing it could get ridiculous, but you have to go out of your way to accomplish that.

Anyways, here's what I have for your update.

You still have that issue with variables and properties appearing out of thin air. $validated I'm assuming should be $not_validated. Though I, personally, would switch that around. I like imagining my variables like this to be asking a question "validated?" and not "telling" you its state. But thats a preference. $TextObject should probably be $text_object.

Another thing I just noticed. You are declaring your properties like so: $this->$text_object; When they should be declared like so: $this->text_object; The first is called a variable variable. It states that whatever the value of $text_object is will be declared as the name of a new property. First of all, these are cosidered bad form, mostly because they are impossible to track and make your code unuseable by anyone but you. Not really, but pretty damn close, not even IDE's can track these. Here it is explained in code, hopefully more clear.

$a = 'apple';
$b = 'banana';
$this->a = $b;
$this->$a = 'orange';
$$a = 'tangerine';

echo $a;//apple
echo $b;//banana
echo $this->a;//banana
echo $this->apple;//orange
echo $apple;//tangerine
//$this->apple and $apple just magically appeared, this is because they were set from the value of $a.

You have a couple of really long statements that could be made more legible. There are a few ways you can do this. The simplest is to bring the parameters out of the method call and convert them to variables in the local scope. This is pretty common and helps clean up code quite a bit.

$email = array( $this->TextObject->get('email') );
if( $this->$database_object->_pdoQuery( 'single', 'signup_check', $email ) )

Another way, usually used in conjunction with the previous, is to use PHP's helpful nature to your advantage. PHP does not care about whitespace, so you can drop long statements like this to new lines and indent to your hearts content.

$this->$database_object->_pdoQuery(
    'single',
    'signup_insert',
    array(
        '',
        $this->TextObject->get('name'),
        $this->TextObject->get('email'),
        $this->TextObject->get('pass'),
        0,
        0
    )
);
//OR
$name = $this->TextObject->get('name');
$email = $this->TextObject->get('email');
$pass = $this->TextObject->get('pass');
$array = array(
    '',
    $name,
    $email,
    $pass,
    0,
    0
);
$this->$database_object->_pdoQuery(
    'single',
    'signup_insert',
    $array
);

Last thing I want to mention is magic numbers. Those are numbers that appear out of thin air and could mean anything. Much like those variable variables I mentioned earlier, they are almost impossible to track down. To you, as the author, you probably know what these are at a glance. But to someone reading your code for the first time, they will not have a clue. This could even be you in a few months after you've started working on other projects. Take a look at the code above. Those zeroes "0" and empty strings '' don't mean anything. They just exist. They are magic. Declare them and set them as variables so people know what they do.


Building an HTML table using JavaScript More readable way to do this?
renderHtmlTable(function(tableItems) {

  var tableArray,_i,item,_len;
  tableArray = ['

' +
                        'Header 1	' +
                        'Header 2	' +
                        'Header 3	' +
                        'Header 4	' +
                        '

'];

  for (_i = 0, _len = tableItems.length; _i < _len; _i++) {
    item = tableItems[_i];

    tableArray.push('
' + item.foo + '	' +
                      '' + item.bar + '	' +
                      '' + item.baz + '	' +
                      '' + item.qux + '
')
  }

  tableArray.push('
');

 ($('#TableDiv')).html(function() {
   return lessonArray.join("");
 });

Use mustache.js!  It's a js templating engine which will point you in the right direction :)


Count number of leaves in binary tree Description:

Given a binary tree find the number of leaves. Although the problem is simple and has been solved many times I am more interested in find object oriented ways to solve the algorithmic problems.

Code:

interface Node {
   public int count();
 }

 class LeafNode implements Node {
   LeafNode() {}

   public int count() {
     return 1;
   }
 }

 class NonLeafNode implements Node {
   private Node left;
   private Node right;

   NonLeafNode(Node left, Node right) {
     this.left = left;
     this.right = right;
   }

   public int count() {
     return this.left.count() + this.right.count();
   }
 }

 class NullNode implements Node {
   public int count() {
     return 0;
   }
 }

 public class Main {
   public static void main(String[] args) {
     Node tree = new LeafNode();
     System.out.println(tree.count()); // 1

     tree = new NonLeafNode(new LeafNode(), new LeafNode());
     System.out.println(tree.count()); // 2

     tree = new NonLeafNode(
       new NonLeafNode(new LeafNode(), new NullNode()), new NullNode());
     System.out.println(tree.count()); // 2
   }
 }

You said in one of the comments that instead of modifying an existing tree, you would rather create a new tree. In this case, having separate classes for the nodes is acceptable (I wouldn't do it, though). But then, to effectively communicate that you are not going to modify the nodes, all their fields must be final.

I don't understand why you need the NullNode. To me it looks like a waste of memory.

The empty default constructor of LeafNode is unnecessary. In the NullNode class you didn't write it out explicitly, which is better.

At the end of your program you have these lines:

tree = new NonLeafNode(
  new NonLeafNode(new LeafNode(), new NullNode()), new NullNode());

As it is written right now, as a reader I have to carefully count parenteses to see which nodes belong together. This is because the text Node(), new appears almost 3 times. I prefer the following form:

tree = new NonLeafNode(
  new NonLeafNode(new LeafNode(), new NullNode()),
  new NullNode());

Or, if the structure gets even more complicated:

tree = new NonLeafNode(
  new NonLeafNode(
    new LeafNode(),
    new NullNode()),
  new NullNode());

The indentation now corresponds exactly to the depth of the node in the tree. Well, except for the root node, but that should be easy to figure out.

All in all it is a very nice piece of code. You chose really good names for the classes and variables, which is one of the most important things in programming. I also liked that you wrote the expected output in comments. For code that you show to others, this is good so that they don't have to run the code. When you do serious programming, you should replace those comments with proper unit tests.


String compression function in python code I need to create a function called compress that compresses a string by replacing any repeated letters with a letter and number. Can someone suggest a better way to do this?

s=input("Enter the string:")
temp={}
result=" "
for x in s:
    if x in temp:
        temp[x]=temp[x]+1
    else:
        temp[x]=1
for key,value in temp.items():
    result+=str(key)+str(value)
        print(result)

Encapsulate your code into functions

Your code is neither reusable nor testable, wrap it into a function and call it from under an if __name__ == '__main__' guard. This will allow you to test it more easily. You will also be able to return values instead of printing them, this will make the code more reusable:

def compress(string):
    temp={}
    result=" "
    for x in string:
        if x in temp:
            temp[x] = temp[x]+1
        else:
            temp[x] = 1
    for key, value in temp.items():
        result += str(key) + str(value)

    return result

if __name__ == '__main__':
    s = input("Enter the string:")
    print(compress(s))

You can then jump into an interactive shell and type:

>>> from your_file_name import compress
>>> compress('aaabbccccddefg')
 a3b2c4d2e1f1g1
>>> compress('b'*42)
 b42

Use existing data structures

collections.Counter offers simplified ways of counting elements of an iterable:

from collections import Counter

def compress(string):
    temp = Counter()
    result = " "
    for x in string:
        temp[x] += 1

    for key, value in temp.items():
        result += str(key) + str(value)
    return result

if __name__ == '__main__':
    s = input("Enter the string:")
    print(compress(s))

You can even simplify further as Counter can take any iterable in its constructor. You can also use str.join to simplify even further:

from collections import Counter

def compress(string):
    counts = Counter(string)
    return ''.join(letter+str(count) for letter, count in counts.items())

if __name__ == '__main__':
    print(compress(input("Enter the string: ")))

Possible bug

To me, compressing a string mean having a mean to decompress it back. Using a dictionnary keyed by each letter as you do, you lost an important information: which letter is next to which one. Also 'aabaabaabaa' will be compressed to 'a8b3' which, to me, doesn't make sense and would be better as 'a2b1a2b1a2b1a2'. But I might be wrong. In this case, itertools.groupby is much more usefull as it will keep the ordering and avoid aggregating separate groups of letters:

import itertools

def compress(string):
    return ''.join(
            letter + str(len(list(group)))
            for letter, group in itertools.groupby(string))

if __name__ == '__main__':
    print(compress(input("Enter the string: ")))


Retrieving font preference setting in Eclipse JDT plugin I have an Eclipse plugin which creates a JavaSourceViewer to visualize specific source code in a separate view.
I would like to configure the font of the viewer to match the settings of the Java Editor Text Font preference:

JavaSourceViewer textViewer = new JavaSourceViewer(...);
...

// get the SWT component which displays the text
StyledText textControl = textViewer.getTextWidget();

// retrieve the font preference from the theme manager
IThemeManager themeManager = PlatformUI.getWorkbench().getThemeManager();
ITheme currentTheme = themeManager.getCurrentTheme();
FontRegistry fontRegistry = currentTheme.getFontRegistry();
Font font = fontRegistry.get(PreferenceConstants.EDITOR_TEXT_FONT);

// set the font on the SWT component
textControl.setFont(font);

My question is especially about the code block which retrieves the font from the theme manager:

Is this the correct approach to get the font?
Is there anything else which needs to be considered?

I'm not familiar with Eclipse plugin development, so just two minor notes after going through javadoc of the relevant APIs:

FontRegistry has an addListener method which you might want and/or need to listen to and change the font of the textControl when the user changes their font settings.
The comment could be a great method name here (retrieveFontPreference, for example):

// retrieve the font preference from the theme manager
IThemeManager themeManager = PlatformUI.getWorkbench().getThemeManager();
ITheme currentTheme = themeManager.getCurrentTheme();
FontRegistry fontRegistry = currentTheme.getFontRegistry();
Font font = fontRegistry.get(PreferenceConstants.EDITOR_TEXT_FONT);

See also: Clean Code by Robert C. Martin, Bad Comments, p67: Don’t Use a Comment When You Can Use a Function or a Variable


Servlet file uploader I think this code looks ugly, especially with the multiple try-catch blocks, but I don't know how to rewrite it:

public String handleFileUpload(@RequestParam String url,@RequestParam String fileName, RedirectAttributes redirectAttributes, HttpServletResponse response) {
        InputStream inputStream;
        try {
             inputStream =  new URL(url).openStream();
        } catch (IOException e) {
            LOG.error("Cannot open stream by url=[{}]", url);
            try {
                response.sendError(HttpServletResponse.SC_INTERNAL_SERVER_ERROR, "Cannot open stream by url=" + url);
            } catch (IOException e1) {
                LOG.error("Cannot send error");
            }
            return null;
        }
        try {
            File file = File.createTempFile("tmp", ".txt", new File(System.getProperty("user.dir")));
            byte[] binary = IOUtils.toByteArray(inputStream);
            FileUtils.writeByteArrayToFile(file, binary);
            UploadedMultipartFile multipartFile = new UploadedMultipartFile(file, file.length(), "jpg",
                    "formParameter", fileName);
            MultipartFileWrapper multipartFileWrapper = new MultipartFileWrapper();
            multipartFileWrapper.setMultipartFile(multipartFile);
            redirectAttributes.addFlashAttribute(multipartFileWrapper);
        } catch (IOException e) {
            LOG.error("Cannot save file [{}] from [{}]",fileName, url);
            try {
                response.sendError(HttpServletResponse.SC_INTERNAL_SERVER_ERROR, "Cannot save file " + fileName);
            } catch (IOException e1) {
                LOG.error("Cannot send error");
            }
            return null;
        }
        return "redirect:/member/uploadImage";
    }

Two try blocks in a method is a smell IMO: your method is trying to do two things, which implies it's not doing one and only one thing.

Let's see:

We open a stream from a given url and if all goes well we have an inputStream to work with.

Or things go bad, and we log an error, and then try to send an error response before we return null - if returning the error response failed, we log and still return null.

We use the stream to create a new file, and return a hard-coded string on success, or null on failure.

That alone looks like a bit of reusable functionality that should be extracted into its own method: one whose responsibility would be to send an error response:

void sendErrorResponse(HttpServletResponse response, string message) {
    try {
        response.sendError(HttpServletResponse.SC_INTERNAL_SERVER_ERROR, message);
    } catch(IOException e) {
        LOG.error("An error occurred while sending error response: " + e.toString());
    }
}

Next, you need a function that takes a url and gives you an inputStream.

InputStream openStreamFromUrl(string url, HttpServletResponse response) {
    InputStream result;
    try {
        result = new URL(url).openStream();
    }
    catch(IOException e) {
        string message = "Cannot open stream for url=" + url;
        LOG.error(message);
        sendErrorResponse(response, message);
    }
    return result;
}

So calling this openStreamFromUrl function either returns a valid and opened InputStream, or a null reference. This means we can now do this:

public String handleFileUpload(@RequestParam String url,@RequestParam String fileName, RedirectAttributes redirectAttributes, HttpServletResponse response) {
    InputStream inputStream = openStreamFromUrl(url, response);
    if (inputStream == null) {
        return null;
    }

    // handle file upload
}


Submit an array as an HTML form value using JavaScript I'm sending data from a form with a variable number of fields via an Ajax call.  Some of the fields are grouped together so I have opted to send them as an array of JSON objects to keep the groupings, but there are a number of ways of doing this. For client-side, is it better to read the form as JavaScript objects and stringify them or just directly work with strings from the start?

Here's an example:

I want to group the data so I know which date goes with which member, so I have created a bunch of objects and added them to an array.  This seems to work but there are issues with using JSON.stringify because it's not supported in IE7.  I don't know if modifying the form is very efficient either. How would you manage this? Or would you submit the form as is and sort it out on the server?  What's the most widely accepted approach?

HTML:

<form id="form">

Group Name

    <input type="text" name="group_name">

Members

    <button type="button">New member</button>

NameDate

        <input type="text" name="name"><input type="text" name="date">

    <input type="submit">
</form>

JavaScript:

$(document).ready(function(){
    $('#form').submit(function(e){
        var memberArr = [];
        $('.member_data').each(function(){
            var thisMember = {};
            $(this).children().each(function(){
                thisMember[$(this).attr('name')] = $(this).val();
            });
            memberArr.push(thisMember);
            // Delete this input
            $(this).remove();
        });

        var input = $("<input>").attr("type", "hidden").attr("name", "members").val(JSON.stringify(memberArr));
        $('#dynamic_form').append($(input));
        e.preventDefault();

        $.post("http://geraintanderson.com/cgi-bin/ajax_array.py",
        $('#form').serialize(),
        function(data, status){
            $('#member_info').html(data);
            // Reset the form
            $('#dynamic_form').html('<input type="text" name="name"><input type="text" name="date">
');
        });
    });

    $('button').click(function(){
        // Add new form fields
        $('#dynamic_form').append('<input type="text" name="name"><input type="text" name="date">
');
    });
});

And here's a live example using a Python script to return some text.

First of all, I'd make sure that supporting IE7 is actually required. It's a really old browser by now, and there are a lot of things that are much more difficult to do.

Anyway, as for sending multiple form values, there's a common - but informal - way of doing that without JSON. It's supported by some server-side languages/frameworks (notably PHP, where I think it started), so it might work for you.

It's all about naming your inputs using square brackets to namespace them.

Typically - again, this isn't a true standard, just an informal one - you would use a name like foo[bar] to specify that bar is a named sub-property of foo. Or you'd use the name foo[] multiple times to indicate that foo is an array (but this only works for plain value arrays).

Here, you need a little of both, something like this (I've skipped a lot of HTML just to make it clearer):

<form id="form">
    <input type="text" name="group_name">

    <div id="dynamic_form">
        <input type="text" name="members[0][name]">
        <input type="text" name="members[0][date]">

        <input type="text" name="members[1][name]">
        <input type="text" name="members[1][date]">

        <input type="text" name="members[2][name]">
        <input type="text" name="members[2][date]">
    </div>
</form>

E.g. what gets sent to the server would be something like:

group_name=SomeGroup
members[0][name]=Alice
members[0][date]=01/01/2015
members[1][name]=Bob
members[1][date]=02/01/2015
members[2][name]=Carol
members[2][date]=03/01/2015

This is roughly equivalent to a JSON structure like:

{
  "group_name": "SomeGroup",
  "members": [
    {
      "name": "Alice",
      "date": "01/01/2015"
    },
    {
      "name": "Bob",
      "date": "02/01/2015"
    },
    {
      "name": "Carol",
      "date": "03/01/2015"
    },
  ]
}

Again, I can't guarantee that how your server will understand it, but it's a common way to do things.

Even if your server-side code doesn't understand it automagically, it's still parseable; you just have to write a bit more code yourself.

The trick is of course to increment the number, when you add new fields. That can be done in many ways, though.

Alternatively, you simply name all your fields the same:

<input type="text" name="name">
<input type="text" name="date">
<input type="text" name="name">
<input type="text" name="date">
<input type="text" name="name">
<input type="text" name="date">

The browser will send all the values, but usually the server-side software has a rule for how to handle duplicate names, and keeps only the first or last one it sees. But again, you can access the raw request data, and parse it yourself, if you want. But there's a lot of work in doing that.

On a side-note: I'd avoid having duplication between the javascript and the HTML (right now, the page starts out with 1 set of member inputs in the HTML, and the rest get added using a string in the JavaScript. That means you have to keep both of them identical in order for things to work - not great.


Python program to calculate the sun angle Given a string representing the hour and minute, where: 00:00 <= time <= 23:59, the program should output the number of angles the sun is above the horizon.

When time is "06:00" the sun angles are 0º
At "12:00" the sun is at 90º
At "18:00" the sun goes to 180º
After "18:00" and before "06:00" the program should output "I don't see the sun!"

Examples:

sun_angle("07:00") == 15
sun_angle("01:23") == "I don't see the sun!"
sun_angle("12:15") == 93.75

The code:

import datetime as dt

def check_you_dont_see_sun(time_dt, end_dt, start_dt):
    return ((time_dt > start_dt and time_dt < end_dt)
    or (time_dt > start_dt or time_dt < end_dt))

def calculate_sun_angle(time_dt, end_dt):
    difference = time_dt - end_dt
    hours_of_difference = (difference.seconds / 60) / 60
    angle_increase_each_hour = 15
    return hours_of_difference * angle_increase_each_hour

def sun_angle(time):

    start_dt = dt.datetime.strptime('18:00', '%H:%M')
    end_dt = dt.datetime.strptime('06:00', '%H:%M')
    time_dt = dt.datetime.strptime(time, '%H:%M')

    if check_you_dont_see_sun(time_dt,end_dt,start_dt):
        return "I don't see the sun!"
    else:
        return calculate_sun_angle(time_dt, end_dt)

What could we improve?

One immediate thing I would improve is the naming of your variables. While in general very descriptive, it took me a while to realize that start_dt is actually the time of sunset and end_dt the time of sunrise, instead of the other way around. I would also make these variables global constants and use those as default values. This way you can do it all in one still short and readable function.

I would add some docstring describing what your function(s) do and use a compound comparison instead of two comparisons to determine if the time is day or night.

The datetime module is often imported as from datetime import datetime, although your import is also OK.

The calling of the code should happen under a if __name__ == "__main__": guard, where I would also put at least some very rudimentary testing code.

from datetime import datetime

SUNRISE = datetime.strptime('06:00', '%H:%M')
SUNSET = datetime.strptime('18:00', '%H:%M')

def sun_angle(time, sunrise=SUNRISE, sunset=SUNSET):
    """Return the angle of the sun at `time`.
    Assumes that the sun moves 180 degrees between `sunrise` and `sunset`.
    During night return 'I don\'t see the sun!'.
    """
    time = datetime.strptime(time, '%H:%M')
    if not sunrise <= time <= sunset:
        return "I don't see the sun!"

    angle_per_hour = 180 / (sunset - sunrise).seconds * 3600
    time_since_sunrise = (time - sunrise).seconds / 3600
    return time_since_sunrise * angle_per_hour

if __name__ == "__main__":
    test_cases = [("07:00", 15),
                  ("01:23", "I don't see the sun!"),
                  ("12:15", 93.75)]
    for time, expected in test_cases:
        output = sun_angle(time)
        print(f"solar_angle({time!r}) == {output!r}")
        assert output == expected, f"Failed: expected {expected!r}!"

The calculation of the sun angle could probably still be a bit better in order to be able to be extended for different latitudes in the future.

If this is not a class exercise or programming challenge, I would also recommend to change the interface of the function a bit. Make time always a datetime object and instead of returning a string in case of invalid input raise a custom exception:

class NightTimeException(ValueError):
    pass

...

    if not sunrise <= time <= sunset:
        raise NightTimeException("I don't see the sun!")


Safely accepting a known SSL certificate with a different host name My app communicates with a server over an internal network through HTTPS. The SSL certificate on this server is listed for the host as its external host name. I want to accept this certificate, but I don't want to accept ANY certificate as many workarounds I've seen seem to do. I would also like to have it so that later on, in the strong possibility that his app is accessing a different server with a correct SSL certificate, that I don't have to change it. I have it working as I would like it seems, but I am very unsure of myself when it comes to implementing my own methods to override a library used for secure communications, and I am not very strong in my knowledge of SSL communications.

I would basically like input on security loopholes and its effectiveness at doing what I need.

Here is the code I have:

HttpClient httpclient = new DefaultHttpClient();
SSLSocketFactory ssf = SSLSocketFactory.getSocketFactory();
ssf.setHostnameVerifier( new X509HostnameVerifier() {

   @Override
   public void verify( String host, String[] cns, String[] subjectAlts )
         throws SSLException
   {
   }

   @Override
   public void verify( String host, X509Certificate cert ) throws SSLException
   {
   }

   @Override
   public void verify( String host, SSLSocket ssl ) throws IOException
   {
      boolean foundHost = false;
      for( javax.security.cert.X509Certificate cert : ssl.getSession().getPeerCertificateChain() )
      {
         String[] tokens = cert.getSubjectDN().getName().split( "," );
         for( String token : tokens )
         {
            String[] keyVal = token.split( "=" );

            if( keyVal.length > 1 && keyVal[0].equals( "CN" ) )
            {
               foundHost = keyVal[1].equals( context.getString( R.string.CertHostName ) );

               if(!foundHost)
               {
                foundHost = keyVal[1].equals( host );
               }
            }

            if(foundHost)
            {
               break;
            }
         }
         if(foundHost)
         {
            break;
         }
      }

      if(!foundHost)
      {
         throw new IOException( "Mismatched host in SSL certificate" );
      }
   }

   @Override
   public boolean verify( String host, SSLSession session )
   {
      return false;
   }
} );
httpclient.getConnectionManager().getSchemeRegistry().unregister( "https" );
httpclient.getConnectionManager().getSchemeRegistry().register( new Scheme( "https", ssf, 443 ) );

Basically, I found out which method to override through some log statements to see which one was being used in the anonymous inner X509HostnameVerifier class, and hopefully handled it appropriately. Input would be appreciated.

There are a few items inhere I would suggest can be improved (assuming the security aspects of the code are valid - i.e. legal disclaimer about 'safe' and SSL means speak to someone who's more of a security expert than me).

First, I assume you are using the apache-commons based HttpClient code.... this is fine, but it would have been useful to show the import statements ;-).

Regardless. There are three things I would change.

First, whenever you do an anonymous class implementation and it has more than 1 method that needs to be overridden, then I recommend that you create a full class implementation for it. In this case, your code should have a (perhaps nested) class that implements the

private static final class AlternateHostnameVerifier implements X509HostnameVerifier {
    ....
}

then your code becomes:

HttpClient httpclient = new DefaultHttpClient();
SSLSocketFactory ssf = SSLSocketFactory.getSocketFactory();
ssf.setHostnameVerifier(new AlternateHostnameVerifier());
httpclient.getConnectionManager().getSchemeRegistry().unregister( "https" );
httpclient.getConnectionManager().getSchemeRegistry().register( new Scheme( "https", ssf, 443 ) );

This is a much better way to show the critical logic of your program, and still make it readable.

Next up, in your new AlternateHostnameVerifier class, I would recommend you implement all the interface methods fully. Leaving two of the methods as stub methods is only a short-term solution. In the future the apache-commons implementation may change and call one of the other verify methods, and then you have a sudden, unexplained bug. Specifically throwing a UnsupportedOperationException is a much better alternative than leaving it blank.

Finally, I have a preference for methods that can to simply 'return' when they succeed rather than trying to break out of loops and keep track of variables outside of the loops.

In this case, you have foundHost which tracks the state. you can eliminate this variable if you just 'return' each time you set it to true, and otherwise always throw the exception.

So, you can rewrite the code as:

@Override
public void verify( String host, SSLSocket ssl ) throws IOException {
  for( javax.security.cert.X509Certificate cert : ssl.getSession().getPeerCertificateChain() ) {
     String[] tokens = cert.getSubjectDN().getName().split( "," );
     for( String token : tokens ) {
        String[] keyVal = token.split( "=" , 2);

        if( keyVal.length > 1 && keyVal[0].equals( "CN" ) ) {
           if (keyVal[1].equals( host ) || keyVal[1].equals( context.getString( R.string.CertHostName ) )) {
               // our CN matches the Android's certificate host.
               return;
           }
        }
     }
  }
  throw new IOException( "Mismatched host in SSL certificate" );
}


Imputing values with non-negative matrix factorization X is a DataFrame w/ about 90% missing values and around 10% actual values. My goal is to use nmf in a successive imputation loop to predict the actual values I have hidden. The mask, msk, selects a random 80% of the actual values (or 80% of the 10% actual values). I initialize all but these 80% to 0 and begin to impute them. Line 2 looks odd because I couldn't find a way to get a random 80% (train set) of the values who weren't np.nan so if I add an np.nan to a number, the value stays np.nan. Then if I subtract that X.values back off the only values that are effected are the non-null values of the array X_imputed. This allows me to get a random 80% of the non-null values.

import pandas as pd
from pandas import DataFrame
import numpy as np

from sklearn.decomposition import ProjectedGradientNMF

# toy example data, actual data is ~500 by ~ 250
customers = range(20)
features = range(15)

toy_vals = np.random.random(20*15).reshape((20,15))
toy_mask = toy_vals < 0.9
toy_vals[toy_mask] = np.nan

X = DataFrame(toy_vals, index=customers, columns=features)
# end toy example data gen.

# imputation w/ nmf loops
X_imputed = X.copy()
msk = (X.values + np.random.randn(*X.shape) - X.values) < 0.8
X_imputed.values[~msk] = 0
nmf_model = ProjectedGradientNMF(n_components = 5)
W = nmf_model.fit_transform(X_imputed.values)
H = nmf_model.components_

while nmf_model.reconstruction_err_**2 > 10:
   nmf_model.fit_transform(X_imputed.values)
   W = nmf_model.fit_transform(X_imputed.values)
   H = nmf_model.components_
   X_imputed.values[~msk] = W.dot(H)[~msk]

I'm pretty sure this can be written in fewer lines but I'm not sure how to do it.

In the while loop, the first call you make to nmf_model.fit_transform() is superfluous and can be removed.  You aren't even using the results of the transformation calculation.  The next line, where you have W = nmf_model.fit_transform(X_imputed.values) is doing all the work.  Removing this line halves the number of model fits and speeds things up by ~twofold.
You don't need to assign H outside/before the while loop.
If minimization of code lines is the goal, you can avoid assigning to temporary variables and just put the expression that you would have used to define the variable in the code line that uses it.  I did that for H in the while loop.  It is more compact but probably harder to understand.
You don't seem to need to full pandas module, so that import can be removed.
I didn't change anything in my code below, but why are you squaring nmf_model.reconstruction_err_?  According to the docs this error is the Frobenius norm of the difference matrix (X - WH), so it will always be positive even without squaring.

This is a tad more compact and significantly faster (because of item 1):

from pandas import DataFrame
import numpy as np
from sklearn.decomposition import ProjectedGradientNMF

# Example data matrix X
nrows, ncols = 200, 150
toy_vals = np.random.random(nrows*ncols).reshape((nrows, ncols))
toy_vals[toy_vals < 0.9] = np.nan
X = DataFrame(toy_vals, index=range(nrows), columns=range(ncols))

# Hiding values to test imputation
X_imputed = X.copy()
msk = (X.values + np.random.randn(*X.shape) - X.values) < 0.8
X_imputed.values[~msk] = 0

# Initializing model
nmf_model = ProjectedGradientNMF(n_components=5)
nmf_model.fit(X_imputed.values)

# iterate model
while nmf_model.reconstruction_err_**2 > 10:
    W = nmf_model.fit_transform(X_imputed.values)
    X_imputed.values[~msk] = W.dot(nmf_model.components_)[~msk]
    print nmf_model.reconstruction_err_


Find biggest number in array I have a function that calculates the biggest number in an array after doing the following:

arrayManipulation(4, [[2, 3, 603], [1, 1, 286], [4, 4, 882]])

The array initially consists of only zeroes. The first parameter tells what the length of the array of 0s should be.

Here it's 4, so [0,0,0,0]

The second parameter is an array of arrays. Each nested array has 3 values. The start index, last index and the value to add to the zeroArray between the two indices. The zeroArray index starts from 1.

For example, in the beginning the zero array is [0,0,0,0]. Then from the nested array [2, 3, 603], add 603 at indices from 2 to 3. So the zero array becomes [0,603,603,0]. Then the second nested array is used. This is done till all the nested arrays are used.

After that's done, I need to find the biggest number in the zeroArray(its value now is [ 286, 603, 603, 882]) which here is 882.

I am trying this here, but it timed out.

function arrayManipulation(n, operations) {
    let zeroArray = Array(n);
    zeroArray.fill(0, 0, zeroArray.length);
    let biggest = 0;
    for (let j = 0; j < operations.length; j++) {

        let startIndex = operations[j][0];
        let endIndex = operations[j][1];
        let number = operations[j][2];
        for (let k = startIndex; k <= endIndex; k++) {
            zeroArray[k - 1] += number;
            if (biggest < zeroArray[k - 1]) {
                biggest = zeroArray[k - 1];
            }
            if (k == endIndex)
                console.log(startIndex, endIndex, number, JSON.parse(JSON.stringify(zeroArray)));
        }
    }
    console.log(biggest);
    return biggest;

}

The challenge only asks for the biggest number after all operations.
Printing the complete array after each operation might cause it to fail –
either by the unexpected output or by the time spent for the log operation.

Also, determining the maximum after each operation is inefficient, this is
better done after all operations.

But what you actually need to pass the challenge is a better algorithm.
The inner loop

for (let k = startIndex; k <= endIndex; k++)

takes too much time because it is executed up to \$ 10^7 \$ times, according
to the given limits.

The crucial idea is not to execute the operations “verbatim,” but to
store and update at each array index the “cumulative increment/decrement” with respect
to the preceding element after all operations encountered so far.
This requires only 2 updates for each operation, plus a final run over
all array elements to find the maximum value.

I don't want to deprive you from the joy to figure out the concrete code
yourself, but you'll find more information at Maximum value in an array after m range increment operations, if necessary.


Parsing URL segments The .htaccess file redirects all requests to this file and the $_REQUEST['path'] variable contains the url after the base url.

$url = '';
$urlSegments = [];
$resource = '';
$action = '';
$parameters = [];

if(isset($_REQUEST['path'])){
    $url = $_REQUEST['path'];
    $urlSegments = explode('/', $url);

    if(!empty($urlSegments[0])){
        $resource = $urlSegments[0];
        array_shift($urlSegments);

        if(!empty($urlSegments[0])){
            $action = $urlSegments[0];
            array_shift($urlSegments);
            $parameters = $urlSegments;
        }
    }
}

$method = $_SERVER['REQUEST_METHOD'];

I was wondering if there is an better way (with regex maybe?) to parse the url, loop through it and extract the MVC data from the url. array_shift looks ugly and it feels that there must be a better way.

Disclaimer: I do eventually answer your question, there are just some other things I wished to point out.

GET vs REQUEST

First of all, I would consider using $_GET instead of $_REQUEST. I'm assuming its get your using judging by the data you are passing. It is generally a better idea to use the specific array you need if you know it rather than one that could potentially contain data from another source. For example, should you ever create a form with a path input, it will supersede any get path. But this isn't even necessary, the remote user can change that value should they wish. However, if you were to call them separately $_GET[ 'path' ] and $_POST[ 'path' ] you will be able to use them both together without losing any information and they are less likely to be hijacked (minus get, its pretty easy to hijack that).

Filter Input

Take a look at PHP's filter_input(). It will mean you only have to type that $url variable once and has the added benefit of sanitizing your input.

$url = filter_input( INPUT_GET, 'path', FILTER_SANITIZE_STRING );
//returns false if not set so
if( $url ) {
    //etc...
}

Array Shift

PHP's array_shift() returns the value shifted, so there is no reason to specifically call that value, simply set the variable as you shift it.

$resource = array_shift( $urlSegments );

Order of Logic

Instead of checking if the value of $urlSegments is empty, I would set it first then check it. A couple of reasons for this. What if "path" starts with a forward slash "/"? I believe explode will produce an empty element at the very beginning. Unless this is the desired result, it could cause errors. Also, it removes the need for those magic numbers, i.e.($urlSegments[0]) Yes its obvious where it came from and why you are using it, but it is a bad habit and is not extendable.

$resource = array_shift( $urlSegments );
if( ! empty( $urlSegments ) ) {
    if( ! empty( $resource ) ) {
        $action = array_shift( $urlSegments );
        if( ! empty( $action ) ) { $parameters = $urlSegments; }
    } else {
        //handle $url starting with a forward slash
        //this is where having this set up in functions would help
    }
}

Here's the Better Solution: Only one parameter

However, the cleanest way of all is to guarantee the length of $urlSegments, then use PHP's list(). So, you can use array_pad() to fill out the rest of $urlSegments should it be too short, otherwise list() will produce errors. If it is already of the proper size it will not do anything so you don't need to test it first. Now, to ensure that it doesn't already contain empty elements, that it didn't start with a forward slash, or have any double slashes, you should run it through array_filter(). Note: This will remove any element that could be considered "false" (0, '', null, -1, etc) so be careful here. This also means that if you were using empty or false parameters before, then it may not work the same way. Edit: This is if you use array_filter() without a callback, you could always make your own callback function to ignore certain values.

$urlSegments = array_filter( $urlSegments );
$urlSegments = array_pad( $urlSegments, 3, '' );
list( $resource, $action, $parameters ) = $urlSegments;

Here's the Better Solution: Multiple parameters

I just realized that $urlSegments could very likely be longer than 3 elements. In this case, I would filter and pad it like I did above, shift the first two elements into their respective variables then set $parameters to whatever remains of $urlSegments. Padding it first ensures that each variable has a default value of an empty string.

$urlSegments = array_filter( $urlSegments );
$urlSegments = array_pad( $urlSegments, 3, '' );
$resource = array_shift( $urlSegments );//default = '';
$action = array_shift( $urlSegments );//default = '';
$parameters = $urlSegments;//default = array( '' );

Shorthand

Finally, it is rarely a good idea to use shorthand in PHP. If I did not know what you were trying to accomplish, or had a slightly older version of PHP, I would think that $urlSegments = []; was syntactically wrong. Yes it works, but you could run across problems in the future. Not to mention its just easier to read $urlSegments = array();

Regex

That should clean up your code considerably. I would hesitate to suggest any regex, mainly because I find it extremely difficult to read.

Edits

Edited a few times to clean up post and add the second "Better Solution".


Recursive grid traveling algorithm I'm trying to come up with a solution to a coding challenge on Kattis, where there is a 2D grid filled with x's and o's, and the goal is to figure out whether or not a given starting coordinate can reach a given end coordinate by traveling up, down, left, or right. Only the o's are navigable, and the width and height of the grid can each be up to 1000 characters.

Here is an example grid:

xoxxx
ooxxo
oxxxo
ooooo
oxxxx
ooooo

I'll define this as

std::vector<std::string> space;

For this space, e.g., space[0][1] can reach space[5][4].

Note: This isn't quite the problem. I've generalized it a bit because there are some nuances that are a bit irrelevant to efficiency. Here's the actual problem.

I've taken a recursive approach to this problem, and my program passes the first 22 out of the 25 total test cases, before exceeding the 1-second time limit. The algorithm that I thought of was to keep checking whether the next valid coordinate could reach the end. The base case occurs when the starting coordinate is the ending coordinate, and it signifies that the problem's starting point can reach the ending point. If there is no valid next location then the current path cannot reach the ending point. If all the paths reach a dead end then that signifies that the problem's starting point cannot travel to the ending point.

Here are the brains of the program (specifically canTravel):

struct Point {
    int x, y;
    friend bool operator==(const Point &p1, const Point &p2);
};

bool operator==(const Point &p1, const Point &p2) {
    return p1.x == p2.x && p1.y == p2.y;
}

std::vector<Point> memo; // take note of the visited locations to avoid revisiting them

bool memoContains(const Point &p1) {
    return std::find_if(memo.begin(), memo.end(), [&](const Point &p) { return p == p1; }) != memo.end();
}

bool canTravel(const std::vector<std::string> &space, const Point &start, const Point &end) {
    if (start == end) // base case: the start == the destination
        return true;

    memo.push_back(start); // add current location to the list of visited locations

    /*
    format:

    const auto &next_location = avoid out of bounds error &&
        next location is an 'o' &&
        avoid revisiting a location &&
        recursively check whether the next location can reach the destination

    */

    const auto &right = start.x < space[start.y].length() - 1 &&
        space[start.y][start.x + 1] == 'o' &&
        !memoContains({ start.x + 1, start.y }) &&
        canTravel(space, { start.x + 1, start.y }, end);

    const auto &left = start.x > 0 &&
        space[start.y][start.x - 1] == 'o' &&
        !memoContains({ start.x - 1, start.y }) &&
        canTravel(space, { start.x - 1, start.y }, end);

    const auto &down = start.y < space.size() - 1 &&
        space[start.y + 1][start.x] == 'o' &&
        !memoContains({ start.x, start.y + 1 }) &&
        canTravel(space, { start.x, start.y + 1 }, end);

    const auto &up = p1.y > 0 &&
        space[start.y - 1][start.x] == 'o' &&
        !memoContains({ start.x, start.y - 1 }) &&
        canTravel(space, { start.x, start.y - 1 }, end);

    return right || left || down || up; // at least one path needs to reach the destination
}

Why is this slow/inefficient? How can it be improved? Can a recursive approach be used here or is an iterative one preferable?

What is killing your performance is memoContains. It turns your algorithm from O(n) into O(n^2).

The better solution is to create a second array of the same size as your input, where you mark each cell visited. Then you can do the check in O(1).

Next, you need to implement a short circuit for your logic. If right is true, there is no point in computing the other directions.

Why do you define right and the other three bools as references?

There are some more tricks you can apply, for example to avoid checking for out-of-bounds indexing by simply making the array one cell larger on all sides, and filling those positions with x.

Finally, regarding recursion. It's a nice way of describing the algorithm. You might get better performance if you use a stack instead. But the only way to know for sure is to try it out and time it. :)

Oh, and one more thing:

std::vector<std::string> space;

Is highly inefficient. Your data is all over the memory because the data for each array is allocated independently. You should probably make this a single vector (or string if you must) that you index as row * width + column. Note that now you can index neighboring cells using simple pointer arithmetic (add 1 to the pointer to access the cell on the right, for example).


Is this appropriate way to pass data from presentation to application layer? What is the best practices to pass data from presentation to application layer?

Currently I have something like this:

in presentation layer:

the controller:

public class ProductController : Controller
{
    private readonly IProductService _productService;

    public ProductController(IProductService productService)
    {
        _productService = productService;
    }

    public ActionResult ProductCreateAction(ProductCreateViewModel vm)
    {
        _productService.CreateProduct(vm);
        return Json(new { Ok = true });
    }
}

and the view model:

public class ProductCreateViewModel : ICreateProductCommand
{
    public string Name { get; set; }
    public int Price { get; set; }
}

in application layer:

public class ProductService : IProductService
{
    public void Create(ICreateProductCommand command)
    {
        var product = new Product(command.Name, command.Price);
        //... add to repositoey and call SaveChanges ...
    }
}

public interface ICreateProductCommand
{
    string Name { get; set; }
    int Price { get; set; }
}

(using FluentValidation here)

public class CreateProductCommandValidator : AbstractValidator<ICreateProductCommand>
{
    public CreateProductCommandValidator()
    {
        RuleFor(x => x.Name)
            .NotEmpty();

        RuleFor(x => x.Price)
            .NotEmpty();
    }
}

Is it ok to continue using this approach or there are better practices?
Is it better to have one save action in controller or separate create/update actions?

i.e.:

public ActionResult Save(SomeViewModel vm)
{
    if(vm.Id.HasValue)
    {
        //update
    }
    else
    {
        //create
    }
}

vs.

public ActionResult Create(SomeViewModel vm)
{
    //create
}

public ActionResult Update(SomeViewModel vm)
{
    //update
}

Overall, your code is very good, there is not much to review..

In your ViewModel, you might want to use DataAnnotations to do client (using jquery unobstrusive script)/server validation ex :

public class ProductCreateViewModel : ICreateProductCommand
{
    [Required]
    public string Name { get; set; }

    [Required]
    public int Price { get; set; }
}

public ActionResult ProductCreateAction(ProductCreateViewModel vm)
{
    if(ModelState.IsValid) //Server validation of the data annotations.
    {
        _productService.CreateProduct(vm);
        return Json(new { Ok = true });
    }
}

To answer your second question, I believe it is better to use two separated actions to separate the use cases of when you will use Create vs. Update. Plus one day you might have different roles for adding and updating (it's a little hardcore but it could happen), you would want your actions to be marked with different authentication attributes (If you use MVC membership)


Add-in for input() with validation What do you think about this code? Do you have some advices and remarks for me about them? I have started learning python recently.

types = {
    "str": str,
    "int": int,
    "float": float,
    "complex": complex
}

def my_input(kind, msg, msg_wrong, detail):
    """ Add-in for input(). Performs validation of the data entered.
    :param kind: data type. The keys of the dictionary types.
                 If an error occurs, a str is expected.
    :param msg: a welcome message, such as "Enter a number -> "
    :param msg_wrong: own message in case of error
    :param detail: detailed error description (True or False)
    :return: entered value with type kind
    """
    method = types.get(kind, str)
    while True:
        try:
            value = method(input(msg))
            break
        except ValueError as e:
            print(msg_wrong, detail * str(e))
    return value

msg = "-> "
msg_wrong = "Error"

print(my_input("int", msg, msg_wrong, False))
print(my_input("hello", msg, msg_wrong, True))  # wrong type, will str
print(my_input("complex", msg, msg_wrong, True))

"""
-> hi
Error
-> 15
15
-> some text
some text
-> some text 2
Error complex() arg is a malformed string
-> 4-j
(4-1j)
"""

Your function looks usable, useful and follows PEP8, well done.
Coupling types and kind together seems like a poor descision. Since functions are firstclass in Python you can just pass the function.

This would change your functions to something like:

my_input(int, msg, msg_wrong, False)

This would have the drawback that if you don't pass a valid function, then it will blow-up. But has the added benifit of allowing more types with less code.

import datetime
my_input(datetime.fromisoformat, msg, msg_wrong, False)

You can also fix the drawback by changing kind to an argument with a default value.

def my_input(msg, msg_wrong, detail, kind=str)
my_input(msg, msg_wrong, True)

It's not immediatly clear why you would want to log only certain errors with the detail argument.

If I were to show or silence errors I would use the logging module.
I'll leave configuring the logger to you, but usage would be:

import logging

logger = logging.getLogger(__name__)
logger.setLevel(logging.INFO)

...

def my_input(kind, msg, msg_wrong):
    method = types.get(kind, str)
    while True:
        try:
            value = method(input(msg))
            break
        except ValueError as e:
            logger.debug(e)
            print(msg_wrong)
    return value

...

print(my_input("int", msg, msg_wrong))
logger.setLevel(logging.DEBUG)
print(my_input("hello", msg, msg_wrong))
print(my_input("complex", msg, msg_wrong))

It's not apparent to me why you'd like to log some but not other errors. If being able to change the level of different calls to my_input is of the upmost importance then it'd make more sense to me for you to pass the logging level.

def my_input(kind, msg, msg_wrong, level=logging.DEBUG):
    ...
            logger.log(level, e)

I'm not used to seeing assignment, break and then a return out of a while true loop. When I do this I only use return.

import logging
logger = logging.getLogger(__name__)
logger.setLevel(logging.INFO)

def my_input(msg, msg_wrong, cast=str):
    while True:
        try:
            return cast(input(msg))
        except ValueError as e:
            logger.debug(e)
            print(msg_wrong)

msg = "-> "
msg_wrong = "Error"

print(my_input(msg, msg_wrong, int))
print(my_input(msg, msg_wrong))
print(my_input(msg, msg_wrong, complex))


Displaying filtered user data using Ajax and PHP I want to get my code cleaner and more efficient. This code gets variables form a PHP file and filters it to show the selected user name, all available usergroups on a list box and the groups he is currently into another.
Currently it's working great but it has a lot of delay between the modal shows up and the groups being shown.
JS:
  function saveUserGroup(id){
         $('#useridgr').attr('placeholder','').val('');
         $("#usernamegr").attr('placeholder', 'Nome').val('');
         $('#list_box').html('');   $('#list_box_ini').html('');

   $.ajax({
            type: 'GET',
            url: 'users/' + id + '/users' + '/id',
            data: {'_token': $("input[name='_token']").val()},
            success: function(data){
              $('#useridgr').attr('placeholder',data.id).val(data.id);
              $("#usernamegr").attr('placeholder',data.name).val(data.name);

              loadgroupsdata(data.id, 0);
            }   }); }

        var loadgroupsdata = (uid, type) => {

          $.ajax({
            type: 'GET',
            url: 'usergrupo/' + uid + '/' + type,
            data: {'_token': $("input[name='_token']").val()},
            success: function(data){

              if (type == 0) listbox = '#list_box';
              else listbox = '#list_box_ini';

              loadgroups(uid, listbox, type, data);

            }

          }) }

        function loadgroups(uid, location, type, data){

          $.each(data,function(i,v){
            $(location).append("<option value='"+v.idgrupo+"'>"+v.nome+"</option>");   })

          if(type == 0) loadgroupsdata(uid, 1);

        }

function sendData(type){
  uid = document.getElementById("useridgr").value;

  if(type == 1) listname = 'list_box_ini'; else listname = 'list_box';
  var e = document.getElementById(listname);

  var value = e.options[e.selectedIndex].value;

  $.ajax({
    type: 'GET',
    url: 'savegroup/' + uid + '/' + type + '/' + value,
    data: {'_token': $("input[name='_token']").val()},
    success: function(data){
      saveUserGroup(uid);

    }

  })
}

PHP:
public function getData($uid, $tablename, $index)
  {

    $data = \DB::table($tablename)->where($index, $uid)->first();
    return response()->json($data, 200);

  }

  public function getGroups($id, $type)
  {

    if ($type == 0){
      $data = \DB::table('grupoutilizador')
      -> join ('users', function ($join) use ($id){
        $join->on('users.id', '=', 'grupoutilizador.iduser')
        ->where('users.id', "=", $id );
      }) -> join ('grupo', 'grupo.idgrupo', "=", 'grupoutilizador.idgrupo') ->
      select('users.id', 'users.name', 'grupo.idgrupo', 'grupo.nome')
      -> get();

    } else {
      include 'config.php';

      $usergroups = mysqli_query($conn2, 'SELECT grupo.idgrupo FROM grupo, grupoutilizador WHERE grupoutilizador.idgrupo = grupo.idgrupo AND grupoutilizador.iduser =' .$id);

      $i = 0;

      if ($usergroups->num_rows != 0){
        foreach ($usergroups as $usergroup) {
          $ids[$i] = $usergroup;
          $i++;
        }
      } else {
        $ids[$i] = 0-1;

      }

      $data = \DB::table('grupo')
      ->whereNotIn('grupo.idgrupo', $ids)
      -> get();

    }

    return response()->json($data, 200);
  }

   public function saveGroups($uid, $type, $value){
        if ($type == 1){
          DB::table('grupoutilizador')->insert([
            'iduser' => $uid,
            'idgrupo' => $value,
          ]);

        } else {

          $result = DB::table('grupoutilizador') -> where([
          ['iduser', '=', $uid],
          ['idgrupo', '=', $value],
          ]) -> delete();

              echo $result;

        }
   }

The fact that you  are making three separate trips to the server stands out as an obvious refinement opportunity.
We don't know exactly where your performance bottleneck is, but you can be sure that best practice is to reduce your total trips to ...well, anywhere.
Think about it like this:

Your mother tells you to go to the shop and get a dozen eggs.
You get on your bicycle and make 12 trips to the shop and deliver one egg to your mother per trip.
Your sister shakes her head disapprovingly.
Your mother tells the neighbors that she loves you because you are "special".

If you know that the full flow of this task is to write data to the db ...then get some data from one table ...then get some data from another table, then the sensible thing is to dictate this flow entirely on the server side then only return to the client-side when everything is finished.
Effectively, I recommend using a single ajax request (with POST instead of GET because you are writing data to your database).
The dedicated server-side receiving script should execute the database writing query, then immediately execute the database fetching queries, then generate a single json string which contains all of the necessary user and usergroup data to satisfy your client-side processes.
If the bottleneck is isolated to the database interactions, then we don't have enough information to assist with optimizing the schema.

General advice:

Don't use mysqli_query() when you have a project that provides querying helper methods.  You whole application should use consistent techniques.

Only use concatenation when necessary.  '/users' + '/id' should be '/users/id'

When sending data to the server to fetch/read data from the db, use $_GET.  When sending to write/insert/update/delete data, use $_POST. There are exceptions, but this is the rule.

Use consistent spacing and tabbing in your code so that it is easier to maintain.

Don't use old school comma-join syntax -- use JOINs.

This is weird:
$i = 0;
if ($usergroups->num_rows != 0){
  foreach ($usergroups as $usergroup) {
    $ids[$i] = $usergroup;
    $i++;
  }
} else {
  $ids[$i] = 0-1;

}

Why declare a counter?  Why subtract 1 from 0?  You should probably trash most this section of code, and maybe entertain something closer to...
foreach ($usergroups as $usergroup) {
    $ids[] = $usergroup;
}

$data = \DB::table('grupo')
    ->whereNotIn('grupo.idgrupo', $ids ?? -1)
    ->get();

Never omit curly braces in an attempt to reduce the size of your code.  If you have a simple condition and like ternary expressions, use those instead.

Try to avoid declaring single-use variables.

If type dictates location, then don't send both parameters to the next function scope.  Generate location conditionally when you get there.