Matching brackets in Rust I implemented the solution to the classic Matching Brackets problem:
Given a string containing brackets [], braces {}, parentheses (), or any combination thereof, verify that any pairs are matched and nested correctly.
The code should be correct since it passes several test cases.
I would like to know if:

the usage of the enum Bracket is an idiomatic construct or a useless complexity
the function brackets_are_balanced can be simplified while keeping the enum Bracket
further improvement/suggestions

#![warn(missing_crate_level_docs)]
#![warn(missing_docs)]
//! # Matching Brackets
//!
//! `matching_brackets` is a collection of utilities to solve [this exercise](https://exercism.io/my/solutions/8ecfdc024536471a8c36ac2a3ff4b119).

enum Bracket {
    OPEN(char),
    CLOSE(char),
}

impl Bracket {
    pub fn new(c: char) -> Option<Bracket> {
        match c {
            '{' | '[' | '(' => Some(Bracket::OPEN(c)),
            '}' => Some(Bracket::CLOSE('{')),
            ']' => Some(Bracket::CLOSE('[')),
            ')' => Some(Bracket::CLOSE('(')),
            _ => None,
        }
    }
}

/// Check if the input `string` has balanced brackets.
///
/// # Examples
///
/// ```rust
/// use matching_brackets::brackets_are_balanced;
/// assert!(brackets_are_balanced("(((185 + 223.85) * 15) - 543)/2"));
/// assert!(brackets_are_balanced("([{}({}[])])"));
/// assert!(!brackets_are_balanced("{[)][]}"))
/// ```
pub fn brackets_are_balanced(string: &str) -> bool {
    let mut brackets: Vec<Bracket> = vec![];
    for c in string.chars() {
        match Bracket::new(c) {
            Some(Bracket::OPEN(char_bracket)) => {
                brackets.push(Bracket::OPEN(char_bracket));
            }
            Some(Bracket::CLOSE(char_close_bracket)) => match brackets.pop() {
                Some(Bracket::OPEN(char_open_bracket)) => {
                    if char_close_bracket != char_open_bracket {
                        return false;
                    }
                }
                _ => return false,
            },
            _ => (),
        };
    }
    brackets.is_empty()
}

the introduction of enum Bracket is good and idiomatic. However, this is not the only useful form, there are at least 2 other forms that would work just fine.
brackets_are_balanced can be simplified when we notice that all Brackets in Vec<Bracket> are of the Open variant, so we do not need to store Brackets, just their chars.
the introduction of fn new is against standard practice. A conversion constructor should not be called fn new and every constructor should return only Self (= Bracket). That's why the function should be renamed to from_char.
it's common practice to name enum variants with PascalCase, in your program, Bracket::Open and Bracket::Close. Not sure why you did all uppercase.
the result of brackets.pop() does not need matching, it can be simply compared.

The refactored code is
#![warn(missing_crate_level_docs)]
#![warn(missing_docs)]
//! # Matching Brackets
//!
//! `matching_brackets` is a collection of utilities to solve [this exercise](https://exercism.io/my/solutions/8ecfdc024536471a8c36ac2a3ff4b119).

enum Bracket {
    Open(char),
    Close(char),
}

impl Bracket {
    pub fn from_char(c: char) -> Option<Bracket> {
        match c {
            '{' | '[' | '(' => Some(Bracket::Open(c)),
            '}' => Some(Bracket::Close('{')),
            ']' => Some(Bracket::Close('[')),
            ')' => Some(Bracket::Close('(')),
            _ => None,
        }
    }
}

/// Check if the input `string` has balanced brackets.
///
/// # Examples
///
/// ```rust
/// use matching_brackets::brackets_are_balanced;
/// assert!(brackets_are_balanced("(((185 + 223.85) * 15) - 543)/2"));
/// assert!(brackets_are_balanced("([{}({}[])])"));
/// assert!(!brackets_are_balanced("{[)][]}"))
/// ```
pub fn brackets_are_balanced(string: &str) -> bool {
    let mut brackets: Vec<char> = vec![];
    for c in string.chars() {
        match Bracket::from_char(c) {
            Some(Bracket::Open(char_bracket)) => {
                brackets.push(char_bracket);
            }
            Some(Bracket::Close(char_close_bracket)) => {
                if brackets.pop() != Some(char_close_bracket) {
                    return false;
                }
            }
            _ => {}
        }
    }
    brackets.is_empty()
}
```


Project Euler Problem 9: Special Pythagorean triplet The problem statement is as follows:

  A Pythagorean triplet is a set of three natural numbers, a < b < c, for which,

  \$a^2 + b^2 = c^2\$
  For example, \$3^2 + 4^2 = 9 + 16 = 25 = 5^2\$.

  There exists exactly one Pythagorean triplet for which a + b + c = 1000.
  Find the product abc.

My code that follows works. I dislike the if (valuesFound) ... break;. What is a better way of structuring this?

public class Program
{
    public static void Main(string[] args)
    {
        Console.WriteLine(GetProductOfPythagoreanTripletsThatSumTo(1000));
    }

    static bool IsPythagoreanTriplet(int a, int b, int c)
    {
        return (a*a + b*b) == c*c;
    }

    static bool ValuesSumTo(int value, int a, int b, int c)
    {
        return value == a + b + c;
    }

    static long GetProductOfPythagoreanTripletsThatSumTo(int value)
    {
        int a=0;
        int b=0;
        int c=0;
        bool valuesFound = false;
        for (int aValue = 1; aValue <=value/2; aValue++)
        {
            for (int bValue = 1; bValue<=value/2; bValue++)
            {
                for (int cValue= 1; cValue<=value/2; cValue++)
                {
                    if (ValuesSumTo(value,aValue,bValue,cValue) && IsPythagoreanTriplet(aValue,bValue,cValue) && aValue<bValue && bValue<cValue)
                    {
                        a = aValue;
                        b = bValue;
                        c = cValue;
                        valuesFound = true;
                    }
                }
                if (valuesFound)
                    break;
            }
            if (valuesFound)
                break;
        }
        return a * b * c;
    }
}

You may immediately

    return a * b * c;

as soon as the values are found.
The loop

    for (int cValue= 1; cValue<=value/2; cValue++)

is superfluous: you already have a and b, and you know that \$a+b+c\$ must be equal to value, so

    cValue = value - aValue - bValue;

suffices.
Thou shalt not brute force.

Recall that the Pythagorean triple \$a, b, c: a^2 + b^2 = c^2\$ can be expressed as $$a = k(u^2 - v^2)\\
b = 2kuv\\
c = k(u^2 + v^2)$$
with coprime \$u,v\$. Since they must add up to a particular value \$N\$ (in your case, 1000), conclude that \$2k(u^2 + uv) = N\$, or \$v = \dfrac{N}{2ku} - u\$.

We know that sides must be positive, so we have

\$u > v \rightarrow u > \dfrac{N}{2ku} - u \rightarrow u > \sqrt{\dfrac{N}{4k}}\$, and
\$v > 0 \rightarrow \dfrac{N}{2ku} - u > 0 \rightarrow u < \sqrt{\dfrac{N}{2k}}\$

Now loop over the range [sqrt(125), sqrt(500)] looking or the divisors of 500 (the range is as small as [11 .. 22], and the only divisor is 20). I intentionally will not explain how to recover \$a, b, c\$ once the divisor is found. It is pretty straightforward.


Replace each element in ArrayList with pair of Ints which sum to existing element Looking for general feedback and considerations to make for the next time :).

Write a method stretch that doubles the size of an ArrayList of integers by replacing every integer in the list with a pair of integers, each half the original. If a number in the original list is odd, then the first number in the replacement pair should be one higher than the second so that the sum equals the original number. For example, if a variable called list stores this sequence of values:

[18, 7, 4, 24, 11]

and you make the following call:

stretch(list);

Then the number 18 is stretched into the pair (9, 9), the number 7 is stretched into (4, 3), the number 4 is stretched into (2, 2), the number 24 is stretched into (12, 12) and the number 11 is stretched into (6, 5) yielding the following overall list which is twice the length of the original list:

[9, 9, 4, 3, 2, 2, 12, 12, 6, 5]

public void stretch(ArrayList<Integer> list) {
    for (int i = 0; i < list.size(); i+=2) {
        int value = list.get(i);
        int right = value / 2;
        int left = right;
        if (value % 2 != 0) {
            left++;
        }
        list.add(i, left);
        list.set(i + 1, right);
    }

}

You don't need the if statement that is calculating left.  You can just make it the value that completes right into value:

int left = value - right;

Also, it is bad practice to modify a collection while you are iterating over it.  Make a new list and populate it, and just assign the reference at the end of the method.


Is this "smart" way of populating the admin page ok code-wise? I am currently hooking a large database up to a django site and want to populate the admin page with a number of models. I am using inspect to get a list of classes from each model file (one file per database in MySQL) and then registering them by name.

from django.contrib import admin
import inspect
from .models import auth

# Auth
for pair in inspect.getmembers(auth):
    # returns a tuple with the class name and ref
    model = tuple[1]
    if tuple[0][0] is not "_" and tuple[0][0].upper() == tuple[0][0]:
        # it doesnt start with _ and does start with an uppercase
        admin.site.register(model)

Is there anything wrong with this code? Is getting class references like this bad practice? It just seems to easy to be true.

The usual "django" way of doing this  - is to use get_app_config() function to get the AppConfig instance, then call .get_models() method:

from django.apps import apps

app = apps.get_app_config('auth')

for model in app.get_models():
    admin.site.register(model)


Retrieve a number in a webpage and store in a SQLite3 db I'm beggining Python. So I wrote a program which is supposed to get a number of connected people on a forum (like this one : http://www.jeuxvideo.com/forums/0-51-0-1-0-1-0-blabla-18-25-ans.htm) and store the number with the datetime in a database file (SQLite3). Every forum has his own table name.

My code is supposed to do this :

  Create object for each forum we want to retrieve with the Forum class.
  Store these objects in a list to put use in a loop For.
  Get a web page .htm (with requests) where the number of connected people is wrote in a span tag with the class "nb-connect-fofo" who looks like this 1799 connecté(s). I'm using BeautifulSoup to get the string and REGEX to get the number  It's supposed to be done for every forum
  Execute a SQLite3 request to store the datetime, number in database file with the same name as the forum which is retrieve

Here's my code :

#!/usr/bin/python3

from bs4 import BeautifulSoup
from time import sleep
import sqlite3
import datetime
import requests
import re

class Forum:

    def __init__(self, forum, url_forum): #initialization all object with there name, URL
        self.forum = forum
        self.url_forum = url_forum

        pattern = '([0-9]{1,5})'
        self.pattern = re.compile(pattern)

    def add_to_database(self): #Add to the SQLite3 database the number of connected people and the datetime to their own table
        connection = sqlite3.connect("database.db")
        c = connection.cursor()
        now = datetime.datetime.today()
        nb_co = self.recup_co()

        text = "INSERT INTO {0}(datetime, nb_co) VALUES('{1}', '{2}')".format(self.forum, now, nb_co)
        c.execute(text)
        connection.commit()
        connection.close()

        print(now, self.forum, str(nb_co))
        sleep(1)

    def recup_co(self): #Retrieving the page and the number of people connected by using REGEX
        r = requests.get(self.url_forum)
        page_html = str(r.text)
        page = BeautifulSoup(page_html, 'html.parser')
        resultat = page.select(".nb-connect-fofo")

        nb_co = re.search(self.pattern, str(resultat))

        return nb_co.group(0)

def main():
    # All forums which are scanned are here
    dixhuit_vingtcinq = Forum("dixhuit_vingtcinq", "http://www.jeuxvideo.com/forums/0-51-0-1-0-1-0-blabla-18-25-ans.htm")
    moins_quinze = Forum("moins_quinze", "http://www.jeuxvideo.com/forums/0-15-0-1-0-1-0-blabla-moins-de-15-ans.htm")
    quinze_dixhuit = Forum("quinze_dixhuit", "http://www.jeuxvideo.com/forums/0-50-0-1-0-1-0-blabla-15-18-ans.htm")
    overwatch = Forum("overwatch", "http://www.jeuxvideo.com/forums/0-33972-0-1-0-1-0-overwatch.htm")

    #All forum name's are stored here to use them with a list
    forums = [dixhuit_vingtcinq, moins_quinze, quinze_dixhuit, overwatch]

    while(True):
        for forum in forums:
            try:
                forum.add_to_database()
            except:
                print("An error occured with the forum '{0}' at {1}".format(forum.forum, datetime.datetime.today()))
                sleep(5)
        sleep(60)

main()

I will use it later to make graphics, make little statistics to improve my skill with Python. Maybe I will retrieve more forum and expand my program to scrap the website and get every post on these forums (If I, I will do this in a lot of time later).

So I'm asking you for some improvements/ideas. As a beginner, there are obviously somes errors that can be very annoying. Because, i really want to improve

Also, my code is running on one of my own server. Isn't it better to buy a cheap VPS for 2€ instead ?

Thanks for reading and thanking you in advance.

PS : If there are somes mistakes relatives to my post about the website tell me

Code smells

your code is vulnerable to SQL injection attacks because you are using string formatting to put query parameters into a query. You need to proper parameterize your query with the help of the database driver:

query = """
    INSERT INTO {table} (datetime, nb_co)
    VALUES(?, ?)
""".format(table=self.forum)
c.execute(query, (now, nb_co))

Note that this way you also don't need to worry about Python to database type conversions and quotes inside parameters - it will all be handled by the database driver.

Performance

instead of re-connecting to the database multiple times, think about connecting to a database once, processing all the data and then closing the connection afterwards
same idea about the use of requests - you may initialize a Session() and reuse
use lxml instead of html.parser as an underlying parser used by BeautifulSoup
you can use SoupStrainer class to parse only the desired element, which will allow you to then simply get the text and split by space instead of applying a regular expression:

parse_only = SoupStrainer(class_="nb-connect-fofo")
page = BeautifulSoup(page_html, 'lxml', parse_only=parse_only)
return page.get_text().split()[0]


Show the results of linked list operations in sequence I'm working on a basic linked list programs. In this program I can insert elements at the beginning, at the middle (after one specific element) of the list and at the end. I also can delete a specific element or delete an element from the end of the list. I have all these four methods in the code.

Now I was trying in the main method to perform this operations in sequence:

insert at the beginning
insert at the middle
delete a specific element
delete an element from the list.

First I want to add 10 elements at the beginning of the list {50, 70, 80, 100, 77, 200, 44, 70, 6, 0}. Then I want to add 10 elements at the middle { 5, 20, 10 ,30 ,7 , 8, 2, 104, 1, 22} after the element 200. Then I want to add 10 elements at the end {40, 30, 20, 1, 7, 76, 4 , 0, 80, 2}. Then I want to delete a specific element 76 and then delete two elements at the end of the list.

So, I want the final list as {0, 6, 70, 44, 200, 22, 1, 104, 2, 8, 7, 30, 10, 20, 5, 77, 100, 80, 70, 50, 40, 30, 20, 1, 7, 4, 0}.

I don't know if it's  a good way. Do you think the code in the main method is OK, the for parts particularly?

Main method:

int main()
{
    printf("hi");
    int i=0;
    int listsize=10;
    int arrBegining[] = {50,70, 80, 100, 77, 200, 44, 70, 6, 0};
    int arrMiddle[] = {5, 20, 10, 30, 7, 8, 2, 104, 1, 22};
    int arrEnd[] = {40, 30, 20, 1, 7, 76, 4 , 0, 80, 2};

    for(i=0;i<listsize;i++){
            insert_at_begning(arrBegining[i]);
        }

    for(i=0;i<listsize;i++){
        insert_at_middle(arrMiddle[i], 200);
    }
    for(i=0;i<listsize;i++){
        insert_at_end(arrEnd[i]);
    }

    for(i=0;i<listsize;i++){
        delete_from_middle(76);
    }

    for(i=0;i<2;i++){
        delete_from_end();
    }
 }

List operations:

void insert_at_begning(int value)
{
    var=(struct node *)malloc(sizeof (struct node));
    var->data=value;
    if(head==NULL)
    {
        head=var;
        head->next=NULL;
    }
    else
    {
        var->next=head;
        head=var;
    }
}

void insert_at_middle(int value, int loc)
{
    struct node *var2,*temp;
    var=(struct node *)malloc(sizeof (struct node));
    var->data=value;
    temp=head;

    if(head==NULL)
    {
        head=var;
        head->next=NULL;
    }
    else
    {
        while(temp->data!=loc)
        {
            temp=temp->next;
        }
        var2=temp->next;
        temp->next=var;
        var->next=var2;
    }
}

int delete_from_middle(int value)
{
    struct node *temp,*var;
    temp=head;
    while(temp!=NULL)
    {
        if(temp->data == value)
        {
            if(temp==head)
            {
                head=temp->next;
                free(temp);
                return 0;
            }
            else
            {
                var->next=temp->next;
                free(temp);
                return 0;
            }
        }
        else
        {
            var=temp;
            temp=temp->next;
        }
    }
    printf("data deleted from list is %d",value);
}

int delete_from_end()
{
    struct node *temp;
    temp=head;
    while(temp->next != NULL)
    {
        var=temp;
        temp=temp->next;
    }
    if(temp ==head)
    {
        head=temp->next;
        free(temp);
        return 0;
    }
    printf("data deleted from list is %d",temp->data);
    var->next=NULL;
    free(temp);
    return 0;
}

Typo in method name?

insert_at_begning looks like it should be called insert_at_beginning?

insert_at_middle

The method name doesn't seem to match what it actually does.  It looks more like it's insert_after.

It also looks like it has a memory leak, what happens if you attempt to insert after a value that isn't currently in the list?

var

var is not a good name for a variable, it is totally nondescript. It is even worse when it is declared at a global/file scope where it's declaration isn't even visible.  You appear to be using a global variable var in your insert methods, however have a local var in your delete_from_middle method.  This is confusing.  Variable should be named after what they represent from a logical perspective.  You should also try to avoid using the same name for variables in nested scopes.

main

Your main looks mostly ok as a test harness, however it's rather inefficient.  Since your list doesn't maintain a tail pointer, inserting at the end is much slower than inserting at the beginning.  Similarly, inserting in the middle is relatively slow.  It would be more efficient to insert everything from the end to the front, rather than from the front to the end.

I'd also personally store the arrays to be inserted in the order that I wanted them to be in the list (not backwards), then iterate through them in reverse order.  I know this has the same effect, but when I look at the code I would then be able to see the list rather than having to mentally flip the arrays.


Print matrix in spiral order in Scala Problem:

  Given a square or rectangular matrix, print its element in spiral
  order.

  For example, if input matrix is this:

    1    2   3   4
    5    6   7   8
    9   10  11  12
    13  14  15  16

  Output should be:

1 2 3 4 8 12 16 15 14 13 9 5 6 7 11 10

This code assumes a matrix of 6X5. I do feel it can look more elegant or functional. I want to get rid of vars.

import scala.util.Random

object MatrixSpiral extends App {
  val matrix = Array.fill(6)(Array.fill(5)(Random.nextInt(10)))
  matrix foreach (row => println(row.mkString(",")))

  var rows = 6
  var cols = 5
  var r = 0
  var c = 0
  var startR = 0
  var startC = 0

  println
  while (r < rows && c < cols) {
    for (i <- c until cols) {
      c = i; print(matrix(r)(i) + " ")
    }
    r += 1
    for (j <- r until rows) {
      r = j; print(matrix(j)(c) + " ")
    }
    c -= 1
    for (k <- c to startC by -1) {
      c = k; print(matrix(r)(k) + " ")
    }
    r -= 1
    for (l <- r to startR + 1 by -1) {
      r = l; print(matrix(l)(c) + " ")
    }

    rows -= 1
    cols -= 1
    c += 1
    startR = r
    startC = c
  }

}

You are using many indexes in your code.

That is, you are using an imperative style of programming.

Functional style is more about decomposing the problem in smaller even tiny steps and using recursion where reasonable.

To print out as a spiral the outermost part of a matrix what do you do?

1    2   3   4
5    6   7   8
9   10  11  12
13  14  15  16

Output should be -

1 2 3 4     8 12 16
^           ^
First row   Last column reversed but avoiding repeating 4

15 14 13
^
Last row reversed (again careful on repetions)

9 5
^
First column reversed omitting both first and last

And then? How to print the inside as a spiral? You just remove the outside, the first and last columns and the first and last items of each row and apply this process again.

This allows you to think, as much as possible in terms in concepts (print given row or column maybe reversed) instead of indexes, you find a way to simplify the problem and then establish a trivial base case (matrix empty = do not print)

So you can a program like this, in pseudocode:

def nth_row
def nth_column
def except_first
def except_last
def reversed

def spiral_print(matrix):
    if len(matrix) == 0: end
    L = len(matrix)
    print(nth_row(0, matrix))
    print(except_first(nth_column(L, matrix)))
    # last row and first column handling missing
    print_spiral(inside_matrix(matrix))

I do not know Scala specifics semantics, but here is an implementation in Python:

def column(i, matrix):
    return [row[i] for row in matrix]

def spiral(matrix):
    if len(matrix) == 0:
        return
    print(matrix[0])
    print(column(-1, matrix)[1:])
    print(matrix[-1][::-1][1:])
    print(column(0, matrix)[::-1][1:-1])
    spiral([row[1:-1] for row in matrix[1:-1]])

[::-1] means "reversed", and [1:-1] means "except first and last"


Counting letters, words, etc. in the input I'm trying to learn some coding to broaden my scope of knowledge, and I seemed to have run into a bit of a conundrum.

I'm trying to create a program to output the number of characters, digits, punctuation, spaces, words and lines that are being read in from a file.

Here is the text file I am reading in:

See Jack run. Jack can run fast. Jack runs after the cat. The cat's fur is black. See Jack catch the cat.
Jack says, "I caught the cat."
The cat says, "Meow!"
Jack has caught 1 meowing cat. Jack wants 5 cats, but can't find any more.

Here is my code:

#include <iostream>
#include <fstream>

using namespace std;

int main()
{
    ifstream lab3;
    string word;
    lab3.open("lab3.txt");
    int countletters=0,countnum=0,countpunc=0,countspace=0,words=0,line=0;
    char character,prevchar = 0;
    if(!lab3)
    {
        cout << "Could not open file" << endl;
        return 1;
    }
    while(lab3.get(character) && !lab3.eof())
    {
        if(isalpha(character))
        {
            countletters++;
        }
        if (isdigit(character))
        {
            countnum++;
        }
        if (ispunct(character))
         {
            countpunc++;
            if (isalpha(prevchar))
            {
               words++;
            }
        }
        if (isspace(character))
          {
            countspace++;
            if (isalpha(prevchar))
            {
               words++;
            }
          }
        if(character=='\n')
        {
            line++;
        }
        prevchar = character;
    }
    cout << "There are " << countletters << " letters." << endl;
    cout << "There are " << countnum << " numbers." << endl;
    cout << "There are " << countpunc << " punctuations." << endl;
    cout << "There are " << countspace << " spaces." << endl;
    cout << "There are " << words << " words." << endl;
    cout << "There are " << line << " sentences." << endl;
    lab3.close();
    return 0;
}

Output:

There are 167 letters.
There are 2 numbers.
There are 18 punctuations.
There are 52 spaces.
There are 47 words.
There are 4 sentences.

Some things I am hoping to learn:

Advice for improvements on my code for learning purposes/efficiency.
Explanation for reading information in from a text file: whether it is letters, numbers, punctuation - whatever you may run across doing this type of data-processing.

Some things I am aware of:

using namespace std; is not good practice - what is the best practice for real world applications?
I am a beginner and this may not be (definitely is not) the cream-of-the-crop coding.

About using namespace std;. It's best to just do, e.g., std::ifstream etc each time. There's a lot of stuff here, and even in this example it would have made it easier for me to see that std::ispunc was built-in, and not something you wrote for this.

Variable names: If your name is two words put together, you should use either snake_case or camelCase consistently. E.g. countletters -> countLetters. I'd find numLetters even more readable. Some people would even say you really ought to make it numberOfLetters. lab3 also ought to be more descriptive.

Arrange your code logically. First you declare lab3. Then you declare word. Then you declare a lot of other stuff. And finally, you make sure lab3 opened correctly. Doing all the ifstream stuff in one place makes it so much easier to read/fix later on. E.g.

std::ifstream input("lab3.txt");
if (!input)
{
    std::cerr << "Could not open file." << std::endl;
    return 1;
}

For reading the file, personally I would use std::getline, then split each line into a vector of "words." First of all, this would it easier to check whether to count something as a word (I think yours breaks on contractions.) And then you can just loop over each character of each word of each line for the small stuff.

Misc. std::ifstream's close() gets called by the destructor when it goes out of scope. No need to do it manually here. Your output says you have 4 sentences. You have 4 lines. Several more sentences. There's probably not much to do about it here, but whenever I see a whole bunch of variable definitions grouped together, my first thought is to move them closer to where they're actually used. But again they're already about as close as they can get here. You never use word.


Writing a vector to a file The following function (function template rather) is to take a vector of any type and write it a file using a filestream object.

template<class T>
void Vec_WriteFile(const vector<T>& vec, ifstream file, char del = '\n')
{
   for (const T& element : vec) file << element << del;
   file << "\b ";
}

This function is very straightforward and simple; however, I'm a student to C++ and would love to learn different ways of doing even the simplest of tasks.

You need to include the headers your code depends on; don't assume that the user of your code will include them for you:
#include <vector>
#include <ostream>  // see note below

Never require your user to bring names into the current namespace; type names used in your public interface should always be qualified:
template<class T>
void Vec_WriteFile(const std::vector<T>& vec, std::ifstream file, char del = '\n')

Don't attempt to write to an istream.  Your output should go to an ostream instead.  You could use an ofstream, but that's quite specific.  Instead, you should accept a reference to any ostream; the user can then pass an ofstream or std::cout or any other output stream:
template<class T>
void Vec_WriteFile(const std::vector<T>& vec, std::ostream& file, char del = '\n')

There's nothing in your code that wouldn't work for a std::list equally as well as for a std::vector, so you can be more general:
template<class Container>
void Vec_WriteFile(const Container& vec, std::ostream& file, char del = '\n')

Naming: del evokes "delete" in my head before I reset and think "delimiter".  It might be better written in full, or perhaps as sep for "separator".

Layout: at first glance, it's not obvious which code is controlled by the for condition, as you have inconsistent indentation.

Writing a backspace over the final delimiter isn't the same as not writing the delimiter.  If you want the delimiter between items but not after the final item, the usual way is to track whether the loop is in its first iteration:
bool first = true;
for (...) {
    if (first)
        first = false;
    else
        file << del;
    file << element;
}

Error reporting: instead of just assuming that the writes are all successful, we could report via the return value from the function.  This may help the user remember to deal with errors, which could be overlooked if they have to check the stream state after the call.

My version:
#include <ostream>

template<class Container>
std::ostream& write_container(const Container& c,
                     std::ostream& out,
                     char delimiter = '\n')
{
    bool write_sep = false;
    for (const auto& e: c) {
        if (write_sep)
            out << delimiter;
        else
            write_sep = true;
        out << e;
    }
    return out;
}


Finding number of possible arrangements of books on a shelf with a Monte-Carlo approach The Problem
I am trying to determine numerically how many possible ways a number of books can be arranged on a shelf. Specifically, there are x3 categories "physics", "sci-fi", and "travel". Each contains N_phys, N_scifi, and N_travel numbers of books respectively. Within their category, the books can be placed in any order, but they must stay within their respective category (i.e all the physics books together). The categories can then be arranged in any order (i.e I could have all the sci-fi books first, followed by travel, followed by physics, for example).

My Attempt
I have decided to label each book and its category by integers, with "physics" = 1, "sci-fi" = 2, and "travel" = 3. The shelf is then a 2D array. So for example, a whole shelf could look like the following:
[3   2   1   4   1   2   2   1   3;   % Book ID
 1   1   1   1   3   3   2   2   2]  % Categrory label

where the the first 4 books are physics (because the second row is 1), followed by 2 travel books, and finally 3 sci-fi books, like this:

This problem can be solved easily and exactly as a permutation, and the result is N_phys ! x N_scifi ! x N_travel ! x 3 !. For N_phys = 4, N_scifi = 3, and N_travel = 2, the result is 1728 possible arrangements.
I have written a "brute-force" numerical attempt, shown below in Matlab, which seems to give the correct result:
N_phys = 4;   % Number of physics books
N_scifi = 3;  % Number of sci-fi books
N_travel = 2; % Number of travel books

books_physics = [1:N_phys;...
                 ones(1,N_phys)]; % Collection of physics books

books_scifi = [1:N_scifi;...
               2*ones(1,N_scifi)]; % Collection of sci-fi books

books_travel = [1:N_travel;...
                3*ones(1,N_travel)];  % Collection of travel books

num_samples = 10000; % Number of random shelves to generate
unique_shelves = zeros(num_samples*2,N_phys+N_scifi+N_travel); % Preallocate

unique_shelves(1:2,:) = [books_physics books_scifi books_travel];
num_unique_shelves = 1;

% Generate "num_samples" permutations of shelves randomly
for sample_num = 1:num_samples

books_physics_shuffled = books_physics( :, randperm(N_phys) ); % Shuffle physics books
books_scifi_shuffled = books_scifi( :, randperm(N_scifi) );    % Shuffle sci-fi books
books_travel_shuffled = books_travel( :, randperm(N_travel) ); % Shuffle travel books

category_order = randperm(3,3); % Choose order of categories, e.g. sci-fi/phsycis/travel  = [2 1 3]

shelf = [];

% Arrange the categories in a random order
for k = 1:3
    if category_order(k) == 1
        shelf = [shelf books_physics_shuffled];
    elseif category_order(k) == 2
        shelf = [shelf books_scifi_shuffled];
    elseif category_order(k) == 3
        shelf = [shelf books_travel_shuffled];
    end
end

% Iterate over discovered shelves, and see if we have found a new unique one
shelf_exists = 0;
for k = 1:num_unique_shelves
    if shelf == unique_shelves( (2*k-1):(2*k),:)
        shelf_exists = 1; % Shelf was previously discovered
        break
    end
end

if ~shelf_exists % New shelf was found
    unique_shelves( (2*num_unique_shelves+1):(2*num_unique_shelves+2),:) = shelf; % Add shelf to existing ones
    num_unique_shelves = num_unique_shelves + 1;
end

end

disp(['Number of unique shelves found = ',num2str(num_unique_shelves)])
disp(['Expected = ', num2str(factorial(N_phys)*factorial(N_scifi)*factorial(N_travel)*factorial(3) )])

As can be seen, I am basically randomly generating a shelf, and then checking if it has been previously found. If it has, I add it to the list.
I am looking for feedback on the way this is implemented, and how it can be improved to make it more concise and efficient. Is there a better data structure for storing such "unique_shelves", instead of the 2D array of integers as above? My code also doesn't scale easily for more categories, since they are hardcoded.
Tips or alternative examples would be great! Thanks.

I don't really understand your question. So I'll take things to the extreme and assume there are n1 books, n2 categories, n3 super categories... etc.
This whole hierarchy can be represented as a rooted tree, and the number of valid permutations would be the number of postorder traversals of the tree (or equivalently, preorder).
(I don't expect this will answer your question, I just thought it was interesting)
Also, if you'd like to find some middle ground between your specific example and my extremely overgeneralised... whatever this is... it might be easier to come up with a reasonable answer.
(edit...)
Some more points
This is not specific to matlab, just DSA in general
I still maintain that randomness is not the way to go for this problem.
As far as data structures go using arrays is fine, but for generality/scalability I suggest encoding the categories into the lists as well.
tree = {{1,2,3},{1,2},{1,2,3,4}} <- a cell array lets you structure data
Also, it's inefficient to check each new permutation against every saved entry. The usual thing to do is to use a set (a hashmap or a hashtable). But this adds the problem of computing a hash of the array (or finding some succinct unique id)


Elegant output separators Today I found myself reinventing output separators for the zillionth time.

I have converged on an approach like the following that seem pretty clean:

Live On Coliru

#include <iostream>

void foo(int a, int b) {
    auto sep = [first=true]() mutable { return std::exchange(first, false); };

    std::cout << "foo(";
    if (a) std::cout << sep() << "a:" << a;
    if (b) std::cout << sep() << "b:" << b;
    std::cout << ")\n";
}

int main() {
    foo(0,0);
    foo(0,1);
    foo(2,3);
    foo(4,0);
}

Extracting Building Blocks?

This could be made a bit more general by extracting the mutating state:

template <typename T = bool, T initial = true, T fallback = false>
struct once {
    mutable T flag = initial;

    operator T() const { return std::exchange(flag, fallback); }
};

So that we can now write the sep() lambda as:

auto sep = [first=once{}]{ return first? "" : ", "; };

Abstracting The Whole One-Off Value

We can go one better using

    auto sep = one_off("", ", ");

With:

template <typename T = char const*>
struct one_off {
    mutable once<> flag;

    T first, other;
    one_off(T first, T other) : first(std::move(first)), other(std::move(other)) {}

    operator T() const { return flag? first : other; }
};

Used as: Live On Coliru

void foo(int a, int b) {
    auto sep = one_off("", ", ");

    std::cout << "foo(";
    if (a) std::cout << sep << "a:" << a;
    if (b) std::cout << sep << "b:" << b;
    std::cout << ")\n";
}

Reflection And Merit?

As you can see these are all potentially useful building blocks. I'm wondering whether anyone else uses similar building blocks and what they learned from actual usage.

Ideas:

what about a dynamic separator? It's much easier to capture, say, columnar formatting in a stateful lambda eg: Live On Coliru

void foo(std::vector<int> const& v) {
    auto sep = [n=0u]() mutable {
        if (n++)
            return n%5==1?"\n" : ", ";
        return "";
    };

    for (auto& el : v)
        std::cout << sep() << std::setw(5) << el;
    std::cout << "\n";
}

Doesn't this conflict with standard library concepts like std::once_flag from C++11 too much?
Intuitively I feel these don't mix too well. Say, we want a every_nth style value wrapper instead of one_off (to replace every nth, instead of just the first)? Suppose we want to return from an input sequence, transforming every nth? Aren't we at risk of reinventing a Range Library?
Conversely, is there a straight-forward way to express these building blocks using an existing (range) library such as Boost Range or RangeV3?

  Notes:

  I used C++17 for brevity of exposition; it's not about using C++17 (I'm aware of ostream_joiner).
  It's not about string joining per se, which is why I chose a contrived example. In this example, obviously you could just print the delimiter if (a && b) but that's not the point of the exercise.
  Thanks to @LucDanton for suggesting std::exchange

That's fine as far as it goes.
Though I am not sure why you want all the added complexity for a problem that seems to be already solved.

What I think we all want is to stream containers in a logical way, that is consistent across containers.

Your basic example:

for(auto const& el: c) {
    std::cout << sep() << el;
std::cout << "\n";

This is a specialization of how I would want containers to be stremed:

auto sep = one_off("", sepObject);
stream << beginMarker;
for(auto const& el: c) {
    stream << sep() << el;
stream << endMarker;

So for example:

// An array looks like this:
[ 1, 2, 3, 4, 5, 6 ]        // Note terminator is "]\n"

// A map look like this:
{ a => b, c => d, e => f }

So would it be not more logical to define a container streamer object.

std::Container  c = {1,2,3,4,5};
ContainerStream(std::cout) << c;

Then you can use traits to define the beginMarker, endMarker and sepObject.

Or alternatively rather than wrapping the stream we could potentially wrap the container in something that knows how to print it (and generalize that).

std::cout << jsonExport(c);


PySpark code that turns columns into rows I am new to PySpark. My goal is to translate this table:

|Key|Value1|Value2|Value3| ...
+---+------+------+------+
|  1|    v1|    v2|    v3|

into a table like this:

|Key|ColumnName|ColumnValue|
+---+----------+-----------+
|  1|    Value1|         v1|
|  1|    Value2|         v2|
|  1|    Value3|         v3|

And here is my code for doing so:

from pyspark.sql import *

sample = spark.read.format("csv").options(header='true', delimiter = ',').load("/FileStore/tables/sample.csv")

class Closure:
  def __init__(self, columnNames):
    self.columnNames = columnNames

  def flatMapFunction(self, columnValues):
    result = []
    columnIndex = 0
    for columnValue in columnValues:
      if not columnIndex == 0:
        result.append(Row(Key = columnValues[0], ColumnName = self.columnNames[columnIndex], ColumnValue = columnValue))
      columnIndex = columnIndex + 1
    return result

closure = Closure(sample.columns)
sample.rdd.flatMap(closure.flatMapFunction).toDF().show()

I know it is weird that I have to explicitly created the closure, but it looks like it is required, without which the execution would fail with a weird exception.

Questions:

Is it good code?
Is it going to scale well? There will be a few hundred columns but millions of rows.
Is there anything I should do to improve the code?

There is a function in the standard library to create closure for you: functools.partial. This mean you can focus on writting your function as naturally as possible and bother of binding parameters later on.

As for your function:

Don't explicitly increment a running index, use enumerate instead;
You can use zip to iterate over two iterables at the same time;
Prefer using a list-comprehension to using [] + for + append;
You can use next on an iterator to retrieve an element and advance it outside of a for loop;
Avoid wildcard imports, they clutter the namespace and may lead to name collisions.
Use an if __name__ == '__main__': guard for your top-level code.

Proposed improvements

from functools import partial

from pyspark.sql import spark, Row

def flatten_table(column_names, column_values):
    row = zip(column_names, column_values)
    _, key = next(row)  # Special casing retrieving the first column
    return [
        Row(Key=key, ColumnName=column, ColumnValue=value)
        for column, value in row
    ]

if __name__ == '__main__':
    sample = spark.read.format("csv").options(header='true', delimiter = ',').load("/FileStore/tables/sample.csv")
    sample.rdd.flatMap(partial(flatten_table, sample.columns)).toDF().show()


Deque implementation with array in Java I'm trying to implement a deque data structure with an iterator in Java using array. Please help to review and point out things that I can improve on.

public class Deque<Item> implements Iterable<Item> {

  private Item[] a; // the array.
  private int itemCount; // number of objects the array holds at any given point in time.
  private int head; //last filled position from Front
  private int tail; //last filled position from END
  private int size; // size of array.

   public Deque() {
      // construct an empty deque
      size = 2;
      a = (Item[])new Object[size];
      itemCount = 0;
      head = 0;
      tail = 0;
   }

   public boolean isEmpty()  {
                   // is the deque empty?
     return (itemCount == 0);
   }
   public int size()                        {
      // return the number of items on the deque
    return itemCount;
   }
   public void addFirst(Item item) {
     try {
       if (item == null) {
       throw new IllegalArgumentException("item cannot be null");
     }
   }
   catch (NullPointerException ex) {
     throw new IllegalArgumentException("item cannot be null");
   }
      if (isEmpty()) {
        addLast(item);
        //return;
      } else if (head == 0) {
        resize(size);
        head--;
        a[head] = item;
        itemCount++;
        //return;
      } else {
        head--;
        a[head] = item;
        itemCount++;
        //return;
      }
      return;
   }

   private void resize (int currentSize) {
     //resize array to squared of current size.
     int newSize = currentSize * currentSize;
     int newStart = (currentSize * (currentSize - 1))/2;
     int newHead = newStart;
     Item[] newA = (Item[]) new Object[newSize];
     for (int j = itemCount; j > 0; j--) {
       newA[newStart++] = a[head++];
     }
     head = newHead;
     tail = newStart - 1;
     size = newSize;
     a = newA;
     return;
   }

   public void addLast(Item item)   {

      if (isEmpty()) {
        tail++;
        a[tail] = item;
        head = tail;
        itemCount++;
        //return;
      }
      else if ( tail == (size -1)) {
        resize(size);
        tail++;
        a[tail] = item;
        itemCount++;
      }
      return;
   }
   public Item removeFirst()  {
      // remove and return the item from the front
      Item k;
      if (isEmpty()) throw new java.util.NoSuchElementException("Empty Deque");
      else {
        k = a[head];
        a[head] = null;
        head --;
        //start --;
        itemCount--;
      }
      return k;
   }
   public Item removeLast()  {
     // remove and return the item from the end
     Item k;
     if (isEmpty()) throw new java.util.NoSuchElementException("Empty Deque");
     else {
       k = a[tail];
       a[tail] = null;
       tail --;
       //end --;
       itemCount--;
     }
     return k;
   }
   public Iterator<Item> iterator() {
    // return an iterator over items in order from front to end
    return new DequeIterator();
   }
   private class DequeIterator implements Iterator<Item> {
     private int startpos;
     private int size;
     public DequeIterator () {
       startpos = head;
       size = itemCount;
     }
     public boolean hasNext() {
       return (size != 0);
     }
     public void remove() {
       throw new UnsupportedOperationException();
     }
     public Item next() {
       if (hasNext()) {
         size--;
         return a[startpos++];
       }
       throw new NoSuchElementException();
     }
   }
}

Pointless try ... catch:

 try {
   if (item == null) {
     throw new IllegalArgumentException("item cannot be null");
   }
 }
 catch (NullPointerException ex) {
   throw new IllegalArgumentException("item cannot be null");
 }

Nothing in the protected code can generate a NullPointerException.  Perhaps something did at one point, but that code was removed, and the try ... catch was not removed?

Missing argument check:

addFirst() protects against adding null, but addLast() does not.

Resizing:

private void resize (int currentSize) {

This is not a static member function, so it has access to the current size of your deque.  Why require the caller to pass in currentSize?

private void resize() {
  int currentSize = size;

... would be sufficient, and more friendly to the caller.

Resizing:

  int newSize = currentSize * currentSize;

You are squaring the current size???  Ok, going from 2 to 4 is fine, and 4 to 16 is within reason, and 16 to 256 could be justified, but 256 to 65536?  I won't even contemplate the next step!

Doubling is a perfectly reasonable, and quite sane, resizing strategy.

Resizing:

  for (int j = itemCount; j > 0; j--) {
    newA[newStart++] = a[head++];
  }

Copying element by element is entirely correct, but you could let the JVM do the heavy lifting.  There is a good chance it will be faster.

  System.arraycopy(a, head, newA, newStart, itemCount);

DequeIterator

You aren't protecting against any external modification to the deque while you are iterating over it.  Consider a deque containing one item, and call the iter = iterator() method.  Then, call removeLast().  At this point, iter.hasNext() will return true, and iter.next() will return a null!

A simple fix is to maintain a modification counter in the deque class, and on every modification, increase it by one.  When you create an iterator, the iterator gets a copy of the modification counter's value.  On any iterator operation, compare the deque's current modification counter with the copy; if they are different, throw a ConcurrentModificationException.

Wasted space:

If your deque has space for 256 items, but only contains 100 items, if the items are all stored at the start or end of the storage array, then a call to addFirst() or addLast() might resize the array, growing it to 65536 entires, 65436 which will be empty.

Two options:

1) If there is still plenty of available space in the storage array, just move the current contents back to the "centre" of the storage array.  System.arraycopy() will do the heavy lifting here, avoiding any copy issue that may arise if the source and destination blocks overlap.

2) Use a circular array.  When you increment past the end of the storage array, you reset the index back to 0; when you decrement past the start of the storage array, you reset the index to the last element. itemCount == size() becomes the trigger condition to resize the storage array.  Take care to ensure you copy data from the src array to the destination array properly; you'll need to do it in two arraycopy() calls.

(Optional, Advanced) Type safety:

Your Item[] a array is not actually an Item[] array.  It is a Object[] array masquerading as an Item[] array, because it was cast to the desired type to force it into the desired member:

a = (Item[]) new Object[size];

This is fine, as long as the array never, ever leaks out of your Deque class.  For instance, don't ever add a toArray() method.

You can make a correctly typed storage array, as long as you are willing to pass the real item's class to the constructor.

public Deque(Class<Item> item_class) {
  size = 2;
  a = (Item[]) Array.newInstance(item_class, size);
  ...

You'd need this item_class in your resize() code as well, so store it as a member.


Loading and displaying JSON quickly in PhoneGap app I have this tap event in an iOS app that I'm developing with PhoneGap. When I click on the left arrow, it finds the desired content and retrieves it from a JSON file. It then displays those results on the screen.

How can I improve this? There's a bit of lag... I know that some of the lag is attributed to the 300ms delay (which fastclick.js and other libraries could resolve), but what else can I do to restructure this code and make it more snappy? I need it to respond quickly.

// PREVIOUS DAYS
    $('.left-arrow').on("tap", function() {
        dateArrayIndex--;
        todaysDate = morehShiur[dateArrayIndex]['date'];
        todaysContent = morehShiur[dateArrayIndex]['description'];
        $('.date').text(todaysDate);
        var path = window.location.href.replace('index.html', '');
        $.getJSON(path + "data/heb-text/" + todaysContent, function(data) {
            $('.title').empty();
            $('ol').empty();
            $('.title').append(data['title']);
            for (var i = 0; i < data.content.length; ++i) {
                $('ol').append('<li>' + data.content[i]['content'] + '</li>');
            }
            $("html, body").animate({ scrollTop: 0 }, 0);
        });
    });

Although I doubt it will make a big difference and help you much,
but it would be more efficient to move operations out of the function whose input and output never change. Such as these:

var path = window.location.href.replace('index.html', '');
path + "data/heb-text/"

No need to execute these every time a tap is fired.
You could execute these once, save the result in a variable,
and use those variables inside the tap handler.

If the document structure doesn't change,
then you could save processing time by caching these dom lookups too:

$('.date')
$('.title')
$('ol')
$("html, body")

Again, this might not make a big difference, but perhaps worth a shot anyway.


Zipfile password recovery program I would like to get a few tips on how to improve this program, which is supposed to help recovering lost passwords from .zip files.

import zipfile
import string
import itertools
import random
import time

"""
Zipfile password cracker using a dictionary attack and, if not successful,
switches to bruteforce
"""

def crack(zipfilename,dictionary):

    t0=time.time()
    password = None
    zip_file = zipfile.ZipFile(zipfilename)
    alphabet = string.ascii_letters + string.digits + string.punctuation

    #Tries at first a dictionary attack

    with open(dictionary, 'r') as f:
        for line in f.readlines():
            password_string = line.strip('\n')
            try:
                password = bytes(password_string, 'utf-8')
                zip_file.extractall(pwd=password)
                t1=time.time()
                total = t1 - t0
                print('Password found : %s' %password_string)
                print('Time spent : %f seconds' %total)
                return
            except:
                pass

    #If the password hasn't been found yet, the function switches to bruteforce

    nbcharmax = 10 #Maximum length to test

    for i in range(1,nbcharmax):
        print('Testing length = %i' % i)
        for j in itertools.product(alphabet, repeat=i):
            try:
                password_string=''.join(j)
                password = bytes(password_string, 'utf-8')
                zip_file.extractall(pwd=password)
                t1=time.time()
                total = t1 - t0
                print('Password found : %s' %password_string)
                print('Time spent : %f seconds' %total)
                return
            except:
               pass

if __name__ == "__main__":
    crack('test.zip','dictionary.txt')

So now the program seems to work fine but is a bit slow in my opinion. I'm wondering if there's a faster way than zip_file.extractall() to open archive files?

1. Follow the styleguide (PEP8)

don't import modules which you're not using (random)
more, imports are always put at the top of the file, just after any module comments and docstrings, and before module globals and constants.
I always like to put my imports in alphabetical order, so that it gets easier to find them.
you should have two newlines before defining your methods
you should also have a space after every ,
you should put a space before and after operators (x = 'something'); this is not required when defining arguments.
instead of commenting code in the middle of the method, try to split the logic of your program in separate functions.
you should be using the new .format() style

2. Working on the code:

you should really let the user choose where the files actually are
don't use bare excepts.
you can directly use .encode() if you want to convert a string to bytes. That's actually the pythonic way of doing it as per this SO discussion
when reading a file, you can omit the r mode, as that's the default one

3. Efficiency
The problem does not come from the zip module but rather from the cartesian product you're doing. As @jonsharpe mentioned in his comments, your program has to go through a lot of passwords.
The final code would look like this:
"""
Zipfile password cracker using a dictionary attack and, if not successful,
switches to bruteforce
"""

import itertools
import string
import time
import zipfile

ARCHIVE_PATH = 'test.zip'
DICTIONARY_PATH = 'dictionary.txt'

def dictionary_attack():
    """Tries at first a dictionary attack"""

    t0 = time.time()
    with open(DICTIONARY_PATH) as f:
        for password in f:
            password = password.rstrip().encode()
            try:
                zipfile.ZipFile(ARCHIVE_PATH).extractall(pwd=password)
                t1 = time.time()
                print('Password found: {}\nTime spent: {} seconds'.format(password.decode(), t1 - t0))

                return True
            except RuntimeError:
                pass
    return False

def bruteforce_attack(nbcharmax):
    """If the password hasn't been found yet, the function switches to bruteforce"""

    alphabet = string.ascii_letters + string.digits + string.punctuation

    t0 = time.time()
    for i in range(1, nbcharmax):
        for j in itertools.product(alphabet, repeat=i):
            password = ''.join(j).encode()
            try:
                zipfile.ZipFile(ARCHIVE_PATH).extractall(pwd=password)
                t1 = time.time()
                print('Password found: {}\nTime spent: {} seconds'.format(password.decode(), t1 - t0))

                return True
            except RuntimeError:
                pass
    return False

if __name__ == "__main__":
    if not dictionary_attack():
        bruteforce_attack(4)


Parsing a key=value config file for a Python FreeCell game I'm new to Python, and my first serious project was a FreeCell game. The game works, with lots of options to combine moves, save games, load them, etc. But I skimped on some specifics of Python. In particular, reading in options.

import re

vertical = False
doubles = False
autoReshuf = False
savePosition = False
saveOnWin = False
annoyingNudge = False

def readOpts():
    global vertical
    global autoReshuf
    global doubles
    global saveOnWin
    global savePosition
    global annoyingNudge
    infile = "fcopt.txt"
    with open(infile) as f:
        for line in f:
            gotOne = 1
            if line[0] == '#': #ignore comments
                continue
            q=re.sub(r'.*=', '', line.rstrip())
            if "autoReshuf".lower() in line.lower():
                autoReshuf = TOrF(q)
            if "savePosition".lower() in line.lower():
                savePosition = TOrF(q)
            if "saveOnWin".lower() in line.lower():
                saveOnWin = TOrF(q)
            if "vertical".lower() in line.lower():
                vertical = TOrF(q)
            if "doubles".lower() in line.lower():
                doubles = TOrF(q)
            if "annoyingNudge".lower() in line.lower():
                annoyingNudge = TOrF(q)
    if gotOne:
        print "Options file read."
        f.close()
    else:
        print "Failed to read options file."
    return

def TOrF(x):
    if x == "False" or x == "0":
        return False
    return True

readOpts();

The options file I have features lines such as this, with #s to comment lines out:

vertical=1
doubles=0
autoReshuf=1
savePosition=0
saveOnWin=0
annoyingNudge=True

I have a feeling I could simplify this code greatly with ConfigParser, but I'm totally busted at the moment, even with examples. And I don't like throwing all those globals around. How can I write this brute-force code more succinctly and efficiently?

You have a good idea on what to do. But rather than using global instead use local objects. But even better use a dictionary.

Currently you have something like:

my_global_variable = 'default'

def read_settings():
    global my_global_variable
    my_global_variable = 'something else'

def foo():
    if my_global_variable == 'something else':
        # Do something

However this means that you have to keep everything in your global scope. Rather than this, I'd change to use a dictionary. To do this you can set all the defaults in a single variable, and then over-write them in your read_settings file. Take:

settings = {
    'key': 'default'
}

def read_settings():
    settings['key'] = 'something else'

def foo(SETTINGS):
    if SETTINGS['key'] == 'something else':
        # Do something

This allows you to later move these functions into their own modules, and they will still be usable. And so I'd go on to change the original code to use a dictionary to hold the settings.

I would also change:

Remove re and instead use str.split, it makes the code easier to understand.
If XKDC is your think they even suggest not using them:

Check if the left hand side is actually correct, autoReshufAn'Ting shouldn't pass, and be allocated to autoReshuf.
Change your settings to have better keywords, like auto_reshuffle. The use of _ allows you to use only lower-case letters and for your keys to still be readable.
Remove the if/else after you read the file. The else is wrong as you were able to read the options file, otherwise there would be an error.
Don't use semicolons, there's no need in Python.
Follow PEP8.

Without using dictionarys, you can get something like:

vertical = False
doubles = False
auto_reshuffle = False
save_position = False
save_on_win = False
annoying_nudge = False

def read_settings(infile="fcopt.txt"):
    global vertical
    global doubles
    global auto_reshuffle
    global save_position
    global save_on_win
    global annoying_nudge

    with open(infile) as f:
        for line in f:
            # ignore comments
            if line[0] == '#':
                continue
            key, value = line.split("=", 1)
            if key.lower() == "vertical":
                vertical = true_or_false(value)
            if key.lower() == "doubles":
                doubles = true_or_false(value)
            if key.lower() == "auto_reshuffle":
                auto_reshuffle = true_or_false(value)
            if key.lower() == "save_position":
                save_position = true_or_false(value)
            if key.lower() == "save_on_win":
                save_on_win = true_or_false(value)
            if key.lower() == "annoying_nudge":
                annoying_nudge = true_or_false(value)

def true_or_false(x):
    if x == "False" or x == "0":
        return False
    return True

read_settings()

After this you want to use the dictionary as I suggested. With this you can use in to check if key.lower() is in it, and allows you to remove all the duplicate code. After checking if the key is in the dictionary, you want to overwrite the default value in the dictionary, so you can just assign the true_or_false index the dictionary, by indexing with the lowercase key.

This can get something like:

SETTINGS = {
    'vertical': False,
    'doubles': False,
    'auto_reshuffle': False,
    'save_position': False,
    'save_on_win': False,
    'annoying_nudge': False,
}

def read_settings(infile="fcopt.txt"):
    with open(infile) as f:
        for line in f:
            # ignore comments
            if line[0] == '#':
                continue
            key, value = line.split("=", 1)
            key = key.lower()
            if key in SETTINGS:
                SETTINGS[key] = true_or_false(value)

def true_or_false(x):
    if x == "False" or x == "0":
        return False
    return True

read_settings()

And so if you want to add a new value to your settings you can! Just by adding it to the dictionary.

You can also try using json to make a config for you. I personally use it as my default data storage. And with by using sort_keys=True and indent=4, you can make a pretty configuration. Which doesn't support comments. Or alternately you can import a Python file with a dictionary, shown below, or use configparser.

setting.py:

settings = {
    'VerTicaL': True,
}

main.py:

import settings

settings = {k.lower(): v for k, v in settings.settings.iteritems()}


HTML5 Elements - First markup This is my first website. I would like to make sure that:

I am using HTML5 sectioning elements correctly.
I didn't forgot anything important for HTML5 compatibility like html5shiv, for example.

Any other advice?

<!DOCTYPE html>

    <meta charset="utf-8">
    <title>DEdesigns</title>

    <link rel="stylesheet" href="style.css">

DEdesigns

Home

About

Services

Contact

About Me

Lorem Ipsum is simply dummy text of the printing and typesetting industry. Lorem Ipsum has been the industry's standard dummy text ever since the 1500s, when an unknown printer took a galley of type and scrambled it to make a type specimen book. It has survived not only five centuries, but also the leap into electronic typesetting, remaining essentially unchanged

My Work

rem Ipsum is simply dummy text of the printing and typesetting industry. Lorem Ipsum has been the industry's standard dummy text ever since the 1500

rem Ipsum is simply dummy text of the printing and typesetting industry. Lorem Ipsum has been the industry's standard dummy text ever since the 1500

rem Ipsum is simply dummy text of the printing and typesetting industry. Lorem Ipsum has been the industry's standard dummy text ever since the 1500

Services

one

two

three

Contact Me

some contact me stuff goes here

This is my fotter

Your formatting is a little off,  indentation is very important when reading any code.

I fixed the indentation and I also added some new lines in some of your paragraph tags. they are formatted by the browser pretty nicely so you can added new lines in there to make them more readable with out side scrolling and they will still show up the way you intended.

I also took out all your empty lines, because it is formatted nicely and is easily readable, blank lines make it look messy to me.

<!DOCTYPE html>

        <meta charset="utf-8">
        <title>DEdesigns</title>

        <link rel="stylesheet" href="style.css">

DEdesigns

Home

About

Services

Contact

About Me

                    Lorem Ipsum is simply dummy text of the printing and typesetting industry.
                    Lorem Ipsum has been the industry's standard dummy text ever since the 1500s,
                    when an unknown printer took a galley of type and scrambled it to make a type specimen book.
                    It has survived not only five centuries, but also the leap into electronic typesetting,
                    remaining essentially unchanged

My Work

                                rem Ipsum is simply dummy text of the printing and typesetting industry.
                                Lorem Ipsum has been the industry's standard dummy text ever since the 1500

                                rem Ipsum is simply dummy text of the printing and typesetting industry.
                                Lorem Ipsum has been the industry's standard dummy text ever since the 1500

                                rem Ipsum is simply dummy text of the printing and typesetting industry.
                                Lorem Ipsum has been the industry's standard dummy text ever since the 1500

Services

one

two

three

Contact Me

some contact me stuff goes here

This is my footer


Finding the number of capital letters in a word grouped by letter Please see the code options below:

Client App - this is the same for both options

LetterCounter lc = new LetterCounter("HeLLo");

Option 1

class LetterCounter
    {
        string _Word;
        List<LetterQuantities> letterQuantities;

        //Other functions will be here.

        public LetterCounter(string Word)
        {
            _Word = Word;
            LetterQuantities lq = new LetterQuantities();
            letterQuantities = lq.GetCapialLetterQuantities(Word);
        }
    }

    class LetterQuantities
    {
        int _Quantity;
        char _Letter;

        public LetterQuantities()
        {
        }

        public LetterQuantities(int Quantity, char Letter)
        {
            _Quantity = Quantity;
            _Letter = Letter;
        }

        public List<LetterQuantities> GetCapialLetterQuantities(string Word)
        {
            List<LetterQuantities> letterQuantitiesList = new List<LetterQuantities>();
            for (char c = 'A'; c <= 'Z'; c++)
            {
                int count = Word.Split(c).Length - 1;
                if (count > 0)
                {
                    LetterQuantities lq = new LetterQuantities(count, c);
                    letterQuantitiesList.Add(lq);
                }
            }
            return letterQuantitiesList;
        }
    }

Option 2

class LetterCounter
    {
        string _Word;
        List<KeyValuePair<char, int>> LetterQuantities = new List<KeyValuePair<char, int>>();

        //Other functions will be here.

        public LetterCounter(string Word)
        {
            _Word = Word;
            GetCapialLetterQuantities();
        }

        public void GetCapialLetterQuantities()
        {
            for (char c = 'A'; c <= 'Z'; c++)
            {
                int count = _Word.Split(c).Length - 1;
                if (count > 0)
                {
                    LetterQuantities.Add(new KeyValuePair<char,int>(c,count));
                }
            }
        }
    }

The concerns I have are:

1) Option 1: LetterQuantities returns a list of itself.  Not sure this is ideal.  However, it is how a Tree data structure works.
2) Option 2: This is simpler.  However, I am not sure whether 'Letter' is a valid key.
3) I am not sure whether the object(s) should be Singletons as they are value objects rather than entity objects.

I believe I should use option 1 from a TDD point of view.

The answer is that I wouldn't use either of those approaches. Counting the number of occurrences on a string isn't complex enough that requires a class of it's own. An extension method however would be a reasonable way to do it, as well as using the LINQ to get the desired result:

public static IDictionary<char, int> GetOcurrences(this string value){
    return value.GroupBy(c => c)
        .ToDictionary(c => c.Key, c => c.Count());
}

Or to count only letters

public static IDictionary<char, int> GetOcurrences(this string value){
    return value
        .Where(char.IsLetter)
        .GroupBy(c => c)
        .ToDictionary(c => c.Key, c => c.Count());
}

Or to count only capital letters

public static IDictionary<char, int> GetOcurrences(this string value){
    return value
        .Where(c => c >= 'A' && c <= 'Z')
        .GroupBy(c => c)
        .ToDictionary(c => c.Key, c => c.Count());
}

Or to count only letters provided by a parameter

public static IDictionary<char, int> GetOcurrences(this string value, string filter){
    return value
        .Where(c => filter.Contains(c))
        .GroupBy(c => c)
        .ToDictionary(c => c.Key, c => c.Count());
}


Using `exec()` in an autoloader After reviewing autoloader functions on the site, I didn't see anything like this, and I was wonder what may be wrong with this?

function findNloader($classFile){
    $file = exec("find ./ -name ".$classFile.".php");
    if(file_exists($file)){
        return require $file;
    } else {
        return false;
    }
}

The shell command returns false if somebody were to ever manage to inject something like:

../../../../sudoers  or the like...

And of course, it could be used with
spl_autoload_register() -- and if the file path were configured properly, it could find anything without chaining down autoload functions.  I understand that if it were pointed at a very large project, it might be resource intensive -- but with a direct enough focus, is there anything else wrong with this?

It could then be shortened into:

public function findnload($class) {
    return (file_exists($st = exec("find ./ -name ".$class.".php")) ?
    require $st : false);
}

And that looks quite elegant, to me.
Or if you want to be safe:

public function findnload($class) {
    return (file_exists($st = exec("find ./ -name ".$class.".php")) &&
           (length($class) < $thirtyTwoOrSo) || ($someOtherSafetyCriteria)
           ?
    require $st : false);
}

Any thoughts ?

The main problem I see with this is that you are assuming a few things:

I am running Linux - Are you? The production server might, but are you locally running linux as well? Maybe, but will the next collaborator/developer?
My Linux distro supports find - That also may or may not be true. Some distros will have that built in, some will not.

You are tying yourself to a specific environment, that's not good. A better approach would be to make sure that the class file sits under the path you expect it to, relatively to a file you know where it is relatively to the project.


Faster way to check if number has factor of different primes than given list of primes im trying to get the factor prime number(s) for given number if its different than the primes that i have in a list.

The number have to:

dividable with any of the primes in the list
should not have any other factor as primes other than in the list

example:

my_primes = [2, 3, 5]

n = 14 should return [7]

14 has factors [1, 2, 7, 14], can be divided by prime 2 (which is in list and prime) but also have 7 as factor prime and it is not in my list so i want to return that. If number has even 1 prime like this its enough for me so i dont need to check till i find other primes.

The code i have come up with so far:

from rTime import timer

@timer
def solve(n):
    def is_prime(n):
        i = 2
        while i**2 <= n:
            if n % i == 0:
                return False
            i += 1
        return True

    primes = [2, 3, 5]
    listo = []

    if n % 2 != 0 and n % 3 != 0 and n % 5 != 0:
        return False

    for i in range(2, n):
        if n % i == 0 and is_prime(i) and i not in primes:
            listo.append(i)
        if listo:
            break

    return listo

result = solve(1926576016)
print(result)

so my problem is it takes 22.36 seconds to check if 1926576016 has any primes like that at the moment. Is there anything i can improve in my code to make it faster or whole different approach needed to do this task.

Should note that i have been learning python since 4-5 months so i might not be aware of any build-in tools that makes this faster and its my first question here sorry if i did something wrong !

(@timer is a function that i have write to check how long it takes to run a function)

A note on terminology,

14 has factors [1, 2, 7, 14]

Usually these are called divisors, while the factors of 14 are taken to be only 2 and 7.
The Algorithm
While it is not entirely clear to me what you need the result to be, there is an alternative approach to finding the answer to the following question:

Does the number have any factors that are not in the list?

Which I hope is a fair re-wording of

[the number] should not have any other factor as primes other than in the list

To answer that question, a possible algorithm is:
For each p in primes, as long as n is divisible by p, divide n by p. The "leftover" n at the end is the product of all the factors of the original n that are not in primes.
For example in Python,
def solve(n):
    primes = [2, 3, 5]
    for p in primes:
      while n % p == 0:
        n = n // p
    return n

Now the result will be 1 if the number n only had the primes as factors, or otherwise it will be whatever is left. In the original function, the leftover would also be essentially be factored (but slowly, and only the first factor is returned). An integer factorization algorithm (there are faster options for that than trial division) could be applied as an extra step to recreate that result, but that is not required to answer the question of whether the number has any other factors that were not in the list primes.
This algorithm would not deal very well with a much longer list of primes (say, millions of primes), but it does deal well with cases such as 1926576016 where the lowest prime factor which isn't in primes is high.


Decoding and encoding JSON for a PHP service I am writing a PHP service which will listen for JSON requests (made using jQuery) and will return the JSON responses.

The whole template for such services (there will be 5) I have shortened into this template:

<?php

// set the headers which are accepted
header("Access-Control-Allow-Origin: *");
header("Access-Control-Allow-Headers: origin, content-type, accept, authorization");
//header("Access-Control-Allow-Methods: GET, POST, PUT, DELETE, OPTIONS, HEAD");
header("Access-Control-Allow-Methods: POST");
header('Content-Type: application/json');

// "HTTPS-only " validation
$isSecure = false;
if (isset($_SERVER['HTTPS']) && $_SERVER['HTTPS'] == 'on') {
    $isSecure = true;
}
elseif (!empty($_SERVER['HTTP_X_FORWARDED_PROTO']) && $_SERVER['HTTP_X_FORWARDED_PROTO'] == 'https' || !empty($_SERVER['HTTP_X_FORWARDED_SSL']) && $_SERVER['HTTP_X_FORWARDED_SSL'] == 'on') {
    $isSecure = true;
}

$REQUEST_PROTOCOL = $isSecure ? 'https' : 'http';

if (!$isSecure) {
    $response['status'] = 1;
    $response['message'] = 'Connection is not secure';
    http_response_code(400);
    echo json_encode($response);
    exit;
}

// !! for debug only !!
error_reporting(E_ALL);

// get the request data
$postdata = file_get_contents("php://input");

// decode
$request = json_decode($postdata, true);

$response = [];
try {
    if (true) {
        throw new Exception();  // validation not passed case
    }

    // proccess request data .. make calculations

    // all went successfull case
    $response['status'] = 0;
    $response['message'] = 'OK';
} catch (Exception $ex) {
    $response['status'] = 3;
    $response['message'] = 'Exception';
} finally {
    if ($response['status'] === 0) {
        // set the response code to 200: OK
        http_response_code(200);
    } else {
        http_response_code(400);
    }

    // output the answer
    echo json_encode($response);
}

Here is the approach I am using:

set the allowed headers
allow only HTTPS based calls (no HTTP)
decode the input data
make data validation and analysis
throw a custom Exception if some validation is not passed
in finally block echo the accumulated response variable

I am quite experienced when it comes down to PHP services so I am looking for an advice - if the above mentioned pattern/template can be improved in any way? Add additional validation?

The basic use cases are: new user registration/login/change password/get data specific to provided object ID, pushing some data to DB etc.

If you are truly trying to decouple the request handling and response building from the service definition (i.e. build a template), you might want common classes which could accept some service configuration information containing things such as:

CORS configuration
Security configuration (is secure request required)
Support HTTP action verbs

And perform some of the basics around building the request and response objects

Imagine a template which looks like this:

// likely stored in configuration somewhere
$request_config = (object) [
    'https_required' => true,
    'https_forward_allowed' => true
    'http_accept' => [
        'application/json' => true,
        'xml/text' => false
    ],
    // other configurations you may need to validate a request or perhaps
    // determine its routing
];

$response_config = (object) [
    'cors_headers' => [
        'Access-Control-Allow-Origin: *',
        'Access-Control-Allow-Headers: origin, content-type, accept, authorization',
        'Access-Control-Allow-Methods: POST'
    ],
    'content-type' => 'application/json'
    // any other things you may want to configure in response
];

// instantiate response object based on configuration
try {
    // perhaps throw exception if configuration is bad
    $response = new Response($response_config);
} catch (Exception $e) {
    error_log(
        'Unable to build Response object. ' .
        'Exception message: "' . $e-getMessage . '"'
    );
    http_response_code(500);
    exit;
}

// try to build a valid request object
try {
    // instantiate request object
    // this instantiation should throw Exception (with reason) if you
    // get an unexpected request.
    // this request object constructor could check request headers
    // against expected values, try to to marshall (json_decode) the POSTed data
    // etc. and fail out with exception if request falls outside expected.
    $request = new Request($request_config);
} catch (Exception $e) {
    error_log(
        'Unable to build Request object. ' .
        'Exception message: "' . $e-getMessage . '"'
    );
    $response->set_error_code(400);
    $response->set_error_message('Bad Request');
    // call method which sends response, but would also be responsible
    // for sending appropriate method (like with internal class class to
    // (send_headers()) or similar method.
    $response->send();
    exit;
}

// now you have valid Request object you can work with and do things like:
$post_data = $request->get_post_data();

// maybe pass to the posted data to some class or otehr logic which builds
// a data structure for return.
$response->set_data(json_encode($some_data));
$response->send();
exit;

Note that the thought here is to define classes which can encapsulate the logic around validating the request against expected request headers.  You make this functionality more usable and extensable in this fashion.  For example, say you want to add an authentication/authorization component in your application.  You could ideally just have to modify the Request class to handle the extra logic around validating authentication information.

Outside of that, a few other thoughts:

$REQUEST_PROTOCOL = $isSecure ? 'https' : 'http'; What is this here for as it doesn't seem to be used anywhere else.
You have a few lines of code that are really long. Try to keep your lines under 80 characters.
You seem to start building your $response array out of order.  Where you define the empty array is several lines past the first time you try to set information into that array.


JavaScript Canvas Animation This animation makes my fans spin up after several seconds, and it seems to have a sub-par framerate. I am trying to understand why, and optimize its performance.

Codepen

The animation function (this is the main part of the code):

function animate() {
    t++
    ctx.clearRect(0, 0, c.width, c.height);

    for (var i = 40; i < max; i+= 60) {
        var radius = Math.floor(i+Math.sin(t/10)*10)
        ctx.beginPath()
        ctx.arc(center[0], center[1], radius, 0, 2 * Math.PI, false)
        ctx.closePath()
        ctx.stroke()
    }

    requestAnimationFrame(animate)
}

The for loop draws the minimum number of circles needed in order to fill the screen. Their radius pulses in and out with a sine function based on the 't' variable.

Am I doing something wrong?

I guess I'm just assuming that it's inefficient because it makes my macbook's fans spin up after maybe 10 seconds

That's not really a good measure of "efficiency". The browser is trying to render 60 frames per second. That's a frame every ~16ms. Of course that's going to cause your processor to churn.

Math.floor(i+Math.sin(t/10)*10)

You can use <number> | 0 to trim off the decimal places of your numbers. In some JS engines and certain versions of JS engines, bitwise operations are faster than your regular math functions.

2 * Math.PI

There is a very small penalty when accessing object properties and becomes evident when you use it all over the place. Since Math.PI is a constant value, it wouldn't hurt if you assigned it to a variable for direct access. In fact, 2 * Math.PI is also constant. Why not put the result in a variable and spare the engine a multiplication operation as well.

function animate() {
  ...
  requestAnimationFrame(animate)
}

The issue with depending on just the render loop is the time it takes between updates. requestAnimationFrame can run faster or slower depending on conditions. If your "tweening" is time-sensitive, you might want to update based on delta-time instead.


Scraping the Bundesliga table and saving it to CSV Wrote my first mini-script to get data from the german football league table and save the data to a csv file. Do you like the approach or could I be more efficient? Thanks in advance!

from bs4 import BeautifulSoup
import requests
import csv

url = "https://www.bundesliga.com/de/bundesliga/tabelle"

r = requests.get(url)

r_soup = BeautifulSoup(r.content, "lxml")

table = r_soup.find_all("tr")

csv_file = open("bundesliga_table.csv", "w")

csv_writer = csv.writer(csv_file)
csv_writer.writerow(["Rank", "Team", "Matches", "Points", "Goal Difference"])

for club in table:
    try:
        rank = club.find("td", class_="rank").text
        team = club.find("span", class_="d-none d-lg-inline").text

        matches = club.find("td", class_="matches").text
        points = club.find("td", class_="pts").text
        difference = club.find("td", class_="difference").text

        print(str(rank) + " " + str(team) + " " + str(matches) + " " + str(points) + " " + str(difference))
        csv_writer.writerow([rank, team, matches, points, difference])

    except:
        print("One team not found")

csv_file.close()

Great answers, as always.
Just a few remarks from me:

There is not enough validation in your project.
You are scraping a website that could change at any time, and your script is expecting DOM elements that may not be there. So you need to check each of them.

From the doc (emphasis is mine):

  If find_all() can’t find anything, it returns an empty list. If find()
  can’t find anything, it returns None

To avoid repetition, instead of repeatedly calling club.find (even if it's just a few times), you could have a for loop, using a list or dictionary containing the DOM elements being sought and the matching HTML attribute. Then you validate the existence of the element and extract the text value in the same pass. Thus, your code becomes more solid and easier to maintain. On the other hand, you have just 5 elements in this code. But your next project may involve retrieving a lot more.

Your HTTP request can fail too, for lots of reasons like lost connectivity. Then the rest of your code will fail. I suggest to wrap the HTTP request in its own try/catch block and stop execution if it fails. There is no point trying to parse the HTML if it was not retrieved.

It is good to have exception handling in the functions that do specific tasks, but the main function should also have its own generic exception handler. Advice: log every exception to a file. Especially if the script is going to run unattended.

One last thing: you should always test your code in less than ideal conditions: try to run it against another, arbitrary  website, or a domain name that does not even exist, and see how it behaves.


Python tkinter bouncing ball animation In the current code, the animation is very unclear, and not "polished" (it doesn't run smooth).

Can I make the animation smoother?
And/or, why it doesn't run smooth?

from tkinter import *
import time

WIDTH = 800
HEIGHT = 500
SIZE = 50
tk = Tk()
canvas = Canvas(tk, width=WIDTH, height=HEIGHT, bg="grey")
canvas.pack()
color = 'black'

class Ball:
    def __init__(self):
        self.shape = canvas.create_oval(0, 0, SIZE, SIZE, fill=color)
        self.speedx = 9 # changed from 3 to 9
        self.speedy = 9 # changed from 3 to 9
        self.active = True
        self.move_active()

    def ball_update(self):
        canvas.move(self.shape, self.speedx, self.speedy)
        pos = canvas.coords(self.shape)
        if pos[2] >= WIDTH or pos[0] <= 0:
            self.speedx *= -1
        if pos[3] >= HEIGHT or pos[1] <= 0:
            self.speedy *= -1

    def move_active(self):
        if self.active:
            self.ball_update()
            tk.after(40, self.move_active) # changed from 10ms to 30ms

ball = Ball()
tk.mainloop()

Your animation isn't smooth because the ball is jumping 9 pixels for every frame. If you want smooth animation, change your "speed" to a much smaller number.

Animation is a set of trade-offs. The speed is a simple multiplier of how many frames you display each second times how many pixels you move in each frame. As the number of pixels you move goes up, the more jerky the animation will be. Likewise, the more often you update, the faster your animation will go. You need to decide which is more important: smoothness or speed.


Traverse and modify a deep object I wrote this piece of code yesterday at work. This function will traverse a deep nested object to find and modify a value.

I know it's bad, so I am trying to refactor it. I have not found any significant way to improve it. This is kind of dealing with legacy code. Changing the object structure is off the book, I should only modify this function, because things are working I don't dare to touch other parts.

How would you refactor this?

Object structure:

{
    panel1Name: {
        screen: {
            screen1Name: {
                variable: {
                    var1: {
                        custom: [],
                    },
                    var2: {
                        custom: [],
                    },
                    var3: {
                        custom: [],
                    },
                    // many other var
                }
            },
            screen2Name: {
                // ...
            },
            // many other screens
        }
    },
    panel2Name: {
        // ...
    },
    // many other panels
}

// custom is the value I need to check and modify

Code

function removeImageFromSettings(imageName) {
  for (let panel in $scope.data) {
    if ($scope.data.hasOwnProperty(panel)) {
      for (let screen in $scope.data[panel].screen) {
        if ($scope.data[panel].screen.hasOwnProperty(screen)) {
          for (let variable in $scope.data[panel].screen[screen].variable) {
            if ($scope.data[panel].screen[screen].variable.hasOwnProperty(variable)) {
              const data = $scope.data[panel].screen[screen].variable[variable];

              if (data.custom && data.custom[0] === imageName) {
                data.custom[0] = '';
              }
            }
          }
        }
      }
    }
  }
}

You could move the logic that gets each panel / screen etc into it's own function, and remove some of the indenting, like so:

function getChildren(obj) {
    // No need to check hasOwnProperty, and keys only returns ownProperties
    return obj ? Object.keys(obj).map(function(key) {return obj[key]}) : [];
}

function removeImageFromSettings(imageName) {
    let panels = getChildren($scope.data);

    let screens = panels.reduce(function(result, panel) {
        return result.concat(getChildren(panel.screen));
    }, []);

    let variables = screens.reduce(function(result, screen) {
        return result.concat(getChildren(screen.variable));
    }, []);

    variables.forEach(function(variable) {
        if (variable.custom && variable.custom[0] === imageName) {
            variable.custom[0] = "";
        }
    });
}


Designing a listener method and making it Testable I have a java based server application that needs to support multiple clients. In the main class I have a method that runs in an infinite loop and waits for client connection. Following is this method:

public void listen() {
    try {
        serverSocket = new ServerSocket(portNo);
        while (true) {
            clientSocket = serverSocket .accept();
            ClientMgr client = new ClientMgr(clientSocket);
            client.start(); //start a separate thread to manage client
        }
    } catch (Exception e) {
        //handle any exceptions
    } finally {
        close(); //close any open sockets
    }
}

I would like to know how I can design (change) this method such that it can be tested by a unit test. If it is already testable then suggest how this can be tested?

I think it's possible to test this method with another threads and some helper code which passes fix requests to your server and checks the server's answer. It definitely requires some work and I'm not sure how should we call this test: unit test, module test or integration test? Anyway, make sure that the checks happens on the the test method's thread, otherwise JUnit won't detect the uncatched exceptions.

To make it more testable I'd separate the ClientManager creation to an IncomingConnectionHandler interface and implementation and pass an instance of this object to the class of the listen method.

public interface IncomingConnectionHandler {
    void handle(Socket socket);
}

public final class IncomingConnectionHandlerImpl {

    public IncomingConnectionHandlerImpl {
    }

    public void handle(final Socket socket) {
            final ClientManager client = new ClientManager(clientSocket);
            client.start();
    }
}

public class MySocketListener {

    private final IncomingConnectionHandler incomingConnectionHandler;

    public MySocketListener(final IncomingConnectionHandler incomingConnectionHandler) {
        this.incomingConnectionHandler = checkNotNull(incomingConnectionHandler,
            "incomingConnectionHandler cannot be null");
    }

    public void listen() {
        try {
            serverSocket = new ServerSocket(portNo);
            while (true) {
                final Socket clientSocket = serverSocket.accept();
                incomingConnectionHandler.handle(clientSocket);
            }
        } catch (Exception e) {
            //handle any exceptions
        } finally {
            close(); //close any open sockets
        }
    }

After this in the unit test you could do the following:

create a MySocketListener instance with a mocked IncomingConnectionHandler,
start an new thread which connects to the port of MySocketListener,
check that the handle method of the mocked IncomingConnectionHandler was called with a live/proper Socket instance,
shutdown the helper thread and the MySocketListener.

Consider not using using abbreviation like Mgr. It would make the code readable.


Two ways to remove duplicates from a list The question is - Write a program to remove the duplicates in a list
Here's how I did it-
numbers = [1, 1, 1, 3, 3, 7, 7]
for i in numbers:
    while numbers.count(i) > 1:
        numbers.pop(numbers.index(i))
print(numbers)

Here's how the teacher in the youtube video did it-
numbers = [2, 2, 4, 4, 6, 6]
uniques = []
for number in numbers:
    if number not in uniques:
        uniques.append(number)
print(uniques)

I know both the programs are objectively different in terms of how they do the task and the second program will I reckon consume more memory to create the new list however given the question which is the better approach and for what reasons?

The teacher is obviously using a better way. Check/read more about time complexity for lists in python here (python wiki).
In your case, you are going for a time complexity:

\$ O(n) \$ for iteration
\$ O(n) \$ for .count()
\$ O(n) \$ for intermediate pop
\$ O(n) \$ for .index

for a final (worst case) time: \$ O(n^2) \$ (see explanation in comments from superb rain).
In case of the tutorial:

\$ O(n) \$ for iteration
\$ O(k) \$ for not in check (using \$ k \$ since the list is different now.
\$ O(1) \$ for append

generating a worst case performance of \$ O(n \cdot k) \$.

A more efficient solution, disregarding the order of elements would be the call:
uniques = list(set(numbers))

as suggested in comments. This would have \$ O(n) \$ time.