Guessing Number(s) Game in Python import random, os

HIGHSCORE_DATA = []
NUM_DIGITS = 1
MAX_GUESS = 10

if os.path.isfile('./highscore.txt'):
    f = open("highscore.txt", "r")
    highscore_data = f.readline().split(',')
    HIGHSCORE_DATA = highscore_data[0], highscore_data[1]

def increment_difficulty(difficulty = 1):
    global NUM_DIGITS
    NUM_DIGITS += difficulty

def get_secret_num():
    numbers = list(range(10))
    random.shuffle(numbers)
    secretNum = ''
    for i in range(NUM_DIGITS):
        secretNum += str(numbers[i])
    return secretNum

def get_clues(guess, secretNum):
    global NUM_DIGITS
    if len(guess) > NUM_DIGITS:
        return 'ERROR_LENGTH_TOO_LONG'

    if guess == secretNum:
        increment_difficulty()
        return 'You got it!'

    clues = []
    for i in range(len(guess)):
        if guess[i] == secretNum[i]:
            clues.append('Fermi')
        elif guess[i] in secretNum:
            clues.append('pico')
    if len(clues) == 0:
        return 'Bagels'

    clues.sort()
    return ' ' . join(clues)

def is_only_digits(num):
    # Returns True if num is a string of only digits. Otherwise, returns False
    if num == '':
        return False

        for i in num:
            if i not in '0 1 2 3 4 5 6 7 8 9'.split():
                return False

        return True

def disp_intro():
    global HIGHSCORE
    print('[Highscore] Name: {name} | Level: {level}\n'.format(name=HIGHSCORE_DATA[0], level=HIGHSCORE_DATA[1]));
    print('I am thinking of a of a %s-digit number. Try to guess what ' % (NUM_DIGITS))
    print('The clues I give are . . .')
    print('When I say:    That means: ')
    print('Bagels         None of the digits is correct.')
    print('Pico        One digit is correct but in the wrong position.')
    print('Fermi          One digit is correct and in the right position.')

while True:
    NUM_DIGITS = 1
    num_guesses = 1
    while num_guesses <= MAX_GUESS:
        secret_num = get_secret_num()

        # Debugging Purposes - print(secret_num)

        print('I have thought up a number. You have %s guesses to get it.' % (MAX_GUESS))
        guess = ''
        disp_intro()

        while len(guess) != NUM_DIGITS or not is_only_digits(guess):
            print("Guess #%s: " % (num_guesses))
            guess = input()

            clue = get_clues(guess, secret_num)
            print(clue)
            if clue == 'You got it!':
                num_guesses = 1;
                print("============================\n")
                break;
            elif clue == 'ERROR_LENGTH_TOO_LONG':
                num_guesses -= 1

            num_guesses += 1

            #if guess == secret_num:
            #    break;
            if num_guesses > MAX_GUESS:
                print('You ran out of guesses. The answer was %s.' % (secret_num))
                print("Please enter your name (FOR HIGHSCORE): ")
                highscore_name = input()

                f = open('./highscore.txt', 'w')
                f.write(highscore_name + "," + str(NUM_DIGITS))
                f.close()
                break;

    print('Do you want to play again? (yes or no)')
    if not input().lower().startswith('y'):
        break

As the disp_intro says. It will give clues depending on the input given.

  Bagels - None of the digits is correct.
  Pico - One digit is correct but in the wrong position.
  Fermi - One digit is correct and in the right position.

So, basically I just want to see the most concise way this can be written as I'm pretty sure most of the code I wrote is redundant and can be improved by a lot.

Features

[X] Generates random SECRET_NUM to guess.
[X] Shows current High Score.
[X] Read/Writes the High Score file.
[X] If length is too long: Do not count tries, and ask again.
[X] If it's not a number: Do not count tries, and ask again.
[X] If guess >=  MAX_GUESS: Ask for player's name, and then ask to terminate.

There might be more, but I think those are the most important ones.

There are a few problems with this code.

In disp_intro() you're using global HIGHSCORE, but it should actually be global HIGHSCORE_DATA
In case highscore.txt doesn't exist, the whole code will fail when trying to access HIGHSCORE_DATA[0] and HIGHSCORE_DATA[1], so you should have some defaults, like HIGHSCORE_DATA = ['', 0]
You're not closing the highscore.txt file when reading (use with) and f is not a nice name (the same goes when writing it).
You don't need another variable for the high score, it's already what you need
When reading the highscore file you probably want rstrip() to remove the extra newline you may (or may not) have

I think it's better:

if os.path.isfile('./highscore.txt'):
    with open("highscore.txt", "r") as highscore_file:
        HIGHSCORE_DATA = highscore_file.readline().rstrip().split(',')

In disp_intro() you also need global NUM_DIGITS
You don't need your own function to check if it's all digits, you have the isdigit() function
Put the main loop in a main() function.

Call it with:

if __name__ == '__main__':
    main()

See here

I think the get_clues() function is a bit chaotic. It's not consistent because you're returning either a code (ERROR_LENGTH_TOO_LONG) or a description (You got it!). You're also using the same function to determine if you need to increment the difficulty or not. Also, the caller uses the returned description to check if the player has guessed correctly, which is actually the same check you're doing inside of the function.

You're probably better off having a function called compare_guess() which only returns code and the print the description of those codes using a dictionary.

If this is a school exercise and you still haven't learnt how to use dictionaries, you can still do it with two arrays, one for the codes and the other for the descriptions. I think that would still be better than mixing the two things.

There are also quite a few pep8 things to fix, but that's a minor concern right now, you should first fix the other issues.


Codeigniter vote routine review I have a model that I've created in Codeigniter to represent a vote. The purpose of the site that it's a part of is to allow users to vote on their preferred pronunciation for a given word.

The routine first checks to see if a user has registered a vote for the given pronunciation and if so, whether it was up or down. Just like the stackexchange network sites, you can only vote either up or down for a given entry and you can only vote once in either direction. Am I doing this wrong? Is there a simpler way to accomplish what I'm trying to do here (such as adding a method or two)? If there's anything else that you'd like to see that I haven't posted, let me know.

public function vote($pronun_id, $user_id, $direction) {
        $data = array(
                'pronun_id' => $pronun_id,
                'user_id' => $user_id,
                'direction' => $direction
            );
        $already_voted = $this->_vote_exists($pronun_id, $user_id);

        if($already_voted == 'up') {
            if($direction == 'up') {
                return false;
            }
            else {
                $this->db->where(array('user_id' => $user_id, 'pronun_id' => $pronun_id));
                $this->db->update('vote', array('direction' => $direction));
                return true;
            }
        }
        elseif($already_voted == 'down') {
            if($direction == 'down') {
                return false;
            }
            else {
                $this->db->where(array('user_id' => $user_id, 'pronun_id' => $pronun_id));
                $this->db->update('vote', array('direction' => $direction));
                return true;
            }
        }
        else {
            $this->db->insert('vote', $data);
            return true;
        }
    }

    private function _vote_exists($pronun_id, $user_id) {
        $data = $data = array(
                'pronun_id' => $pronun_id,
                'user_id' => $user_id
            );
        $query = $this->db->get_where('vote', $data);

        if($query->num_rows() > 0) {
            $row = $query->row_array();
            return $row['direction'];
        }
        else {
            return false;
        }
    }

Update: Also, I'm very new to Codeigniter, so please feel free to be brutal about anything here. I'm not the best programmer and I also just don't know Codeigniter "style" yet, so either of those may be issues here.

You have some cut and pasted code which can be re-factored by comparing already_voted with the direction rather than with 'up' and 'down' specifically.

if ($already_voted === false) {
    $this->db->insert('vote', $data);
    return true;
}
elseif ($already_voted == $direction) {
    return false;
}
else {
    $this->db->where(array('user_id' => $user_id, 'pronun_id' => $pronun_id));
    $this->db->update('vote', array('direction' => $direction));
    return true;
}


Securely shuffling an array with 75 numbers in Go I have a SHA512 hex string consisting of my random input and that of the user.

Since rand.Seed only accepts a int64 I can never generate enough possibilities to shuffle an array with numbers 1-75: 75! = 2.480914e+109.

So my plan was to loop over the hex string 11 times, each time converting 10 hex characters to decimal and keep on shuffling the numbers.

func Shuffle(t []int, hex string) {
    for j := 0; j < 11; j++ {
        shuffleSeed, _ := strconv.ParseInt(hex[j*10:(j+1)*10], 16, 64)
        rand.Seed(shuffleSeed)
        for i := 1; i < len(t); i++ {
            r := rand.Intn(i + 1)
            if i != r {
                t[r], t[i] = t[i], t[r]
            }
        }
    }
}

items := []int{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15,
    16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30,
    31, 32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45,
    46, 47, 48, 49, 50, 51, 52, 53, 54, 55, 56, 57, 58, 59, 60,
    61, 62, 63, 64, 65, 66, 67, 68, 69, 70, 71, 72, 73, 74, 75}

hex := "099b5721eaa9b5bcd87a07366aa175a1fb5b6fa6e5671b93ef443df927474198ec2ae959928f95c46f9fe9621575aa4e1e6f70780b059044da17737fcc99a322"
Shuffle(items, hex)

What do you think about this? I'm not a cryptography expert, not a math expert and certainly not a Go expert (I started yesterday).

Taken from another post I shuffled an array with [1,2,3] and here are the occurrences of each after 1 million times:

167136
166403
166877
166520
166925
166145

Initialization

A simple loop would be easier to maintain than hard-coding an array of 75 values.

items := make([]int, 75, 75)
for i := range items {
    items[i] = i + 1
}

Seeding

The PRNG should be seeded just once.

You are correct that an Int64 seed is "only" capable of producing 264 ≈ 1019 distinct sequences, which is well under 75! ≈ 10109.

However, you should also consider the magnitude of the numbers involved.  The age of the Universe is well under 1018 seconds.  There is therefore no way you could remotely come close to producing a small fraction of the 75! possible permutations, just due to lack of time.  A 64-bit seed is more than enough to generate as many unreproducible sequences as you could ever want.

So, calling rand.Seed(time.Now().UnixNano()) just once in func init() will be sufficient.


Sheet display in function of date and time on open and save Here's a simple one for u guys,
The code is called upon workbook_open and BeforeSave, it displays the right sheet in function of date and time (Day vs night shift vs date). The code run trough the sheet and activate it if the date on sheet is the same as the actual date. Since there's 2 sheets with the same date (day and night shift) i had to add a time condition. The nights shifts are before the day one (look at image) so i had to split my loop in 2 to make it work (could maybe be one). Also, if we're not in the week of that report, so that no sheets has a date that fits, i wanted it to select Friday cause it has a report on it. Since its called often i wanted to make sure that it is optimal.

Code :
Sub selectiondequartauto()
' Code that display sheet in fucntion of date and time

    Dim count As Long        ' Counter
    Dim xSh As Worksheet
    Dim activation as Boolean
    Dim sheetdate As Date

    Dim limitpm As Variant
    Dim limitam As Variant
    limitpm = TimeValue("16:15:00")  ' Night shift Start
    limitam = TimeValue("3:00:00")   ' Night shift end

    For Each xSh In Worksheets                   ' Loop on every visible sheet
        If xSh.Visible Then
           sheetdate = xSh.Cells(4, 2).Value ' Gives sheetdate the date of the sheet in present loop

            If TimeValue(Now) < limitpm And TimeValue(Now) > limitam Then

                If count < 1 Then                ' If count is smaller than 1 (no sheet w same date as now yet)

                    If sheetdate = Date Then     ' Si Sheetdate = actual date
                        count = count + 1        ' Count = 1 and next sheet, because 2 sheet have sheetdate = actual date
                    End If
                ElseIf count = 1 Then            ' Si count = 1
                    Activation = True
                    xSh.Activate                 ' Activate curent loop sheet and quit
                    Exit For
                End If

            Else  ' If we are in night shift time frame

                If sheetdate = Date Then
                    Activation = True
                    xSh.Activate                ' Activate curent loop sheet and quit
                    Exit For
                End If
            End If
        End If

    Next
    If Activation = False Then
        Sheets("Vendredi jour").Activate
    End If
End Sub

selectiondequartauto() defines night shift to determine whether to select either the night shift or day shift sheet.  If none of the worksheets meet the criteria of Night Shift or Day Shift then the Friday worksheet is selected.  This all works fine but consider the advantages of my refactored code which separates these tasks into their own methods.
• isNightShift: Using a function that determines what is Night Shift and what is Day Shift means that you can update the hours of Night Shift in one spot and not have to look over every procedure that is dependent on on the Night Shift.
• NightShiftSheet() and DayShiftSheet():  Having these functions will prevent you from having to repeat the logic to find the sheets in the future.  They also make  selectiondequartauto() easier to read
Public Sub selectiondequartauto()
    Dim ws As Worksheet

    If isNightShift(Now) Then
        Set ws = NightShiftSheet
    Else
        Set ws = DayShiftSheet
    End If

    If ws Is Nothing Then
        Sheets("Vendredi jour").Activate
    Else
        ws.Activate
    End If
End Sub

Public Function DayShiftSheet() As Worksheet
    Dim ws As Worksheet

    For Each ws In ThisWorkbook.Worksheets
        With ws.Range("B4")
            If IsDate(.Value) Then
                If Not isNightShift(.Value) Then
                    Set DayShiftSheet = ws
                    Exit Function
                End If
            End If
        End With
    Next

End Function

Public Function NightShiftSheet() As Worksheet
    Dim ws As Worksheet
    For Each ws In ThisWorkbook.Worksheets
        With ws.Range("B4")
            If IsDate(.Value) Then
                If isNightShift(.Value) Then
                    Set NightShiftSheet = ws
                    Exit Function
                End If
            End If
        End With
    Next
End Function

Public Function isNightShift(DateTime As Date) As Boolean
    isNightShift = TimeValue(DateTime) > TimeValue("3:00:00") And TimeValue(DateTime) < TimeValue("16:15:00")
End Function


Authentication via LDAP and authorization via JDBC I have a custom AuthenticationProvider for getting authentication via LDAP and authorization via JDBC. It's running but I think it's not the best way.

My custom AuthenticationProvider:

public class ADAuthenticationJDBCAuthorizationProvider implements AuthenticationProvider {

    private final ActiveDirectoryLdapAuthenticationProvider adDelegate;

    @Autowired
    private UtilisateurService utilisateurService;

    public ADAuthenticationJDBCAuthorizationProvider(ActiveDirectoryLdapAuthenticationProvider adDelegate) {
        this.adDelegate = adDelegate;
    }

    @Override
    public Authentication authenticate(Authentication authentication) {
        final Authentication a = adDelegate.authenticate(authentication);
        final String name = a.getName().toLowerCase();

        // Load additional authorities and create an Authentication object
        final Collection<GrantedAuthority> authorities = utilisateurService.loadRolesFromDatabase(name);
        final boolean isActive = utilisateurService.isActive(name);

        return new AbstractAuthenticationToken(authorities) {
            @Override
            public Object getCredentials() {
                return a.getCredentials();
            }

            @Override
            public Object getPrincipal() {
                return a.getPrincipal();
            }

            @Override
            public boolean isAuthenticated() {
                return isActive && a.isAuthenticated();
            }

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

        };
    }

    @Override
    public boolean supports(Class<?> authentication) {
        return adDelegate.supports(authentication);
    }
}

My configuration file:

public class GlobalSecurityConfig extends GlobalAuthenticationConfigurerAdapter {

    @Value("${ldap.domain}")
    private String DOMAIN;

    @Value("${ldap.url}")
    private String URL;

    @Autowired
    private JDBCUserDetailsService userDetailsService;

    @Override
    public void init(AuthenticationManagerBuilder auth) throws Exception {
auth.authenticationProvider(activeDirectoryLdapAuthenticationProvider()).userDetailsService(userDetailsService);
    }

    @Bean
    public AuthenticationProvider activeDirectoryLdapAuthenticationProvider() {
        ActiveDirectoryLdapAuthenticationProvider adAuthenticationProvider
                = new ActiveDirectoryLdapAuthenticationProvider(DOMAIN, URL);
        adAuthenticationProvider.setConvertSubErrorCodesToExceptions(true);
        adAuthenticationProvider.setUseAuthenticationRequestCredentials(true);

    ADAuthenticationJDBCAuthorizationProvider authenticationProvider = new ADAuthenticationJDBCAuthorizationProvider(adAuthenticationProvider);

        return authenticationProvider;
    }
}

The custom UserDetailsService:

public class JDBCUserDetailsService implements UserDetailsService {

    @Autowired
    private UtilisateurService utilisateurService;

    @Override
    public UserDetails loadUserByUsername(String username) throws UsernameNotFoundException {
        final String name = username.toLowerCase();
        Collection<GrantedAuthority> authorities = utilisateurService.loadRolesFromDatabase(name);
        final boolean isActive = utilisateurService.isActive(name);

        return new User(username, "", isActive, true, true, true, authorities);
    }
}

Do you have any suggestion for a better solution?

By using a framework like Spring Security you have all you cicle already defined and your task is just to customize the parts you need that should be integrated with your application and need some logic for that.

First of all I would raise here a good practice that Spring developers don't follow.
In documentation you always found code like:

@Autowired
private UtilisateurService utilisateurService;

That is good for Spring to show you a cool and quick short way to inject stuff in your class avoiding boilerplate code.

Said that, it is not good in production code.

You should, instead, write a proper construction method for a mandatory dependency:

private UtilisateurService utilisateurService;

@Autowired
public ADAuthenticationJDBCAuthorizationProvider(ActiveDirectoryLdapAuthenticationProvider adDelegate, UtilisateurService utilisateurService) {
    this.adDelegate = adDelegate;
    this.utilisateurService = utilisateurService;
}

Or a setter if the dependency is not mandatory:

private UtilisateurService utilisateurService;

@Autowired
public void setUtilizateurService(UtilisateurService utilisateurService) {
    this.utilisateurService = utilisateurService;
}

The reason to do so is to expose your dependencies to the user of your class, instead of hiding.

You should just hide your data in the classes, not the dependencies.

Showing up all dependencies make your code easy to use and test.

Another point I would like to rise is in your UserDetailsService implementation:

@Override
public UserDetails loadUserByUsername(String username) throws UsernameNotFoundException {
    final String name = username.toLowerCase();
    Collection<GrantedAuthority> authorities = utilisateurService.loadRolesFromDatabase(name);
    final boolean isActive = utilisateurService.isActive(name);

    return new User(username, "", isActive, true, true, true, authorities);
}

I think you should avoid to instantiate here a UserDetails object.

Instead you should have a builder that initialize the object as needed for you:

public class User implements UserDetails {
    // ... code ...

    public static User from(String userName, boolean isActive, List<GrantedAuthority> authorizations) {
        User user = new User(userName, "", isActive, true, true, true, authorizations);
    }
}

...

@Override
public UserDetails loadUserByUsername(String username) throws UsernameNotFoundException {
    final String name = username.toLowerCase();
    Collection<GrantedAuthority> authorities = utilisateurService.loadRolesFromDatabase(name);

    return User.from(username, utilisateurService.isActive(name), authorities);
}

In this way you can ensure to have just valid users, and you have a single point in your code where you handle what a valid user is.


How to write a stable WCF service I'm writing a WCF service for validating password and getting company ID for the clients that requests the services. The service runs on a server locally as a windows service, which sends queries to the database and returns the appropriate info back to the client.

I'd first like some comments on how the overall design looks like as I'm going to be adding more operations but wanted to avoid any situations where I have to rewrite them all just in case if I'm headed the wrong direction.

I have a specific question regarding the stability of this service. In case if the operation somehow fails on the server's end, such as an exception being thrown because the server couldn't connect to a database.

Where should I handle the exception? Should I wrap my methods in try catch statements on the service methods or where the call is being made (client)?
What can I do to increase the stability of this service? Under any circumstances, I cannot allow the service to shut down aside from the user intentionally doing so.

These are probably trivial questions but I'd like some guidance on this matter. Thank you.

namespace TopwareOutlookSyncService
{
    [ServiceContract]
    public interface ITopwareOutlookSyncService
    {
        [OperationContract]
        bool IsPasswordValid(string password);

        [OperationContract]
        string GetCompanyID(string userID);
    }
    public class TopwareOutlookSyncService : ITopwareOutlookSyncService
    {
        private SqlConnection con;
        public void ConnectDB()
        {
            if (!IsDBConnected())
            {
                con = new SqlConnection("user id=nexol;" +
                                           "password=;server=666.666.666.666;" +
                                           "database=hell; " +
                                           "connection timeout=15");
                con.Open();
            }
        }

        public bool IsDBConnected()
        {
            return (con != null && con.State == ConnectionState.Open);
        }

        public bool IsPasswordValid(string userID, string password)
        {
            if (!IsDBConnected())
                ConnectDB();
            bool isPasswordValid = false;

            string sql = "SELECT USER_PW FROM HELL.GRP_USER WHERE USER_ID = '" + userID + "'";
            string encryptedPassword = PasswordTable.GetEncryptedString(password);
            SqlCommand command = new SqlCommand(sql, con);
            SqlDataReader reader = command.ExecuteReader();

            while (reader.Read())
            {
                if (reader["USER_PW"].ToString() == encryptedPassword)
                {
                    isPasswordValid = true;
                    break;
                }
            }

            reader.Close();
            con.Close();
            return isPasswordValid;
        }

        public string GetCompanyID(string userID)
        {
            if (!IsDBConnected())
                ConnectDB();
            string companyID = String.Empty;
            string sql = "SELECT COM_CD FROM HELL.GRP_USER WHERE USER_ID = '" + userID + "'";
            SqlCommand command = new SqlCommand(sql, con);
            SqlDataReader reader = command.ExecuteReader();

            while (reader.Read())
            {
                companyID = reader["COM_CD"].ToString();
            }

            reader.Close();
            con.Close();
            return companyID;
        }
    }
}

You should never generate dynamic SQL by concatenating incoming values into an SQL string that is executed later. This type of code leads to SQL injection vulnerability, and by using your service as it is now it's very easy to get access to any user account without knowing its password.

If you plan to develop your service further take your time to learn one of the ORM frameworks (Entity Framework, NHibernate, or maybe one of the NoSQLs like MongoDB or RavenDB) and get rid of low-level DB management code from your service. Alternatively (if you prefer inventing a wheel :)) move DB-related code to separated class and name it "DB repository" so that your service doesn't have too many responsibilities (currently it has business logic responsibilities combined with DB access management).

About exception handling - here is a good article that describes what you should and should not do. Basically you should only catch specific exceptions, and only if you know how to recover from that exception in a meaningful way, that doesn't leave your application in an inconsistent state. In case when WCF service has no way to return a meaningful response (e.g. when response is based on DB data and database is not available) you should pass exception to the client.

And finally, store your connection strings in the configuration files rather than hardcoded.


Phone number to words based on the dictionary This is the problem:

  Given a 10 digit phone number, you must return all possible words or combination of words from the provided dictionary, that can be mapped back as a whole to the number.

  With this we can generate numbers like 1-800-motortruck which is easier to remember then 1-800-6686787825

  The phone number mapping to letters is as follows:

2 = a b c
3 = d e f
4 = g h i
5 = j k l
6 = m n o
7 = p q r s
8 = t u v
9 = w x y z

  The phone numbers will never contain a 0 or 1.  Words have to be at least 3 characters.

  To get give you an initial verifiation, the following must be true:

  6686787825 should return the following list [["motor", "usual"], ["noun", "struck"], ["nouns", "truck"], ["nouns", "usual"], ["onto", "struck"], "motortruck"] # These words exist in a dictionary file.
  The conversion of a 10 digit phone number should be performed within 1000ms.

My solution works but takes longer than 1 sec.
How can I improve my code so execution time will be less than 1 sec?

class NumberToWord

  def letter_combinations(digits)
    #return if number not valid
    return [] if digits.nil? || digits.length != 10 || digits.split('').select{|a|(a.to_i == 0 || a.to_i == 1)}.length > 0
    #number to letters mapping
    letters = {"2" => ["a", "b", "c"],"3" => ["d", "e", "f"],"4" => ["g", "h", "i"],"5" => ["j", "k", "l"],"6" => ["m", "n", "o"],"7" => ["p", "q", "r", "s"],"8" => ["t", "u", "v"],"9" => ["w", "x", "y", "z"]}

    # Read dictionary file and hold all values in a array
    dictionary = []
    file_path = "dictionary.txt"
    File.foreach( file_path ) do |word|
      dictionary.push word.chop.to_s.downcase
    end
    # get all letters for numbers in form of array
    keys = digits.chars.map{|digit|letters[digit]}

    results = {}
    total_number = keys.length - 1 # total numbers
    #Loo through all letters and get matching records with dictionary
    for i in (2..total_number)
      first_array = keys[0..i]
      second_array = keys[i + 1..total_number]
      next if first_array.length < 3 || second_array.length < 3
      first_combination = first_array.shift.product(*first_array).map(&:join) # Get product of arrays
      next if first_combination.nil?
      second_combination = second_array.shift.product(*second_array).map(&:join)
      next if second_combination.nil?
      results[i] = [(first_combination & dictionary), (second_combination & dictionary)] # get common values from arrays
    end
    #arrange words like we need as a output
    final_words = []
    results.each do |key, combinataions|
      next if combinataions.first.nil? || combinataions.last.nil?
      combinataions.first.product(combinataions.last).each do |combo_words|
        final_words << combo_words
      end
    end
    # for all numbers
    final_words << (keys.shift.product(*keys).map(&:join) & dictionary).join(", ") # matche with all character
    final_words
  end

end

final_words = NumberToWord.new().letter_combinations("6686787825")
print final_words

I would first start by doing some basic profiling of your application to see where the issues are. This could be as simple as inserting console.log( (new Date()).valueOf() ) in a few places.

Some things I noticed.

Is your dictionary file sorted? If not then sort it first. Either way use bsearch to locate entries rather han using & which is very naive.
It looks like you are trying every combination of letters but you probably want to use an algorithm that stops if there is no match. For example given 668, if your dictionary contains no entries starting with mn (66) then there is no point in checking mnt, mnu, mnv` or any of the longer combinations.
Try to reduce the number of times you copy and modify arrays ([a..b] or shift)
This check: next if first_array.length < 3 || second_array.length < 3 could go before you copy the array (in a slightly modified form). You could just change for i in (2..total_number) to for i in (4..total_number-2)


Reorder columns in an HTML form I have the following HTML structure for a form (simplified):

<form class="form">

First

Last

Whatever

City

Country

Phone

</form>

As I don't have any control over the output, I need to reorder the columns by using JavaScript:

const form = (() => {
  const colClassName = 'formCol';
  const rowClassName = 'formRow';
  const clearClassName = 'clear';

  function createRow(elForm) {
    const elRow = document.createElement('div');
    const elClear = document.createElement('div');
    elRow.className = rowClassName;
    elClear.className = clearClassName;
    elRow.appendChild(elClear);
    elForm.insertAdjacentElement('beforeEnd', elRow);

    return elRow;
  }

  function moveColumn(elForm, elRow, colName) {
    const elCol = elForm.querySelector(`.${colClassName}[data-id='${colName}']`);
    elRow.insertAdjacentElement('afterbegin', elCol);
  }

  function removeEl(el) {
    el.parentNode.removeChild(el);
  }

  function reorder(elForm, order) {
    const elRows = elForm.querySelectorAll(`.${rowClassName}`);
    const maxRows = Math.max(elRows.length, order.length);
    let elCol;

    for (var i = 0; i < maxRows; i++) {
      if (order[i]) {
        if (!elRows[i]) {
          elRows[i] = createRow(elForm);
        }

        if (!Array.isArray(order[i])) {
          moveColumn(elForm, elRows[i], order[i]);
        } else {
          for (j = order[i].length - 1; j >= 0; j--) {
            moveColumn(elForm, elRows[i], order[i][j]);
          };
        }
      } else {
        elCol = elRows[i].querySelector(`.${colClassName}`);

        if (!elCol) {
          removeEl(elRows[i]);
        }
      }
    }
  }

  return {
    reorder,
    // other functions for form
  };
})();

form.reorder(document.querySelector('.form'), [
  ['Last', 'City'],
  'First',
  'Country',
  'Phone',
]);

It does the following:

Reorders the elements as specified in the array (multiple columns).
Does not delete columns if they are not in the array (see column "Whatever").
Creates new rows if necessary and deletes empty rows

While that works as is (see https://jsfiddle.net/6cy4mjvd/), I don't know if there are ways to improve readability / code in general, e.g. it feels wrong how I currently create new rows and modify elRows and also that I  pass elForm to all helper functions... and probably a few other spots.

Btw. I plan on adding other functions other than reorder to modify the form. That's why I return an object (line // other functions for form) in the function.

@demrks please take a look at the code below. It's a result of multiple refactoring iterations.

While the original code does the job, I find it very hard to read. Hopefully, you agree that the improved version reads better, even though there's very little new in it. Many developers including myself believe that readability is a super important aspect of coding because it directly affects complexity, and thus correctness of the code.

Here's the list of techniques I applied (in order of importance as I understand things):

Naming

Name things properly and do not abbreviate words. createNewRowElementAndInsertIntoForm(...) is definitely not a short name, but it's describing what it does with 100% precision.

Similarly, original elCol transformed into columnElement. It may be funny, but first seeing elCol I thought I'm reading code in Spanish. :)

Please never never never use the i, j, c, and similar. They are absolutely meaningless. Yes, the other dev "may understand" it, but you don't want anyone to guess what the code does. Spell things out, in most of the modern editors it's a matter of hitting <F2>.

Temporary variables constants

Do not hesitate to introduce reusable temporary constants with descriptive names. They make cryptic conditions if (order[i]) or if (!elRows[i]) relay the intent of the writer.

Avoid branching when possible, or minimize its "scope"

An unnecessary if-then-else for processing array/non-array orders may be generalized, if we normalize the array first.

Was:

if (!Array.isArray(order[i])) {
  moveColumn(elForm, elRows[i], order[i]);
} else {
  for (j = order[i].length - 1; j >= 0; j--) {
    moveColumn(elForm, elRows[i], order[i][j]);
  };
}

Became (longer because of the name, but the branching is almost eliminated by a ternary operator that cleans the input):

const normalizerRowPrescribedOrder =
  Array.isArray(rowPrescribedOrder) ? rowPrescribedOrder : [rowPrescribedOrder];

for (let columnIndex = normalizerRowPrescribedOrder.length - 1; columnIndex >= 0; columnIndex--) {
  findColumnByNameAndMoveToBeginningOfRow(formElement, currentRowElement, normalizerRowPrescribedOrder[columnIndex]);
};

"Newspaper"

Make code read top to bottom aka newspaper style (reorder() is the entry point which invokes createNewRowElementAndInsertIntoForm(), therefore reorder() should go first). Forcing the reader to jump around the code is distracting from the act of active reading, therefore leading to avoidable complications.

Result

const form = (() => {
  const columnClassName = 'formCol';
  const rowClassName = 'formRow';
  const clearClassName = 'clear';

  function reorderForm(formElement, prescribedOrder) {
    const rowElementList = formElement.querySelectorAll(`.${rowClassName}`);
    const maxRowLength = Math.max(rowElementList.length, prescribedOrder.length);

    for (let rowIndex = 0; rowIndex < maxRowLength; rowIndex++) {
      const currentRowElement = rowElementList[rowIndex] || createNewRowElementAndInsertIntoForm(formElement);

      const rowPrescribedOrder = prescribedOrder[rowIndex];
      if (!rowPrescribedOrder) {
        const columnElement = currentRowElement.querySelector(`.${columnClassName}`);
        if (!columnElement) {
          removeRowElement(currentRowElement);
        }
      } else {
        const normalizedRowPrescribedOrder = Array.isArray(rowPrescribedOrder) ? rowPrescribedOrder : [rowPrescribedOrder];
        for (let columnIndex = normalizedRowPrescribedOrder.length - 1; columnIndex >= 0; columnIndex--) {
          findColumnByNameAndMoveToBeginningOfRow(formElement, currentRowElement, normalizedRowPrescribedOrder[columnIndex]);
        };
      }
    }
  }

  function createNewRowElementAndInsertIntoForm(formElement) {
    const rowElement = document.createElement('div');
    rowElement.className = rowClassName;

    const clearElement = document.createElement('div');
    clearElement.className = clearClassName;

    rowElement.appendChild(clearElement);

    formElement.insertAdjacentElement('beforeEnd', rowElement);

    return rowElement;
  }

  function findColumnByNameAndMoveToBeginningOfRow(formElement, rowElement, columnName) {
    const columnElement = formElement.querySelector(`.${columnClassName}[data-id='${columnName}']`);
    rowElement.insertAdjacentElement('afterbegin', columnElement);
  }

  function removeRowElement(rowElement) {
    rowElement.parentNode.removeChild(rowElement);
  }

  return {
    reorderForm,
    // other functions for form
  };
})();

form.reorderForm(document.querySelector('.form'), [
  ['Last', 'City'],
  'First',
  'Country',
  'Phone',
]);

P.S.

It's really nice to see how one uses native API instead of jQuery!


Will this code block in node.js When reigstering a user I request two reads from the database to see if there is a username and email and then I write if the checks pass.. Can anyone tell me if this code will block other users?

I am using node with express.js and mongojs.

I know I am saving passwords in plaintext.. This will be changed.

app.post('/register', function(req, res) {

    var checked = 0;
    var errors = [];

    var finishedCheck = function() {
        checked++;
        register();
    }

    var register = function() {

        if(checked === 2){

            if(errors.length > 0) {
                console.log('errors', errors);
                res.statusCode = 409;
                res.send(errors);
            } else {

                var newuser = {
                    username: req.body.username,
                    uppercase: req.body.username.toUpperCase(),
                    password:req.body.password,
                    email: req.body.email.toLowerCase(),
                    userLevel: 0,
                    createdOn: new Date()
                };

                db.users.save(newuser, function(err, val){
                    if(!err) {
                        req.session.userid = val['_id'];
                        res.end();
                    } else {
                        res.statusCode = 406;
                        res.end(['Something went wrong', err]);
                    }
                });
            }
        }
    }

    var check = function(username, email) {
        console.log('chekc');
        db.users.ensureIndex({email:1},{unique:true});
        db.users.ensureIndex({uppercase:1},{unique:true});
        if(!username.match(/^[A-Za-z0-9_]*$/)) {
            errors.push('Username is invalid');
            finishedCheck();
        } else {
            db.users.findOne({uppercase: username}, function(err, val) {
                if(val) {
                    errors.push('Username Taken');
                }
                finishedCheck();
            });
        }

        //check if the is a valid email address if so then check to see if its already registered
        if(!email.match(/^[-a-z0-9~!$%^&*_=+}{\'?]+(\.[-a-z0-9~!$%^&*_=+}{\'?]+)*@([a-z0-9_][-a-z0-9_]*(\.[-a-z0-9_]+)*\.(aero|arpa|biz|com|coop|edu|gov|info|int|mil|museum|name|net|org|pro|travel|mobi|[a-z][a-z])|([0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}))(:[0-9]{1,5})?$/i)) {
            errors.push('Email is invalid');
            finishedCheck();
        } else {
            db.users.findOne({email: email}, function(err, val) {
                if(val) {
                    errors.push('Email is already in the database');
                }
                finishedCheck();
            });
        }

    };

    check(req.body.username.toUpperCase(), req.body.email.toLowerCase());

});

No

app.post('/register'...

Has a callback and is non-blocking within Node

db.users.save(...

Has a callback and is non-blocking within Node (but will block on your database level, which is normal)

db.users.findOne(...

Has a callback and is non-blocking within Node (but will block on your database level, which is normal)

Your code looks pretty normal. MongoDB has a global lock across an entire mongoDB server daemon, so this is the only level that your current code will block. It will not block on your Node.js level for your concurrent users.


PHP user account login with salt I'm creating a user account login with PHP and Mysql which will be used by school teachers. To login there are 3 items required : school, username and password. This will allow for 2 of "MRSMITH" in two different schools.

The database is:

schools.login_name is the school part of the login
admin_users.username this is the teacher's username
admin_users.pass_hash(&salt) is the password and its salt

// salt / hash creation - returned SALT & HASH are inserted into user MySQL table
function hash_password($password)
    {
    $return_array=array();

    $salt = mcrypt_create_iv(32, MCRYPT_DEV_RANDOM);
    $hexsalt = bin2hex($salt);

    $pass_hash = hash('sha256', $password . $hexsalt);

    $return_array['SALT'] = $hexsalt;
    $return_array['HASH'] = $pass_hash;
    return $return_array;
    }

(Note, the salt is unique and specific to each user if is is not clear from the above.)

/*
<input type='text' name='school_login_id'>
<input type='text' name='admin_login_id'>
<input type='password' name='admin_password'>

login form above, inouts from $_POST are checked for SQL inject before being passed to the function below to validate login

*/

function validate_login_details($school_login_id, $admin_login_id, $admin_password)
    {
    global $schools_table, $admin_users_table;
    $return_result = false; // invalid unless proven otherwise

    // find user and their pass hash and salt
    $query = "select S.school_id, pass_hash, pass_salt
        from $schools_table as S join $admin_users_table as A using(school_id)
        where S.login_name = '$school_login_id' and A.username ='$admin_login_id'";

    log_this($query, "loginQueries.log"); // temp debug

    $login_res = query_db($query); // wrapper function for MYSQL_QUERY - yes, i will stop using this i promise!
    if (mysql_num_rows($login_res) == 1)
        {
        $db_pass_hash = mysql_result($login_res, 0, "pass_hash");
        $db_pass_salt = mysql_result($login_res, 0, "pass_salt");

        // re-create hash from supplied password and compare to d/b
        $hash_from_entered_pass = hash('sha256', $admin_password . $db_pass_salt);
        if ($hash_from_entered_pass == $db_pass_hash)
            {
            $return_result = true;
            }
        }
    else
        {
        log_this("mysql_num_rows(\$login_res) not 1 row", "loginfails.log"); // temp debug
        }

    return $return_result;
    }

Before data is passed to  validate_login_details() it is checked against these regexps:

School and user login ID:

    if (preg_match("/[^-a-z0-9_]/i", $text) )
        {
            $error_count++;
        }

Password:

$match_list = "\x{20}-\x{5f}\x{61}-\x{7e}"; // basic ascii chars excluding backtick
$match_list .= "\x{a1}-\x{ff}"; // extended latin 1 chars excluding control chars
$match_list .= "\x{20ac}\x{201c}\x{201d}"; // euro symbol & left/right double quotation mark (from Word)
$match_list .= "\x{2018}\x{2019}"; // left/right single quotation mark (from word)

if (preg_match("/[^$match_list]/u", $text) )
    {
    $error_count++;
    }

Is this code correct and secure? I have seen many more complex has generation functions, but given that there are no credit cards or personal details involved, is this robust enough?

Security

  Is this code correct and secure?

Probably not.

EDIT: I just read your comments about the input being checked elsewhere (which might invalidate my answer somewhat, depending on how you check the user input). You should really check user input right before database interaction. Otherwise it will become unmaintainable really quick (you will always ask yourself if you already checked it, and you will inevitably forget to do it in some places, as you falsely believe you already checked them).

SQL Injection

You are missing the part where you actually retrieve user input, but the code you did post looks pretty open to SQL injection (which in general means that an attacker can for example bypass a login, can steal data from the database, and possibly write to a file/read from a file, change data, etc).

You should really stop using mysql immediately, and you should start using prepared statements (either with mysqli or PDO).

Concrete Attacks

For your concrete query, it might not look like an attacker can do all that much with this injection (the data is not displayed, and the login actually cannot be bypassed). But that is actually not true, as blind SQL injection is possible if the attacker knows a valid school and login id (blind sql injection means that you ask true/false questions, eg "is the first character of the first user in ascii smaller than 100? yes? is it also smaller than 60?", and so on; this does generate quite a bot of requests, but getting the database user or admin is certainly doable in short amount of time). You should be able to test this by injecting $school_login_id = valid id and $admin_login_id = valid id ' and 1=1 # (you might need a valid password as well) and then doing the same thing again with 1=2, which should give you a different result (meaning you can post yes/no questions to the database).

If the database user is allowed to write to a file, that would also be a possible attack vector.

Also, it should be possible to login as any user if the attacker knows the password of a specific user (by database id). An injection could look like this: $school_login_id = desired school name, $admin_login_id = desired user id' or 1=1 limit 1,1 # with the password not of the desired user, but of the first user in the database (this becomes a lot easier if you expose database ids to endusers).

To summarize: If you do not use prepared statements (or at least something) to defend against SQL injections, there will always be a way to exploit it.

Hashing, Comparison, Misc

  given that there are no credit cards or personal details involved, is this robust enough?

For password hashing, bcrypt is generally recommended over simple one-round sha256 hash. Weak hashing might actually be ok for your security requirements, but as bcrypt is actually easier to use in PHP (it manages salts for you), and is more secure, why not use it?

Also, for hash comparisons, you should not use ==, but a timing safe function such as hash_equals to avoid timing attacks.

Also, currently it looks as if your query logs might be in a public directory. If so, that is a really bad idea (exposes database names, etc; invalid logins might be mistypes, also exposes valid logins).

Misc

be consistent with your names. Eg $school_login_id vs S.login_name (is it an integer, as id implies, or a string as name implies?).
your indentation is off, which makes it really hard to see what else belongs to what if.
I would get rid of $return_result. Just return false at the end, and in the middle return $hash_from_entered_pass == $db_pass_hash directly.
global variables generally hint towards bad design.


Dependency Injector Container in PHP I am practicing the SOLID principles and my first step was taking a dive into dependency injection(DI), so I created a class, a container for all DIs. It works, but I'm just not entirely sure if this is how I should go about doing it, although to me it seems fine.

interface ContainerInterface
{
    public function set($name, callable $value);
    public function get($name);
}

class DI implements ContainerInterface
{

    protected $services;

    public function set($name, callable $value)
    {
        $this->services[$name] = $value();
    }

    public function get($name)
    {
        return $this->services[$name];
    }

}

That covers the DI class, now I'll set an example of how I could use it:

$di = new DI();

$di->set('database', function() {
    return new Database();
});

$di->set('pagination', function() use ($di) {
    return new Pagination(DatabaseInterface $di->get('database'));
});

$application = new Application(ContainerInterface $di);

In the last line, I just injected the container and now I can access all the DIs.

A few thoughts:

Why not write anonymous functions in form of function($container) { ... } to allow your container to inject itself into function scope vs. having to use function () use ($di) { ... }?  This would allow wider compatibility to other forms of callables besides anonymous functions (for example, objects implementing __invoke() and other named functions/closures).
Alternately, if you truly want to narrow the type of callable functions that can be passed to set(), you might consider type-hinting against Closure class, which is what PHP in fact uses to represent a "function" created using closure syntax.
Why are you invoking the callable on set() and not on get()? This means you are pre-instantiating every dependency even if it is not needed for a particular code path. This kind of limits your framework to only allowing singleton-like behavior.  You could not pass a factory provider in one of your methods.  Perhaps it would be useful to have flag on set() if you want to implement singleton-like behavior, either by calling immediately or by storing information in separate arrays for "services" vs. single objects.
Consider explicitly instantiating your services property to empty array protected $services = [] such that IDE's (and readers) can infer type (that is if you are not going to use Doc blocks).
You might consider better enforcing/validating the $name parameter.  Right now, I could pass it pretty much anything, making your container fragile to outside input. If using PHP 7+, you should type-hint for string.
Is DI really a good class name? DIContainer perhaps?
Should you throw an exception if get() is called and key doesn't exist? I would guess that this would not be an expected condition in a well-written application and failing loudly in such a case will hopefully make it obvious to a developer that they have a problem in their code.
Is it OK that set() can overwrite existing item in the container?  You might consider preventing this in favor of needing an explicit unset() method to remove an item from the container.  This might suggest the need for a has() method.

For example:

public function has($name) {
    return array_key_exists($name, $this->services);
}

public function set($name, callable $value) {
    if($this->has($name)) {
        throw new Exception('...');
    }
    ...
}

public function get($name) {
    if(!$this->has($name)) {
        throw new Exception('...');
    }
    ...
}

public function unset($name) {
    unset($this->services[$name]);
}


Longest common substring approach using suffix arrays I am trying to speed up a function to return the longest common substring.

The requirements:

The function takes two strings of arbitrary length (although on average they will be less than 50 chars each)
If two subsequences of the same length exist it can return either
Speed is the primary concern

This is what I have and it works according to my tests:

from os.path import commonprefix

class LongestCommonSubstr(object):

    def __init__(self, lstring, rstring):
        self.lstring = lstring+'0'
        self.rstring = rstring+'1'
        self._suffix_str_array = sorted(self._get_suffix_str(self.lstring)
                                    + self._get_suffix_str(self.rstring))
        self.longest_common_substr = self._get_lcsubstr()

    @staticmethod
    def _get_suffix_str(string):

        return [string[i:] for i in range(len(string))]

    def _get_lcsubstr(self):

        try:
            substr_len =0
            max_len = 0
            lcs = None
            for i,n in enumerate(self._suffix_str_array):
                if n[-1] != self._suffix_str_array[i+1][-1]:
                    substr = commonprefix([n,self._suffix_str_array[i+1]])
                    substr_len = len(substr)
                    if substr_len > max_len:
                        max_len = substr_len
                        lcs = substr

        except IndexError:
            pass
            return lcs

Can the code be made faster in pure Python? Is there a better option to differentiate the input strings when made into one sorted list than concatenating an ID to them?

OK then: since your concern is speed, let's track our progress with actual timing data. The first step is to run the code through the Python profiler. With the addition of a bit of driver code that just calls LongestCommonSubstr().longest_common_substr 10000 times, I get the following results:

$ python3.4 -m profile lcs_profile.py abcdeffghiklnopqr bdefghijklmnopmnop
         1130008 function calls in 3.083 seconds

   Ordered by: standard name

   ncalls  tottime  percall  cumtime  percall filename:lineno(function)
        1    0.000    0.000    0.000    0.000 :0(__build_class__)
        1    0.000    0.000    3.082    3.082 :0(exec)
        1    0.000    0.000    0.000    0.000 :0(hasattr)
   280000    0.257    0.000    0.257    0.000 :0(len)
   260000    0.330    0.000    0.330    0.000 :0(max)
   260000    0.350    0.000    0.350    0.000 :0(min)
        1    0.000    0.000    0.000    0.000 :0(setprofile)
    10000    0.042    0.000    0.042    0.000 :0(sorted)
        1    0.000    0.000    0.000    0.000 <frozen importlib._bootstrap>:2264(_handle_fromlist)
   260000    1.067    0.000    1.746    0.000 genericpath.py:69(commonprefix)
        1    0.022    0.022    3.082    3.082 lcs_profile.py:1(<module>)
    20000    0.069    0.000    0.157    0.000 lcs_profile.py:11(_get_suffix_str)
    20000    0.071    0.000    0.071    0.000 lcs_profile.py:13(<listcomp>)
    10000    0.807    0.000    2.794    0.000 lcs_profile.py:15(_get_lcsubstr)
        1    0.000    0.000    0.000    0.000 lcs_profile.py:3(LongestCommonSubstr)
    10000    0.067    0.000    3.060    0.000 lcs_profile.py:4(__init__)
        1    0.000    0.000    3.083    3.083 profile:0(<code object <module> at 0x10996f030, file "lcs_profile.py", line 1>)
        0    0.000             0.000          profile:0(profiler)

The most important column is the second one, tottime, marking the total amount of time consumed by each function (but not the functions it calls). The largest entries in that column are for the commonprefix function and the _get_lc_substr function, so those are the spots you should focus on optimizing.

To keep track of our progress, I coupled your class with the following driver program which prints the execution time for 10000 runs:

def lcs_longest_match(a, b):
    return LongestCommonSubstr(a, b).longest_common_substr
if __name__ == '__main__':
    import timeit, sys
    print('lcs    ', timeit.timeit('lcs_longest_match(sys.argv[1], sys.argv[2])',
                                   setup='from __main__ import lcs_longest_match',
                                   number=10000))

(this is for Python 3). A first run on my computer gives

$ python3.4 lcs.py abcdeffghiklnopqr bdefghijklmnopmnop
lcs     0.49046793299203273

I'll start with _get_lc_substr, since you wrote that code and it'll be easier to work through. The algorithm you use is to go through each pair of consecutive strings in the sorted suffix array, find the common prefix, and save that prefix only if it's longer than any common prefix already found. I can suggest a few improvements:

Iterating over pairs of consecutive elements is a common task that has a fairly standard recipe, pairwise(iterable), given in the documentation for the itertools module. You can use the implementation from the more-itertools package if you want. This also lets you get rid of the try/except block (which probably didn't affect runtime much, but it helps code clarity).
Python has a built-in function, max, to find the maximum value of an iterable. If you use it, then the nuts and bolts of the loop as well as the compare-and-store-if-greater process get handled internally by the interpreter, which should be faster than doing them manually in pure Python.
Repeatedly accessing an attribute of an object, namely the method self._suffix_str_array, is slower than accessing it once and storing it locally as a new variable.

Let's check the effect of these changes on the runtime of the driver program:

lcs     0.49046793299203273
optlcs1 0.4739605739887338
optlcs2 0.4895538759883493
optlcs3 0.4828952929965453

Not much of a change. Actually, using max is slower here, so let's discard that change.

Time now to take a closer look at commonprefix.

If you look at the source code of this function,

def commonprefix(m):
    "Given a list of pathnames, returns the longest common leading component"
    if not m: return ''
    s1 = min(m)
    s2 = max(m)
    for i, c in enumerate(s1):
        if c != s2[i]:
            return s1[:i]
    return s1

you see that it goes through some preliminary steps relating to the fact that it needs to handle a list of potentially several paths. You only ever have two strings to compare, so you can skip that - and in fact, if you look back at the profiling data, you'll see that these calls to max and min do take up a significant amount of time. Therefore, it makes sense to implement your own version of commonprefix without the max and min calls.

This makes a big difference in the runtime, cutting it down by over 30%:

lcs     0.49046793299203273
optlcs3 0.4828952929965453
optlcs4 0.3215277789859101

If you think about it, you don't even really need the common prefix itself, except for the one string you actually return from _get_lcsubstr. You only need its length, so you can decide which substring is the longest. So instead of using a function that finds the full common prefix, just write one that will give you its length. You store the length, along with one of the strings, and at the end of _get_lcsubstr, use the length to trim the stored string.

def _get_lcsubstr(self):
    # initialize
    for s1, s2 in mt.pairwise(suffix_str_array):
        if s1[-1] != s2[-1]:
            substr_len = get_common_prefix_length(s1, s2)
            if substr_len > max_len:
                max_len = substr_len
                max_substr = s1
    return max_substr[:max_len]

This shaves another few percent off the runtime:

lcs     0.49046793299203273
optlcs4 0.3215277789859101
optlcs5 0.30241094100347254

If it's faster to avoid computing a substring in your commonprefix replacement, you might think of doing the same thing when you're finding the suffixes in the first place. In other words, instead of calculating all the suffixes of a string in _get_suffix_str, just make a list of (index, which_string) tuples to represent the suffixes. Then whenever you need to actually compare two suffixes, instead of taking a substring of the original string, you just start comparing characters at the required indices.

There are two problems with this in practice: first, Python isn't well suited to iterating from an arbitrary point in the middle of a string. In a language like C, where strings are character pointers, this would work out quite well, because you can jump into the middle of the string by advancing a pointer. But in Python, iterating from the middle of a string requires you to either start from the beginning and just skip the first several characters, or bypass the whole iteration mechanism and use a for loop with an integer index to access characters inside the string by their indices (which typically involves more Python code that is relatively inefficient). And besides, the other reason is you need to create the substrings anyway to sort them. If you try to do it so that you use the substrings as comparison keys without actually storing them, the program spends a lot of time converting between a substring and its index.

All told, the changes you need to make to the code to use indices everywhere wind up hurting, not helping. Here's the timing result:

lcs     0.49046793299203273
optlcs5 0.30241094100347254
optlcs6 0.3683815289987251

At this point, you can spend a lot of time making little tweaks to try to squeeze some extra performance out of the program, but I don't think there are any major performance gains left. It's already something like 40% faster than the original, which is not bad.

Of course, as dawg wrote in a comment, you can accomplish this task using Python's standard module difflib.

def difflib_longest_match(a, b):
    i, j, k = difflib.SequenceMatcher(a=a, b=b
        ).find_longest_match(0, len(a), 0, len(b))
    return a[i:i+k]

I have no idea what's in the difflib code (well, I could look, but I'll leave that as an exercise), but it's clearly heavily optimized for this kind of task. It's another 25% faster than my best version of your program:

lcs     0.49046793299203273
optlcs5 0.30241094100347254
difflib 0.2154458940058248

So if you really want to do this not for educational purposes, just use difflib.

Here is the content of the test script I used.

import difflib
import itertools as it
import more_itertools as mt
from os.path import commonprefix

class LongestCommonSubstr(object):
    def __init__(self, lstring, rstring):
        self.lstring = lstring+'0'
        self.rstring = rstring+'1'
        self._suffix_str_array = sorted(self._get_suffix_str(self.lstring)
                                    + self._get_suffix_str(self.rstring))
        self.longest_common_substr = self._get_lcsubstr()

    @staticmethod
    def _get_suffix_str(string):
        return [string[i:] for i in range(len(string))]

    def _get_lcsubstr(self):
        try:
            substr_len =0
            max_len = 0
            lcs = None
            for i,n in enumerate(self._suffix_str_array):
                if n[-1] != self._suffix_str_array[i+1][-1]:
                    substr = commonprefix([n,self._suffix_str_array[i+1]])
                    substr_len = len(substr)
                    if substr_len > max_len:
                        max_len = substr_len
                        lcs = substr

        except IndexError:
            pass
        return lcs

class OptimizedLongestCommonSubstr1(object):
    def __init__(self, lstring, rstring):
        self.lstring = lstring+'0'
        self.rstring = rstring+'1'
        self._suffix_str_array = sorted(self._get_suffix_str(self.lstring)
                                    + self._get_suffix_str(self.rstring))
        self.longest_common_substr = self._get_lcsubstr()

    @staticmethod
    def _get_suffix_str(string):
        return [string[i:] for i in range(len(string))]

    def _get_lcsubstr(self):
        substr_len =0
        max_len = 0
        lcs = None
        for s1, s2 in mt.pairwise(self._suffix_str_array):
            if s1[-1] != s2[-1]:
                substr = commonprefix([s1,s2])
                substr_len = len(substr)
                if substr_len > max_len:
                    max_len = substr_len
                    lcs = substr
        return lcs

class OptimizedLongestCommonSubstr2(object):
    def __init__(self, lstring, rstring):
        self.lstring = lstring+'0'
        self.rstring = rstring+'1'
        self._suffix_str_array = sorted(self._get_suffix_str(self.lstring)
                                    + self._get_suffix_str(self.rstring))
        self.longest_common_substr = self._get_lcsubstr()

    @staticmethod
    def _get_suffix_str(string):
        return [string[i:] for i in range(len(string))]

    def _get_lcsubstr(self):
        return max((commonprefix([s1,s2]) for s1, s2 in mt.pairwise(self._suffix_str_array) if s1[-1] != s2[-1]), key=len)

class OptimizedLongestCommonSubstr3(object):
    def __init__(self, lstring, rstring):
        self.lstring = lstring+'0'
        self.rstring = rstring+'1'
        self._suffix_str_array = sorted(self._get_suffix_str(self.lstring)
                                    + self._get_suffix_str(self.rstring))
        self.longest_common_substr = self._get_lcsubstr()

    @staticmethod
    def _get_suffix_str(string):
        return [string[i:] for i in range(len(string))]

    def _get_lcsubstr(self):
        s_array = self._suffix_str_array
        return max((commonprefix([s1,s2]) for s1, s2 in mt.pairwise(s_array) if s1[-1] != s2[-1]), key=len)

class OptimizedLongestCommonSubstr4(object):
    def __init__(self, lstring, rstring):
        self.lstring = lstring+'0'
        self.rstring = rstring+'1'
        self._suffix_str_array = sorted(self._get_suffix_str(self.lstring)
                                    + self._get_suffix_str(self.rstring))
        self.longest_common_substr = self._get_lcsubstr()

    @staticmethod
    def _get_suffix_str(string):
        return [string[i:] for i in range(len(string))]
    @staticmethod
    def _get_common_prefix(s1, s2):
        for i, c in enumerate(s1):
            if c != s2[i]:
                return s1[:i]
        return s1

    def _get_lcsubstr(self):
        s_array = self._suffix_str_array
        gcp = self._get_common_prefix
        substr_len =0
        max_len = 0
        lcs = None
        for s1, s2 in mt.pairwise(s_array):
            if s1[-1] != s2[-1]:
                substr = gcp(s1, s2)
                substr_len = len(substr)
                if substr_len > max_len:
                    max_len = substr_len
                    lcs = substr
        return lcs

class OptimizedLongestCommonSubstr5(object):
    def __init__(self, lstring, rstring):
        self.lstring = lstring+'0'
        self.rstring = rstring+'1'
        self._suffix_str_array = sorted(self._get_suffix_str(self.lstring)
                                    + self._get_suffix_str(self.rstring))
        self.longest_common_substr = self._get_lcsubstr()

    @staticmethod
    def _get_suffix_str(string):
        return [string[i:] for i in range(len(string))]
    @staticmethod
    def _get_common_prefix_length(s1, s2):
        for i, c in enumerate(s1):
            if c != s2[i]:
                return i
        return len(s1)

    def _get_lcsubstr(self):
        s_array = self._suffix_str_array
        gcpl = self._get_common_prefix_length
        max_len = 0
        max_substr = ''
        for s1, s2 in mt.pairwise(s_array):
            if s1[-1] != s2[-1]:
                substr_len = gcpl(s1, s2)
                if substr_len > max_len:
                    max_len = substr_len
                    max_substr = s1
        return max_substr[:max_len]

class OptimizedLongestCommonSubstr6(object):
    def __init__(self, lstring, rstring):
        self.lstring = lstring
        self.rstring = rstring
        self._suffix_str_array = sorted(
            [(i, True) for i, _ in enumerate(lstring)]
            + [(i, False) for i, _ in enumerate(rstring)],
            key=lambda t: (lstring if t[1] else rstring)[t[0]:])
        self.longest_common_substr = self._get_lcsubstr()

    @staticmethod
    def _get_common_prefix_length(s1, start1, s2, start2):
        for i, c in enumerate(it.islice(s1, start1, None)):
            if c != s2[i + start2]:
                return i
        return i

    def _get_lcsubstr(self):
        s_array = self._suffix_str_array
        gcpl = self._get_common_prefix_length
        max_len = 0
        max_start = 0
        max_substr = ''
        for (i1, b1), (i2, b2) in mt.pairwise(s_array):
            if b1 != b2:
                if b1:
                    s1 = self.lstring
                    s2 = self.rstring
                else:
                    s1 = self.rstring
                    s2 = self.lstring
                substr_len = gcpl(s1, i1, s2, i2)
                if substr_len > max_len:
                    max_len = substr_len
                    max_start = i1
                    max_substr = s1
        return max_substr[max_start:max_start+max_len]

def lcs_longest_match(a, b):
    return LongestCommonSubstr(a, b).longest_common_substr

for n in it.count(1):
    if 'OptimizedLongestCommonSubstr{}'.format(n) in globals():
        exec('''def optimized_lcs_longest_match_{}(a, b):
    return OptimizedLongestCommonSubstr{}(a, b).longest_common_substr
'''.format(n, n))
    else:
        break

def optimized_lcs_longest_match(a, b):
    return OptimizedLongestCommonSubstr(a, b).longest_common_substr

def difflib_longest_match(a, b):
    i, j, k = difflib.SequenceMatcher(a=a, b=b).find_longest_match(0, len(a), 0, len(b))
    return a[i:i+k]

if __name__ == '__main__':
    import timeit, sys
    n_runs = 10000
    print('lcs    ', timeit.timeit('lcs_longest_match(sys.argv[1], sys.argv[2])', setup='from __main__ import lcs_longest_match', number=n_runs))
    for k in range(1, n):
        print('optlcs{}'.format(k), timeit.timeit(
            'optimized_lcs_longest_match_{}(sys.argv[1], sys.argv[2])'.format(k),
            setup='from __main__ import optimized_lcs_longest_match_{}'.format(k),
            number=n_runs))
    print('difflib', timeit.timeit('difflib_longest_match(sys.argv[1], sys.argv[2])', setup='from __main__ import difflib_longest_match', number=n_runs))

    print('control results:', difflib_longest_match(sys.argv[1], sys.argv[2]))
    for k in range(1, n):
        print('test results {}: '.format(k), eval('optimized_lcs_longest_match_{}(sys.argv[1], sys.argv[2])'.format(k)))


How to Make Strongly-typed References to Web.Config? Let's say I have a configuration file with a property in it:

<add key="LoadedCode" value="L" />

I know I can reference this using the ConfigurationManager:

System.Configuration.ConfigurationManager.AppSettings["LoadedCode"];

This has some drawbacks.  It can get bulky when it gets used in multiple locations.  I lose support for refactoring and "Find All References" because I'm using an item in the AppSettings collection.  I don't have strong-typing in cases where the config setting isn't a string.  So I wrap it into a static property in a static class:

public static string LoadedCode
{
    get { return System.Configuration.ConfigurationManager.AppSettings["LoadedCode"]; }
}

Great so far.  Now I want to use this on multiple pages and this leads me to my conundrum.  Should I reference this static property everywhere, or should each page have its own local wrapper?

On the one hand, a local wrapper pushes all uses on the page through a single point which can be modified without impacting other pages if needed.  On the other hand, it adds a layer of indirection to the dependency chain and it looks a bit like overkill.

Is there a best practice for this type of behavior, or am I simply over-thinking things?

I don't do much asp.net, but if this StackOverflow answer applies then you could just do this:

using Settings = MyProject.Namespace.Properties.Settings;

var loadedCode = Settings.Default.LoadedCode;

Like this other StackOverflow answer is saying:

  We prefer to use Properties.Settings (aka. settings.settings) because it's strongly typed.

Now I know for a fact that this works with an app.config, but it might not work with a web.config file... but I think it's worth trying.

Alternatively, this Haacked article describes how to use the configuration API to define your own ConfigurationSection classes, which would be the ultimate way to address this issue. Might be overkill if the above works though.


Element-comparison NumPy matrix I need a NumPy matrix with comparisons among all elements in a sequence:

from numpy import matrix
seq = [0, 1, 2, 3]
matrix([[cmp(a, b) for a in seq] for b in seq])

I'd like to know if there is a better way to do this.

Why are you using matrix rather than array? I recommend sticking with array

As for a better way:

seq = numpy.array([0, 1, 2, 3])
numpy.clip(seq[:,None] - seq, -1, 1)

seq[:, None], moves the numbers into the second dimension of the array
seq[:, None] - seq broadcasts the numbers against each other, and subtracts them.
numpy.clip converts lower/higher values into 1 and -1


How to optimize a section in HTML/CSS How to set in HTML/CSS a section just like in this picture https://snag.gy/mStL9H.jpg ?
I have tried something but am not sure if it's right.

#trd-banner{
    background-color:#284c57;
    width:100%;
    height:50%;
    margin:auto;
}
#list{
    position:relative;
 left:110px;
 top:1px;
 width:200px;
 height:150px;
    margin:0px 15px 15px 0px;
}
#list img{
    margin:0px 50px 50px 15px;
}
#prah {
    position:right;
 right:610px;
 top:180px;
 width:240px;
 height:171px;
    margin:0px 0px 15px 15px;
}
#prah img{
    float:right;

}

Unsere Zeolithprodukte sind reine Naturprodukte.

Sie bestehen zu 100% aus Zeolith mit einem hohen

Klinoptilolithanteil.

Das Produkt wurde von uns selbst 2 Jahre lang getestet,

bevor wir auf die Idee kamen, dass wir allen Menschen

diese Freude übermitteln wollen, dieses überwältigende

positive Gefühl von dessen Wirkung.

Da wir viel auf Reisen sind, nehmen wir Zeolith überall mit.

Für uns ist es das Wichtigste, etwas worauf man nicht

vergessen darf.

Deswegen wollen wir, dass jeder der Zeolith probieren will,

sich auch ein Produkt von hochwertiger Qualität leisten kann.

In Liebe und Respekt,

Ihre ZeolithFit Familie

If you write this by hand you should pay attention to consistent
indentation and such as well because it makes things much easier to
read.

The IDs "tekst" and "prah" are a bit opaque, I assume that the first
one should really be "text" instead.

The major complaint I have is the repetition of the style for each and
every line of text.  That should be moved to the surrounding div so
that it applies to all subelements instead.  Also, you should probably
make sure that you supply some fallback fonts in case this one can't be
loaded.

The <br>s at the end of some lines shouldn't be necessary.  If you
want a force empty line maybe move each block of paragraphs into their
own div and control it via that.  The fact that you're manually doing
line breaks is obviously a problem.  It would be much better in terms of
organisation if you just have five or six paragraphs with all the text
and then control text flow by setting the boundaries of the
surrounding box.  You don't have any hyphenation, so that too is no
excuse.  N.b. take a look at how the formatting looks in different
screen sizes as well as on mobile screens (i.e. using the inspector
and emulating a smaller screen).

Having additional divs for the images is maybe not needed?  But then
again, it's really up to the CSS how that works out in the end.


Filter email with JavaMail I'm trying to develop code to filter my e-mail with JavaMail, but I doubt it's the best way of doing this. And I want to collect all messages without attachment.

package service.forkinjoin;

import java.io.IOException;
import java.util.ArrayList;
import java.util.List;
import java.util.concurrent.RecursiveAction;

import javax.mail.Message;
import javax.mail.MessagingException;
import javax.mail.Multipart;
import javax.mail.Part;
import javax.mail.internet.MimeBodyPart;

import service.FileUtil;

public class ForkSortMessages extends RecursiveAction {

    private static final long serialVersionUID = -1092415796824205832L;
    private List<Message> listMessages;
    private List<Message> listMessagesToDelete;

    public ForkSortMessages(List<Message> listMessages, List<Message> listMessagesToDelete) {
        this.listMessages = listMessages;
        this.listMessagesToDelete = listMessagesToDelete;
    }

    @Override
    protected void compute() {

        List<RecursiveAction> actions = new ArrayList<>();

        if (this.listMessages.size() <= Runtime.getRuntime().availableProcessors()) {
            try {
                this.separateMessages();
            } catch (MessagingException | IOException e) {
                e.printStackTrace();
            }
        } else {

            int end = this.listMessages.size() / 2;
            actions.add(new ForkSortMessages(this.listMessages.subList(0, end), this.listMessagesToDelete));
            end += this.listMessages.size() % 2 == 0 ? 0 : 1;
            actions.add(new ForkSortMessages(this.listMessages.subList(end, this.listMessages.size()), this.listMessagesToDelete));
            invokeAll(actions);
        }
    }

    private void separateMessages() throws MessagingException, IOException {

        for (Message message : this.listMessages) {

            if ((this.hasNoAttachment(message.getContentType()) || (this.hasNoXmlAttachment(message)))) {
                listMessagesToDelete.add(message);
            }
        }
    }

    private boolean hasNoAttachment(String content) {
        return !content.contains("multipart/MIXED");
    }

    private boolean hasNoXmlAttachment(Message message) throws IOException, MessagingException {

        Multipart multipart = (Multipart) message.getContent();

        for (int i = 0; i < multipart.getCount(); i++) {

            MimeBodyPart mimeBodyPart = (MimeBodyPart) multipart.getBodyPart(i);

            if (Part.ATTACHMENT.equalsIgnoreCase(mimeBodyPart.getDisposition())) {

                if (FileUtil.isXmlFile(mimeBodyPart.getFileName())) {
                    return false;
                }
            }
        }

        return true;
    }
}

So what can I do to improve my code? I've tried to do like Oracle tutorial but I can't.

In order to correctly work in parallel, you must follow the Actor Model, which basically says that each iteration should not share memory when doing their work. In your code, you pass a List<Message> to the child processes, which they are expected to fill. Each child process changes his own copy of the list, which makes the final result unexpected.

To build your actors correctly, each should create its own solution vector, and later its parent process should merge the result of the children, and pass them on (you can see samples here).

If the order of the messages is important to you, even this is not enough, since you don't know which child process will finish first, so the messages in the resulting list will not be in the same order as in the input list.

After all being said and done - you might find out that you do not get better performance, and sometimes even worse performance.

My suggestion is to keep it simple - the actual work you are doing on the array is simple enough - do a straight forward iteration. By they way you describe your use-case - at most you will have to wait a couple of minutes (tens of minutes?) for the initial bulk to be computed, but at least you'll be confident that you have the correct result. I assure you it would take less time to run it synchronously than to develop and debug a fork/join solution...


Convertors converting unconverted objects In my app, I have several convertors, like these two:

public class ThemeToTableStripeConverter : IValueConverter
{
    public object Convert(object value, Type targetType, object parameter, string language)
    {
        if ((int)value == 1) { return new SolidColorBrush(Color.FromArgb(255, 34, 34, 34)); }
        return new SolidColorBrush(Color.FromArgb(255, 238, 238, 238));
    }

    public object ConvertBack(object value, Type targetType, object parameter, string language)
    {
        return null;
    }
}

public class ThemeToLBStyleConverter : IValueConverter
{
    public object Convert(object value, Type targetType, object parameter, string language)
    {
        if ((int)value == 1) { return Application.Current.Resources["LBDark"]; }
        else if ((int)value == 2) { return Application.Current.Resources["LBGold"]; }
        else { return Application.Current.Resources["LBLight"]; }
    }

    public object ConvertBack(object value, Type targetType, object parameter, string language)
    {
        return null;
    }
}

In the first, I have an int representing a theme style, with 0, 1, and 2 being the three themes I support.  Themes 0 and 2 are identical in the part affected by this (white background), while 1 has a black background.  In the second, I want theme 0 to always be the default, no matter the value, and themes 1 and 2 to return different items.

Beyond the general comments, there are a couple things I would like specific feedback on.  Both of these convertors are one-way convertors, and always will be; however, that return null; looks a little off - should I replace that with throw new NotImplementedException();?  Instead of formatting my code like this:

if (condition) { return val1; }
return val2;

Should I write that as:

if (condition) { return val1; }
else { return val2; }

Avoid long one-liners:

This:

if ((int)value == 1) { return new SolidColorBrush(Color.FromArgb(255, 34, 34, 34)); }

Should be:

if ((int)value == 1)
{
    return new SolidColorBrush(Color.FromArgb(255, 34, 34, 34));
}

Typically one-liner if-statements are-so because they're short and can't be justified to take up 3 lines for something so small.

Avoid 'else' one-liners:

This is a clutter:

if (condition) { return val1; }
else { return val2; }

This isn't as big of a clutter, just space-consuming:

if (condition)
{
    return val1;
}
else
{
    return val2;
}

Acceptable alternatives:

I would do the following for maximum readability:

return condition ? val1 : val2;

Making use of the ternary operator and being the most maintainable/readable method.

In cases where you're not able to keep it one line, something like you had above is perfectly acceptable:

if (condition) { return val1; }
// Do something...?
return val2;

One-way converters:

I would consider the following situation before throwing an NotImplementedException().

public interface IOneWayValueConverter
{
    object Convert(object value, Type targetType, object parameter, string language);
}

// Optionally Adding "TwoWay" to the name (or some other similar naming method).
public interface IValueConverter : IOneWayValueConverter
{
    object ConvertBack(object value, Type targetType, object parameter, string language);
}

If this isn't an option (for any number of reasons) then it would seem the only acceptable answer to this situation would be throwing a NotImplementedException() as you stated.


Managing "Jobs" data in a database I've been working in a small page with some easy CRUD. The data I get from the database is getting shown on the page. But the way how I show the data feels a bit off. I guess it can be done in a much cleaner way, and really want to know how.

I do it as follows:

First: I require the necessary class, and call the function the the HTML file:

<?php
require 'classes/vacatureData.php';

$db = new database();
$db->listAllVacatures();

require 'templates/head.php';
require 'templates/vacatureOverzicht.php';
require 'templates/footer.php';

In my class I got a public variable public $listAllVacatures;, which I use to store all the values which I retrieve from the database.

The method to retrieve the data from the database looks as following:

public function listAllVacatures() {
    $sql = 'SELECT * FROM vacatures';
    $stmt = $this->db->prepare($sql);

    if($stmt->execute()) {
        $vacatures = $stmt->fetchAll();

        foreach($vacatures as $vacature){
            $this->listAllVacatures .= "
";
            $this->listAllVacatures .= "$vacature[1]";
            $this->listAllVacatures .= " - delete";
            $this->listAllVacatures .= " - Edit";
            $this->listAllVacatures .= "
";
        }
    } else {
        $e = $stmt->errorInfo();
        echo $e[2];
    }

    $this->db = null;
}

As you can see I use a foreach to fill $listAllVacatures with data, but also some HTML (Which i feel shouldn't be there, but right now I'm not quite sure how to organise this properly).

Then I got the template file which uses $listAllVacatures:

Vacatures:

    <?php echo $db->listAllVacatures ?>

This all works fine, but like I said, it doesn't feel right to do it this way. But for now it's the best way I know.

I see you want to apply Separation of Concerns by separating business logic from the View's.
The database::listAllVacatures() method should return the $vacatures array. This way you can move the foreach() (which prepares the HTML output) from inside the method into the template like so:

Vacatures:

<?php foreach ($db->listAllVacatures() as $vacature): ?>

        <a href="vacature.php?id=<?= $vacature[0] ?>"><?= $vacature[1] ?>
        - <a href="delete.php?id<?= $vacature[0] ?>">delete
        - Edit

<?php endforeach; ?>

Additional Information

It would be best to follow a naming convention that uses one language. It only makes everything harder to understand when you have to read code that is written in multiple languages (like English and Dutch). I'd say stick to the same language of the programming language you're using. So listAllVacatures() method would now actually be renamed something like findAllJobs().

Your database class isn't actually what the name implies it to be. It's actually a class which responsibility is (or should be) to retrieve all job data from the database. You might wanna call it JobFetcher or something like that (note the CamelCase naming convention for classes in PHP).

I see that you're instantiating your database class like $db = new database();. No dependencies are injected. In the database::listAllVacatures() you use $this->db to communicate with the database. Which means you have your database class tightly coupled with PDO.
What if you need to update the credentials to connect to the database? You will have to change your class, which is a bad thing. I recommend you to utilize Dependency Injection to get rid of this problem, so you can inject the PDO instance the class needs to operate. It introduces many benefits.

Since you showed interest in wanting to separate your concerns I would advise you to take a look at the MVC pattern that does just what you want: It separates business logic from the View's.
Also try to adhere to the widely known coding standards defined by the PHP Framework Interop Group. It improves the code's readability, especially to other programmers who follow these standards as well. Of course, you do not have to be perfect with it.


Counting Bloom Filter I'm working on writing a Java class library which includes the following implementation of a counting bloom filter:

package sj224.lib.util;

import java.util.function.Predicate;
import java.util.Random;

public class BloomFilter<V> implements Predicate<V>{
    private final int size;
    private int count;
    private final int bpe;
    private final int[] data;
    public int hashCode(){
        long t=count;
        for(int i=0;i<size;i++){
            t+=(long)((data[i]%2)*Math.pow(2, i%64));
        }
        return new Random(t).nextInt();
    }
    public String toString(){
        return size+"-bit Bloom Filter containing "+count+" elements (hash code "+hashCode()+")";
    }
    public BloomFilter(int space){
        data=new int[space];
        size=space;
        bpe=Math.max(3, Math.min(size/8,(int)Math.sqrt(size)));
        count=0;
    }
    public int size(){
        return count;
    }
    private int[] hash(V v){
        Random r=new Random(v.hashCode());
        int[] keys=new int[size];
        for(int i=0;i<bpe;i++){
            keys[r.nextInt(size)]=1;
        }
        return keys;
    }
    public BloomFilter(){
        this(64);
    }
    public BloomFilter(Iterable<V> source){
        this(64);
        for(V i:source)insert(i);
    }
    public boolean remove(V v){
        int[] a=hash(v);
        for(int i=0;i<size;i++){
            if(data[i]<a[i])return false;
        }
        for(int i=0;i<size;i++){
            data[i]-=a[i];
        }
        count--;
        return true;
    }
    public void clear(){
        for(int i=0;i<size;i++)data[i]=0;
    }
    public void insert(V v){
        int[] a=hash(v);
        for(int i=0;i<size;i++){
            data[i]+=a[i];
        }
        count++;
    }
    public boolean contains(V v){
        int[] a=hash(v);
        for(int i=0;i<size;i++){
            if(data[i]<a[i])return false;
        }
        return true;
    }

    public boolean test(V v){
        return contains(v);
    }
}

Is there anything here that can reasonably be improved?

Typo

This probably happened when you cut and pasted your code, but your contains function had a few lines switched in their ordering.

Operations taking O(size) time, could be faster

Your add, remove, and contains operations all do approximately the same thing.  They create a hash where the hash contains up to bpe bits set in an array of size bits.  Then they iterate over the size bits.

The important thing here is that bpe is much smaller than size.  For any size larger than 64, bpe will be sqrt(size).  Since only bpe bits are set, you only need to take O(bpe) time instead of O(size) time.

Operations using 32x the required space

Also, when you create your array of bits, you use one int per bit even though you only ever set the int to 1 or 0.  If size is very large, such as 100 million, you could be using 400 MB when you only need to be using 12.5 MB.

Putting it all together

I rewrote your functions, and also combined all of their functionality into the hashing function since all 3 functions were so similar.  In the new code, the loops only run through bpe iterations instead of size iterations.  I also packed the bits to minimize the space used.

/**
 * Performs one of three operations: add, remove, or contains.
 *
 * @param        v                The element to add/remove/test.
 * @param        operation        1 = add, -1 = remove, 0 = contains
 * @return                        If operation is "contains", returns true
 *                                if v is contained in the bloom filter, and
 *                                false if it is not.  If operation is
 *                                anything else, returns true.
 */
private boolean hashOperation(V v, int operation)
{
    Random r         = new Random(v.hashCode());
    int[]  bitsFound = new int[(size+31)/32];

    for(int i=0;i<bpe;i++){
        int bitNum   = r.nextInt(size);
        int bitIndex = bitNum >> 5;
        int bit      = (1 << (bitNum & 31));
        if ((bitsFound[bitIndex] & bit) == 0) {
            // New bit.  Perform operation.
            bitsFound[bitIndex] |= bit;
            if (operation == 0) {
                if(data[bitNum] == 0)
                    return false;
            } else {
                data[bitNum] += operation;
            }
        }
    }
    return true;
}

public boolean remove(V v)
{
    if (contains(v)) {
        hashOperation(v, -1);
        count--;
        return true;
    }
    return false;
}

public void insert(V v)
{
    hashOperation(v, 1);
    count++;
}

public boolean contains(V v)
{
    return hashOperation(v, 0);
}

Addendum: Testing the fastest way to hash using maaartinus's method

Maaartinus showed a fast way to hash using just 2 hash functions.  I decided to test out 4 variants:

Using a power of 2 size (should be fastest)
Using modulus operator every loop (should be slowest)
Using a compare + subtract
Doing the subtract without using any compare

Note, the following is all done in C:

Variant 1

All variants use the following setup.  The thing that varies will be in the loop:

uint32_t hash, hash1, hash2, size;

size  = rand();
hash1 = rand() % size;
hash2 = rand() % size;

hash = hash1;
uint32_t sizeMask = 0x3fffffff;  // Variant 1 only
for (i=0;i<iterations;i++) {
    hash = (hash + hash2) & (sizeMask);
}

Variant 2

for (i=0;i<iterations;i++) {
    hash = (hash + hash2) % size;
}

Variant 3

// If size is < 0x80000000
for (i=0;i<iterations;i++) {
    hash += hash2;
    if (hash > size)
        hash -= size;
}

// This works for all cases (and has the same runtime as the one above)
for (i=0;i<iterations;i++) {
    if (size - hash > hash2)
        hash += hash2;
    else
        hash -= (size - hash2);
}

Variant 4

// In C, >> is not guaranteed to do an arithmetic shift, so
// we use a logical shift instead.  The compiler optimizes this code
// to do an arithmetic shift anyways.  In java, change the ">>" here
// to a ">>>".
for (i=0;i<iterations;i++) {
    hash += hash2;
    hash -= (-((size - hash) >> 31)) & size;
}

// In java, where >> is arithmetic shift, you could write it like this.
for (i=0;i<iterations;i++) {
    hash += hash2;
    hash -= ((size - hash) >> 31) & size;
}

Timing results

Running each variant for 1 billion iterations (results in seconds):

Variant 1 (& power of 2): 0.58
Variant 2 (% size)      : 6.56
Variant 3 (if, subtract): 1.25
Variant 4 (no if)       : 1.44

However, this is a bit misleading because there was nothing in the loop except for the hash calculation.  If I add in the part where we use the hash on each loop:

    bits[hash>>5] |= (1 << (hash & 31));

Then the timings all become much closer:

Size = 1 billion
Variant 1 (& power of 2): 15.92
Variant 2 (% size)      : 18.92
Variant 3 (if, subtract): 17.48
Variant 4 (no if)       : 16.76

Size = 4 million
Variant 1 (& power of 2): 2.48
Variant 2 (% size)      : 7.11
Variant 3 (if, subtract): 2.58
Variant 4 (no if)       : 2.52

Size = 256K
Variant 1 (& power of 2): 1.24
Variant 2 (% size)      : 6.64
Variant 3 (if, subtract): 1.79
Variant 4 (no if)       : 1.95

So it appears that at large sizes, the memory manipulation of the bit array will be the limiting factor and not the hash calculation.  So you can choose from the above 4 variants based on:

Whether you can force the size to be a power of 2 or not.
How much you need that extra bit of speed versus how clear/simple you want your code to be.


Small animation with Raphael I have a small animation with Raphael using a large amount of code.  Is there any way to reduce it?

Demo

window.onload = function () {
    var elements = document.querySelectorAll('.colbr');
    for (i=0; i<elements.length; i++) {
        var loadSpinner = spinner(elements[i], 7, 12, 10, 3, "#000");
    }
};

function spinner(holderid, R1, R2, count, stroke_width, colour) {
    var sectorsCount = count || 12,
        // color = colour || "#000",
        color = "#000",
        width = stroke_width || 15,
        r1 = Math.min(R1, R2) || 35,
        r2 = Math.max(R1, R2) || 60,
        cx = r2 + width,
        cy = r2 + width,
        r = Raphael(holderid, r2 * 2 + width * 2, r2 * 2 + width * 2),

        sectors = [],
        opacity = [],
        beta = 2 * Math.PI / sectorsCount,

        pathParams = {stroke: color, "stroke-width": width, "stroke-linecap": "round"};
        Raphael.getColor.reset();
    for (var i = 0; i < sectorsCount; i++) {
        var alpha = beta * i - Math.PI / 2,
            cos = Math.cos(alpha),
            sin = Math.sin(alpha);
        opacity[i] = 1 / sectorsCount * i;
        sectors[i] = r.path([["M", cx + r1 * cos, cy + r1 * sin], ["L", cx + r2 * cos, cy + r2 * sin]]).attr(pathParams);
        if (color == "rainbow") {
            sectors[i].attr("stroke", Raphael.getColor());
        }
    }
    var tick;
    (function ticker() {
        opacity.unshift(opacity.pop());
        for (var i = 0; i < sectorsCount; i++) {
            sectors[i].attr("opacity", opacity[i]);
        }
        r.safari();
        tick = setTimeout(ticker, 1000 / sectorsCount);
    })();
    return function () {
        clearTimeout(tick);
        r.remove();
    };
}

I really like what you wrote there, though I am afraid at this point you ought to expand the code, not reduce it.

r, r1, r2, cx, cy, beta are all variables that need better ( more meaningful ) names
You need to indent your code, just hit TidyUp in your fiddle and you will see
This is not part of your core code, but you should not simply assign something to window.onload, you should use addEventListener
Your code had support for using custom colors, and then you took it out, if you really want to take that out, then you should also take out the parameter, the commented out code and the  if (color == "rainbow"){ statement
in your var statement, you could simply do cy = cx
be consistent in your name, and use lowerCamelCase: stroke_width -> strokeWidth
You did not declare i with var
While straightforward, your code has no comments at all..


Find the number surrounded by square braces using regex I need to remove all the numbers appearing inside square braces such as [9] , [2] , ....

This is my regex:

"(?:\[\d*\])"

Which is working fine.
But I am totally new to regex so let me know if is there a better regex that can be used.

Try Online

You don't need the grouping here, and also you'd want to match for 1 or more digits: "\[\d+\]"