Proper way to insert a record and assign the id to the Entity object Scenario: I need to insert a record in a database and use the id resulting from that operation for every other operation needed.

So I've created a class that contains all details of the object that needs to be persisted in the database and added an Id field to store the record id returned by the database operation.

public class MetadataObject {
    protected long Id { get; private set; }
    protected long RegistrationDate { get; private set; }
    protected long FileName { get; private set; }

    public MetadataObject(string fileName)
    {
        FileName = fileName;
        RegistrationDate = DateTime.Now;
    }
}

1) Once I create a MetadataObject, the Id property is 0 (not initialized). Then I make the call to the database.

metadata.Id = RegisterMetadataAndReturnId(metadata);

2) Another thing that I could do is to assign a value to the Id field immediately after I get it back from the database. The method that handles this would look like this

public void RegisterMetadata(MetadataObject metadata)
{
    long recordId;
    var metadataInsert = CreateStoredProcedure("RegisterMetadata");
    metadataInsert.Execute();

    [assign recordId with value received from stored procedure]

    metadata.Id = recordId;
}

Second version looks better in my opinion, the only doubt I have is if the method calling the stored procedure must make sure the metadata object Id property is set before exiting. Or is it better if I just return the Id value and let the calling procedure take care of that.

Is there a better/cleaner way of getting this done?

I would use a slightly modified form of your option #2, which, instead of returning a modified version of the object, returns a new instance of that same entity.

There are two main reasons for this. The first is that you can't be entirely sure if the data stored in your DB is exactly that of your original item. The auto-incremented ID field is new, of course, but there could be triggers, constraints or login in stored procedures that might make the data subtly different. It's best to return the newly-stored entity from the DB call and have it fully synced with the DB.
The second reason is immutability. If your logic relies on updating a field in your entity after it's created, it means the ID field is mutable and can be changed elsewhere in your code. Having the class be immutable - unchangeable after instantiation - can make a lot of your logic simpler. You can have two threads operate on the same instance without worrying about race conditions between them, and other benefits beside.

Reason #2 is enough, in my opinion, to always clone your object, whether it's cloning-through-database-query or just cloning the instance before returning it.


How to improve readability of a big lisp function My main method (remove-random-edge) looks quite difficult to read. I'm new to list, so would appreciate any advice on how to improve the code.

(defun find-node (node graph)
  (find-if #'(lambda (i) (eql node (first i))) graph))

;; Input, graph, is in form ((from-1 to-1 to-2 to-3 ...)
;;                           (from-2 to-4 to-5 to-6 ...) ...)
;; where from-n and to-n are integers.
(defun remove-random-edge (graph)
  "Remove one random edge from a graph given as adjacency list."
  (let* ((node-list-1 (elt graph (random (length graph))))
         (node-1 (first node-list-1))
         (destinations (remove-duplicates (rest node-list-1)))
         (node-2 (elt destinations (random (length destinations))))
         (node-list-2 (find-node node-2 graph)))
    (flet ((replace-tail-for-head (node) (if (eql node node-2) node-1 node))
           (is-head-p (node) (eql node-1 node))
           (is-tail-p (node) (eql node-2 node))
           (starts-with-tail-p (nodes) (eql node-2 (first nodes))))
      (setf (rest node-list-1) (concatenate 'list
                                 (rest node-list-1)
                                 (remove-if #'is-head-p (rest node-list-2))))
      (loop for node in (remove-duplicates (rest node-list-2))
            with match
            with repcd
            do (setf match (find-node node graph))
            do (setf repcd (if (eql node node-1)
                             (remove-if #'is-tail-p (rest match))
                             (map 'list #'replace-tail-for-head (rest match))))
            do (setf (rest match) (sort repcd #'<)))
      (remove-if #'starts-with-tail-p graph))))

UPD: With review comments applied:

(defun remove-random-edge (graph)
  "Remove one random edge from a graph given as adjacency list."
  (let* ((head-list (elt graph (random (length graph))))
         (head (first head-list))
         (destinations (remove-duplicates (rest head-list)))
         (tail (elt destinations (random (length destinations))))
         (tail-list (assoc tail graph)))
    (flet ((replace-tail-for-head (node) (if (eql node tail) head node)))
      (setf (rest head-list) (concatenate 'list
                               (rest head-list)
                               (remove head (rest tail-list))))
      (loop for node in (remove-duplicates (rest tail-list))
            for match = (assoc node graph)
            do (setf (rest match) (if (eql node head)
                                    (remove tail (rest match))
                                    (mapcar #'replace-tail-for-head (rest match)))))
      (remove tail graph :key #'first))))

Before:

15.109 seconds of real time
50,245,719,578 processor cycles
767,039,256 bytes consed

After:

2.312 seconds of real time
7,665,669,728 processor cycles
778,172,816 bytes consed

Superficial

Your find-node is actually (almost) assoc or, if you prefer, (find node graph :key #'first).

Use mapcar instead of map 'list because it makes the intent clearer (the difference is that mapcar takes only lists and map any sequence).

You need just one do in loop; you can even fold all your setfs into one (setf a b c d e f).

In loop, with should (stylistically) come before for.

Deep

Your code looks too complicated for what it does.

None of your flet local functions is really necessary.
E.g., (remove-if #'is-head-p ...) should be (remove node-1 ...),
and (remove-if #'starts-with-tail-p ...) should be (remove node-2 ... :key #'first);
this would make the code both faster and cleaner.

Your loop should be

(loop for node in (remove-duplicates (rest node-list-2))
      for match = (find-node node graph)
      for repcd = (if (eql node node-1)
                      (remove-if #'is-tail-p (rest match))
                      (map 'list #'replace-tail-for-head (rest match))))
      do (setf (rest match) (sort repcd #'<)))

General

Usually it is better to use the most specific function you need. E.g., use setq on simple variables (instead of setf which also supports general references). Use mapcar instead of map 'list when working with lists (as opposed to vectors).


Distance checking of normalized vectors? I've been learning C#/XNA in an attempt to make a game for the past few weeks so maybe this is an obvious newbie thing, but here goes:

I've been making a fairly simple 2D game, so I have a list of ~1000 objects (NPC's), and I'm iterating through them extremely frequently to do distance checking on each one. The problem is when I increase the distances, everything slows down considerably to the point where the game is unplayable, and I don't really understand why.

It works fine when the distances are short (if all objects are within one 800x480 world size), but when they are increased (say, an 8000x4800 world size), my performance tanks.

Here is some relevant code (not all of it, obviously, since that would be too much stuff, but this is the gist of what's happening):

List<Human> humans;
List<Enemy> enemies;

public void Update(City city, float gameTime)
{
    humans = city.GetHumans();
    enemies = city.GetEnemies();

    for (int h = 0; h < humans.Count; h++)
    {
        Vector2 human_position = humans[h].position;

        Vector2 nearestEnemyPosition = Vector2.Zero;

        for (int e = 0; e < enemies.Count; e++)
            {
                Vector2 enemy_position = enemies[e].position;
                float distanceToEnemy = Vector2.DistanceSquared(human_position, enemy_position);

                if (distanceToEnemy < distanceToNearestEnemy)
                {
                    distanceToNearestEnemy = distanceToEnemy;
                    nearestEnemyPosition = enemy_position;
                }

                if (distanceToNearestEnemy < 2500f)
                {
                     logic code here (very little performance impact)
                }
            }

        for (int hh = 0; hh < humans.Count; hh++)
            {
                if (h == hh)
                {
                    continue;
                }

                if (humanMoved == true)
                {
                    continue;
                }

                Vector2 other_human_position = humans[hh].position;

                if (Vector2.DistanceSquared(human_position, other_human_position) < 100f)
                {
                   do more stuff here (movement code, no performance impact)
                }

I also have almost identical loops for the enemy list.

The code to make them is here:

foreach (Human human in this.humans)
        {
            Vector2 position = new Vector2(random.Next(800), random.Next(480));
            human.Spawn(position);
        }

This works fine. I get 60FPS and it's totally smooth & everything runs perfectly. However, if I do this:

foreach (Human human in this.humans)
        {
            Vector2 position = new Vector2(random.Next(8000), random.Next(4800));
            human.Spawn(position);
        }

Everything tanks and I get 1FPS. I tried normalizing the vectors, which solved the performance issue, but whenever I checked to find the closest object it gave me inaccurate results.

Is there any way to accurately check for the distance of two objects that have been normalized with Vector2.Normalize()? Am I even asking the right question?

Let me start by agreeing with the comments that have been posted. It is really strange that you're getting this behavior. In fact, it seems like it should be the opposite.

I Could Not Reproduce the Performance Delay

I started off by trying to recreate the problem you're describing. I first attempted by starting from the code you've given. I could measure no performance difference (using the Stopwatch class) when selecting between the small 800x480 size and the larger 8000x4800 size.

The next thing I tried was to strip the example down to just simply Vector2 objects, leaving out all of your Human, Enemy, and City types. I used the following code:

private static Random random = new Random();

protected override void Update(GameTime gameTime)
{
    // Generate initial list of vectors.
    List<Vector2> vectors = new List<Vector2>();
    for (int index = 0; index < 1000; index++)
    {
        vectors.Add(new Vector2(random.Next(800), random.Next(480)));
    }

    double sumOfSquares = 0;

    // Start timing.
    Stopwatch stopwatch = new Stopwatch();
    stopwatch.Start();

    // Add up the
    foreach(Vector2 a in vectors)
    {
        foreach(Vector2 b in vectors)
        {
            sumOfSquares += Vector2.DistanceSquared(a, b);
        }
    }

    // End timing and print results.
    stopwatch.Stop();
    Console.WriteLine(stopwatch.ElapsedMilliseconds);
}

In both cases, there were no measurable differences. The size of the vectors does not have an impact on the performance.

I think this jives with what people would usually expect, hence the comments indicating it was a surprise. Regardless of the size of the values contained in the variables, the code should produce the same IL code, and ultimately the same machine instructions. It should be running the exact same code.

Looking for Other Problems

I think at this point, you should turn your attention elsewhere in your code. (You probably already did. This question is six months old now.)

One thing that strikes me as a possibility is if all you do is essentially change the size of your world (800x480 to 8000x4800), you potentially need to change a number of other variables as well, if you're making the change to create a higher resolution world, instead of just simply a larger world. In particular, the number you're using as the minimum distance between a human and an enemy, and a human and another human. Without changing these appropriately, you'll get different percentages of breakdowns in how often it gets into each of these if-statements.

What's still odd to me is that it seems backwards. If you increase the size of the world without increasing the number of objects in it, they'll be less dense. You should be hitting the inside of those if-statements less frequently. In theory, it should increase the performance to use a bigger world.

So in the end, you'll probably want to reconsider what the code inside of those if-statements actually do. There's a much better chance that that is causing the lag, not the actual size of the world. If you've got break or continue, you'll want to take an especially close look at how these behave, because these could be short circuiting your loop.

The Difference in Performance is Not Caused by Not Having Space Partitioning

One thing I don't agree with, as far as the comments go, is that this case warrants looking into space partitioning. It's possible that you'll want it in the end, but your underlying performance problem isn't necessarily tied to the spatial arrangement of your objects. There is something else strange going on here, and you'll want to figure that out first.

If both of your cases had the same performance, you felt like it was too slow, and the performance delay was clearly and measurably caused by the distance comparisons (i.e., profiling) that's when you'd look into some form of partitioning.

Vector Normalization

You mentioned that you normalized the vectors, and that solved your issue. Vector normalization takes a vector and turns it into one that points in the same direction, but has a length of 1. If a Vector2 object represents a velocity, an acceleration, or a vector from one point to another, vector normalization can be used to tell you the direction of that vector.

But if your vector is being used to represent a point in 2D space (or 3D space) normalizing it will just tell you the direction that the point lies from the origin of your world. It loses its meaning, as far as determining how far away two points are from each other. So that's why you're no longer getting correct results when you do this.


A Decorator that turns Function Annotations into Type Checks Answering a question on S/O about type annotations led to me writing a little decorator that adds type checking to functions with annotated arguments. It supports the meta types Any, Optional[], and Union[], as well as combinations such as Optional[Union[...]]. It takes no arguments itself, for ease of use.

I'm fairly new to non-SQL programming in general so I'd love feedback. There's a short README in its Github repo.

from typing import Callable, Union, Optional
from functools import wraps
from itertools import zip_longest
import re

def enforce_types(func: Callable) -> Callable:
    """Adds run-time type checking to a fully-annotated function"""

    @wraps(func)
    def wrapper(*args, **kwargs):

        for argument, annotation, value in zip_longest(
            *zip(*func.__annotations__.items()), args,
            fillvalue=""
        ):
            # If we have Optional args, we could have more annotations than supplied arguments
            if value == "":
                break
            # Type Any can take... any type, so continue
            elif annotation == "Any":
                continue

            value_type = type(value).__name__

            # Check for meta types
            is_optional: bool = "Optional" in str(annotation)
            is_union: bool = "Union" in str(annotation)

            # If Optional[], get the optional type
            if is_optional:
                annotation = re.search("Optional\[(.*)\]", str(annotation)).group(1)

            # If Union[], get the list of types
            if is_union:
                annotation = re.search("Union\[(.*)\]", str(annotation)).group(1)

            # If there's just one type allowed, check against it
            if not is_union and value_type != annotation.__name__:
                raise TypeError(
                    f"Argument {argument} supplied wrong type: expected {annotation.__name__}, got {value_type}."
                )
            # If it's not a Union and it passed the last check,
            # or if it's a Union and a valid type was passed in, continue
            elif (not is_union) or (is_union and value_type in annotation):
                continue
            # If there's a type Union, check against the possible types
            elif is_union and value_type in annotation:
                continue
            # If we get here, the argument isn't any of the appropriate values
            else:
                raise TypeError(
                    f"Argument {argument} supplied wrong type: expected one of [{annotation}], got {value_type}"
                )

        return func(*args, **kwargs)

    return wrapper

Example of usage:

>>> from type_enforcer import enforce_types
>>> from typing import Union, Optional

>>> @enforce_types
... def foo(x: Union[int, float], y: Optional[int] = 1) -> int:
...     return int(x * y)

>>> foo(5, 2)
10

>>> foo(5)
5

>>> foo(5.)
5

>>> foo(5., 2)
10

>>> foo('5')
Traceback (most recent call last):
  ...
TypeError: Argument x supplied wrong type: expected one of [int, float], got str

>>> foo(5, 2.)
Traceback (most recent call last):
  ...
TypeError: Argument y supplied wrong type: expected one of [int, NoneType], got float

This is a very very poor implementation.

Firstly the code has little care for nested datatypes, and makes them a pain to add in a future version.

To add support for this all that needs to be done is change the, per type, control flow to use recursion. The option to use standard loops rather than recursion is available too, however that will likely make the code harder to read.
The code just looks very error prone. Whilst RegEx has it's place in some limited scenarios, this is not then. The following snippet is almost never a good idea.

is_optional: bool = "Optional" in str(annotation)
if is_optional:
    annotation = re.search("Optional\[(.*)\]", str(annotation)).group(1)

This is the kind of nonsense I would expect of a junior that just learnt what RegEx is.
Whilst 'proper' low level typing inspection isn't that readable, mangling it with strings makes the code harder to read for people used to interacting with typing.

Also, when performing low level changes on core Python libraries read the PEPs that accompany them. They're their for a reason. For instance, PEP 484 states how to remove the need for using strings.

  The string literal should contain a valid Python expression (i.e., compile(lit, '', 'eval') should be a valid code object) and it should evaluate without errors once the module has been fully loaded. The local and global namespace in which it is evaluated should be the same namespaces in which default arguments to the same function would be evaluated.

Furthermore, reading the PEP is not required, as the typing documentation shows a function that does this out of the box.

  typing.get_type_hints(obj[, globals[, locals]])
  Return a dictionary containing type hints for a function, method, module or class object.

Whilst I can understand that hacking a solution may be the most fun way to implement something. It's how I learnt the typing library. You should at least read the documentation of the module. Python's documentation is very high-quality, and ignoring it leads to poor code.

Here's a naive implementation:

import typing
import inspect

def is_type(instance, type_info):
    if type_info == typing.Any:
        return True
    if hasattr(type_info, '__origin__'):
        if type_info.__origin__ in {typing.Union, typing.Optional}:
            return any(is_type(instance, arg) for arg in type_info.__args__)
    return isinstance(instance, type_info)

def enforce_types(func):
    types_info = typing.get_type_hints(func)
    signature = inspect.signature(func)

    def inner(*args, **kwargs):
        sig = signature.bind(*args, **kwargs)
        sig.apply_defaults()
        for name, value in sig.arguments.items():
            if not is_type(value, types_info.get(name, typing.Any)):
                raise TypeError(
                    '{name} is not of the correct type'
                    .format(name=name)
                )
        return func(*args, **kwargs)

    return inner


Python rock paper scissors My code implementing rock, paper and scissors in Python, using dicts to hold choices.

import random

choices = ['rock', 'paper', 'scissors']

def choice():

    player_choice = raw_input('Choose rock, paper or scissors: ')
    if player_choice.lower() in choices:
        computer_choice = random.choice(choices)
        play(player_choice, computer_choice)
    else:
        choice()

def play(p_choice, c_choice):

    win_table = {'rock' : {'rock':'It was a Tie', 'paper':'You Lose', 'scissors':'You Win'},
                 'paper' : {'rock':'You Win', 'paper':'It was a Tie', 'scissors':'You Lose'},
                 'scissors' : {'rock':'You Lose', 'paper':'You Win', 'scissors':'It was a Tie'}}

    print 'Computer chose', c_choice
    print win_table[p_choice][c_choice]

    new = raw_input('Play again ? (Y/N):  ')
    if new.lower() == 'y':
        choice()
    else:
        print 'Have a nice day !'

choice()

win_table dictionary names are player choices and dictionary keys are computer choices, seems more efficient to me. I haven't run any memory usage tests, though.

The biggest problem with this code is that you are using functions as glorified GOTO labels, when choice() is called from within choice, when choice() is called from play, and when choice() is called initially.  If you hit CtrlC after a few rounds, you'll see that the call stack is inappropriately deep.  The remedy is to use functions, return values, and loops properly.

The other change I would suggest is removing the redundancy between the choices list and the win_table.

import random

MOVES = {
    'rock': {'rock':'It was a Tie', 'paper':'You Lose', 'scissors':'You Win'},
    'paper': {'rock':'You Win', 'paper':'It was a Tie', 'scissors':'You Lose'},
    'scissors': {'rock':'You Lose', 'paper':'You Win', 'scissors':'It was a Tie'},
}

def choices():
    """Produce a tuple of the player's choice and the computer's choice."""
    while True:
        player_choice = raw_input('Choose rock, paper or scissors: ')
        if player_choice.lower() in MOVES:
            computer_choice = random.choice(MOVES.keys())
            return player_choice, computer_choice

def reveal(p_choice, c_choice):
    """Print both players' choices and the result of the game."""
    print 'Computer chose', c_choice
    print MOVES[p_choice][c_choice]

def play():
    """Play a round of rock-paper-scissors, then repeat if desired."""
    while True:
        reveal(*choices())
        if raw_input('Play again? (Y/N):  ').lower() != 'y':
            break
    print 'Have a nice day!'

play()

There isn't much benefit to having choices() produce both the player's choice and the computer's choice, though.  You should break that up into two functions.


ERP-system with UserForm that transfers data to next available row in an archive spreadsheet I made this huge code (Edit: huge for such a simple task) for transferring data and adding an ID number to every new row of data. The only functionality flaw I see is that it creates new ID numbers if you transfer blank cells. I was wondering if anyone can help me improve this code? I'm very bad at loops so if there is any possibilities for loops that I overlooked please tell me.

Edit: also had to add another worksheet for temporarily storing the data, so I could copy it as a range, since it is first saved as a value.

Private Sub CommandButton2_Click()

'Variabler for inputs
shipFrom = TextBox1.Text
shipTo = TextBox2.Text
shipDate = TextBox3.Text
NP = TextBox4.Text
desc = TextBox10.Text
gramEx = TextBox6.Text
tareWeight = TextBox7.Text
weight = TextBox8.Text
dims = TextBox9.Text

Sheets("test").Range("b2").Value = shipFrom
Sheets("test").Range("c2").Value = shipTo
Sheets("test").Range("d2").Value = shipDate
Sheets("test").Range("e2").Value = NP
Sheets("test").Range("f2").Value = desc
Sheets("test").Range("g2").Value = gramEx
Sheets("test").Range("h2").Value = tareWeight
Sheets("test").Range("i2").Value = weight
Sheets("test").Range("j2").Value = dims

Dim sf As Range
Set sf = Sheets("test").Range("b2")

Dim st As Range
Set st = Sheets("test").Range("c2")

Dim d As Range
Set d = Sheets("test").Range("d2")

Dim l As Range
Set l = Sheets("test").Range("e2")

Dim i As Range
Set i = Sheets("test").Range("f2")

Dim g As Range
Set g = Sheets("test").Range("g2")

Dim t As Range
Set t = Sheets("test").Range("h2")

Dim w As Range
Set w = Sheets("test").Range("i2")

Dim di As Range
Set di = Sheets("test").Range("j2")

Dim nextRowB As Range
Set nextRowB = Sheets("Arkiv").Range("B" & Rows.Count).End(xlUp).Offset(1, 0)

Dim nextRowC As Range
Set nextRowC = Sheets("Arkiv").Range("C" & Rows.Count).End(xlUp).Offset(1, 0)

Dim nextRowD As Range
Set nextRowD = Sheets("Arkiv").Range("D" & Rows.Count).End(xlUp).Offset(1, 0)

Dim nextRowE As Range
Set nextRowE = Sheets("Arkiv").Range("E" & Rows.Count).End(xlUp).Offset(1, 0)

Dim nextRowF As Range
Set nextRowF = Sheets("Arkiv").Range("F" & Rows.Count).End(xlUp).Offset(1, 0)

Dim nextRowG As Range
Set nextRowG = Sheets("Arkiv").Range("G" & Rows.Count).End(xlUp).Offset(1, 0)

Dim nextRowH As Range
Set nextRowH = Sheets("Arkiv").Range("H" & Rows.Count).End(xlUp).Offset(1, 0)

Dim nextRowI As Range
Set nextRowI = Sheets("Arkiv").Range("I" & Rows.Count).End(xlUp).Offset(1, 0)

Dim nextRowJ As Range
Set nextRowJ = Sheets("Arkiv").Range("J" & Rows.Count).End(xlUp).Offset(1, 0)

sf.Copy nextRowB
st.Copy nextRowC
d.Copy nextRowD
l.Copy nextRowE
i.Copy nextRowF
g.Copy nextRowG
t.Copy nextRowH
w.Copy nextRowI
di.Copy nextRowJ

Dim ID As Range
Set ID = Sheets("Arkiv").Range("A" & Rows.Count).End(xlUp).Offset(1, 0)

If IsEmpty(Sheets("Arkiv").Range("B2").Value) Then

    MsgBox "Please fill in the <Ship From> box"

Else

    'Sheets("Arkiv").Range("A" & Rows.Count).End(xlUp).Offset(1, 0) = "1"
    Sheets("Arkiv").Range("A2").Value = "1"

End If

If IsEmpty(Sheets("Arkiv").Range("C2").Value) Then

Else

    ID = Sheets("Arkiv").Range("A" & Rows.Count).End(xlUp).Value + 1

End If

Sheets("Arkiv").Activate
MsgBox "The ID number for this data is: " & Sheets("Arkiv").Range("A" & Rows.Count).End(xlUp).Value

End Sub

Ouch. I say this in a kind way - the code is painful to read.

Use Option Explicit at the top of every module - always!
Properly indent your code - it makes it easier to read, easier to pick up inconsistent logic and easier to maintain.

Arrays, and Excel's ability to convert values of ranges to arrays is your friend in this instance. No need for loops.

Private Sub CommandButton2_Click()
Dim inputs as Variant 'Variabler for inputs
    inputs = Array(TextBox1.Text, TextBox2.Text, TextBox3.Text, TextBox4.Text, _
                   TextBox10.Text, TextBox6.Text, TextBox7.Text, TextBox8.Text, _
                   TextBox9.Text)
    ' order is important for you here.
    ' shipFrom, shipTo, shipDate, NP, desc, gramEx, tareWeight, weight, dims

Dim nextRowB As Range
    Set nextRowB = Sheets("Arkiv").Range("B" & Rows.Count & ":J"& Rows.Count).End(xlUp).Offset(1, 0)
    nextRowB.Value = inputs

    ' Now that we put everything into the array, the individual ranges and the ".copy" is not longer required
    ' sf.Copy nextRowB etc <-- not needed, see line of code above!

Dim ID As Range
    Set ID = Sheets("Arkiv").Range("A" & Rows.Count).End(xlUp).Offset(1, 0)
    If IsEmpty(Sheets("Arkiv").Range("B2").Value) Then '<-- This is your logic, but not sure if this is really what you want.
    ' Is it really only row 2 that you check?
        MsgBox "Please fill in the <Ship From> box"
    Else
        'Sheets("Arkiv").Range("A" & Rows.Count).End(xlUp).Offset(1, 0) = "1"
        Sheets("Arkiv").Range("A2").Value = "1"
    End If
    If Not IsEmpty(Sheets("Arkiv").Range("C2").Value) Then ' added "Not" here, why have an empty code block?
    'Else <-- why have an empty code block?
        ID = Sheets("Arkiv").Range("A" & Rows.Count).End(xlUp).Value + 1
    End If
    Sheets("Arkiv").Activate
    MsgBox "The ID number for this data is: " & Sheets("Arkiv").Range("A" & Rows.Count).End(xlUp).Value
End Sub

Two other things to look at to make this more readable:

Consider assigning "Sheets("Arkiv")" to a variable (e.g. ws) and
using that throughout.
Consider using With Sheets("Archiv") block.

And finally, ensure that all references to range objects and methods are fully qualified. This includes the Rows elements within the Range(...) code.

Edit (From Stack Overflow @AhmedAU, alternate way of inserting the values - this has been tested and works according to OP):

Dim inputs As Variant
inputs = Array(TextBox1.Text, TextBox2.Text, TextBox3.Text, TextBox4.Text, TextBox10.Text, TextBox6.Text, TextBox7.Text, TextBox8.Text, TextBox9.Text)
Dim nextRowB As Range
Set nextRowB = Sheets("Arkiv").Range("B" & Rows.Count).End(xlUp).Offset(1, 0)
nextRowB.Resize(1, UBound(inputs) + 1).Value = inputs


Building a basic framework I am trying to build a very basic JS framework for my own uses, ultimately collating a lot of pieces of JavaScript I have created over the years. But I would not call myself a JavaScript expert.

What I have written below works well enough but I really would like to know if I am making any fundamental errors in the way this is set up, rather than re-writing a lot of code further down the line.

This basic start point is mirroring the way jQuery can be written to select an element and chain an event to that object, but using all the querySelectorAll instead of the older methods.

If anyone can find basic or advanced flaws in what I've done that would be most useful.

What you've got is not a framework. It is a library.

I can see you are basically creating a jQuery-like wrapper for selecting DOM nodes and event handling. Nothing terribly wrong with the main idea.

Why have a function called Framework which has a function inside of it? Every call to Framework(...) causes myFramework to be recreated. Instead, just use a regular prototype-based "class" in JavaScript:

function Framework(sel) {
    if (!(this instanceof Framework)) {
        // return a new Framework object if called without the "new" keyword
        return new Framework(sel);
    }

    this.sel = sel;

    if (this.sel) {
        if (typeof this.sel !== 'string') {
            this.selArray = new Array(this.sel);
        } else {
            this.what = this.sel.charAt(0);

            if (this.what == ".") {
                this.selArray = document.querySelectorAll(this.sel);
            } else if (this.what == "[") {
                this.selArray = document.querySelectorAll(this.sel);
            } else if (this.what == "#") {
                this.sel = this.sel.substr(1);
                this.selArray = new Array(document.getElementById(this.sel));
            }
        }
    }
}

Framework.prototype = {
    constructor: Framework,

    ready: function(func) {
        // ...
    },

    click: function(func) {
        // ...
    }
};

The code that inspects the selector and calls querySelectorAll or getElementById is completely redundant and unneeded. If this.sel is a string, just blindly pass it to document.querySelectorAll. You've gained nothing by inspecting the selector. The speed difference between getElementById("foo") and querySelectorAll("#foo") will be negligible at worst, so why complicate your code?

if (this.sel) {
    this.selArray = typeof this.sel === "string"
        ? document.querySelectorAll(this.sel)
        : [this.sel];
}

Optimizing The Attaching Of Events

In your code, you loop over the array of elements, and on each iteration you test for attachEvent or addEventListener. Browsers won't magically change their native DOM API, so testing for these methods repeatedly when the result will be the same is extra work for the browser. Instead, create a method that does the event binding, which we will call bind.

Instead of testing for the "attachEvent" method on each iteration of the loop in click, test it once:

Framework.prototype = {
    constructor: Framework,

    ready: function(func) {
        // ...
    },

    click: function(func) {
        for (var i = 0; i < this.selArray.length; i++) {
            this.bind(this.selArray[i], "click", func);
        }

        return this;
    },

    bind: !!document.addEventListener
        ? function(element, event, func) {
            element.addEventListener(event, func, false);
            return this;
        }
        : function(element, event, func) {
            element.attachEvent(event, "on" + func);
            return this;
        }
};

The bind method is set on the prototype and makes use of a ternary operator to assign a function to the bind property that supports the current browser's native API instead of checking for support.

       // The Test: Does the browser have a document.addEventListener method?
    bind: !!document.addEventListener

     // Yes: The browser does. Use this function to bind events using standard API
        ? function(element, event, func) {
            element.addEventListener(event, func, false);
            return this;
        }

     // No: The browse does not. Use Internet Explorer's API
        : function(element, event, func) {
            element.attachEvent(event, "on" + func);
            return this;
        }

The decision to use attachEvent or addEventListener is made once at page load. The Framework.prototype.bind method will directly use the supported API for each browser rather than test for it.

More information on ternary operators: Conditional (ternary) Operator

Implicitly Declared Global Variables

Don't forget that variable assignments without var before the variable name create implicitly declared global variables. The for loop in the click method does this with the i variable, and your original Framework method mysteriously declares a global variable newTask.


Implementing a Scheduler(something that implements a given schedule) I am trying to implement a class that implements a given schedule. I am trying to design this Scheduler class so it can be paused and everything. I have just written some example code so I can better convey my requirements. I would be grateful if more experienced minds could take a look and tell me if what I have right now is the right way to go about things or if there are better ways to do this. I'd really appreciate some thoughts.

class Scheduler {

    private object locker = new object();
    private System.Timers.Timer timer = new System.Timers.Timer();
    private int[] schedule = null;
    private int scheduleIndex = 0;
    private bool offline = true;
    private bool timerElapsed;

    private Thread th;

    public Scheduler() {
        timer.Elapsed += new System.Timers.ElapsedEventHandler(TimerElapsedHandler);
        th = new Thread(RunSchedule);
        th.Start();
    }

    public void RunSchedule() {
        lock (locker) {
            while (schedule == null || scheduleIndex > schedule.length) // this will not throw a NullReferenceException since schedule == null is being checked first
                Monitor.Wait(locker);

            while (offline)
                Monitor.Wait(locker);

            // pick up next schedule and start timer and wait till timer finishes
            int nextSchedule = schedule[scheduleIndex];

            // call external program to execute schedule

            timer.Interval = 120 * 1000;
            timerElapsed = false;
            timer.Start();
            while(!timerElapsed)
                Monitor.Wait(locker);
        }
    }

    public void Offline() {
        lock (locker) {
            // pause thread executing RunThread
            offline = true;
            // also kill timer if running
            if (timer.Enabled) {
                timer.Stop();
                timerElapsed = true;
                Monitor.Pulse(locker);
            }
        }

    }

    public void Online() {
        lock (locker) {
            //Pulse thread executing RunThread
            offline = false;
            Monitor.Pulse(locker);
        }
    }

    public void OfflineTooLong() {
        lock(locker) {
            this.schedule = null;
        }
    }

    public void ScheduleReady(int[] schedule) {
        lock (locker) {
            this.schedule = schedule;
            this.scheduleIndex = 0;
            Monitor.Pulse(locker);
        }
    }

    private void TimerElapsedHandler(object sender, System.Timers.ElapsedEventArgs) {
        lock(locker) {
            scheduleIndex++;
            timerElapsed = true;
            Monitor.Pulse(locker);
        }
    }
}

As you can see from the code, the intent is to "pause" the thread implementing the schedule when Offline and restart the thread when Online. Also, if you've run through all schedules(ie either schedule == null because you've been Offline too long or scheduleIndex > schedule.length), it should wait to receive a new schedule.

Any thoughts and help will be much appreciated.

Your code is super-comlicated for such simple logic. I'm also pretty sure it deadlocks in pretty much any use case.

class Scheduler : IDisposable
{
    private readonly object scheduleLock = new object();
    private int[] schedule = null;
    private int scheduleIndex = 0;
    private readonly ManualResetEvent collectionResetEvent = new ManualResetEvent(false);
    private readonly AutoResetEvent delayEvent = new AutoResetEvent(false);
    private volatile bool working;

    private Thread th;

    public Dispose()
    {
        Offline();
    }

    public void RunSchedule()
    {
        while (true)
        {
            collectionResetEvent.WaitOne();
            if (!working) return;

            lock (scheduleLock)
            {
               if (schedule == null || scheduleIndex >= shedule.Count)
               {
                   collectionResetEvent.Reset();
                   if (!working) return;
               }
               else
               {
                   var nextSchedule = schedule[scheduleIndex++];
                   //execute,
                   //probably can be done outside the lock
               }
            }

            delayEvent.WaitOne(120 * 1000);
        }
    }

    public void Offline()
    {
        if (!working) return;
        working = false;
        delayEvent.Set();
        collectionResetEvent.Set();
        th.Join();
        th = null;
    }

    public void Online()
    {
        if (working) return;
        working = true;
        th = new Thread(RunSchedule);
        th.Start();
    }

    public void OfflineTooLong()
    {
        ScheduleReady(null);
    }

    public void ScheduleReady(int[] schedule)
    {
        lock (scheduleLock)
        {
            schedule = schedule;
            scheduleIndex = 0;
            collectionResetEvent.Set();
            delayEvent.Set();
        }
    }
}

This is just an example of how it can be refactored, to give you an idea. It can still potentially deadlock (for example if you call offline and dispose at the same time from different threads) so it requires some tweaking depending on your requirements. RunSchedule() method is somewhat hard to read, it should be splitted into sub-methods. BartoszKP makes a perfectly valid points as well, check his answer, if you haven't already.


Capitalizing a string I wrote the following function that takes a string and returns a new string with each word capitalized: first letter uppercase, following letters lower-cased.

It works, but I would be interested in seeing how a more sophisticated Haskell programmer would do this (if there is already a built-in, great, but still interested in an example implementation).

ghci> let capWord word = [toUpper $ head word] ++ (map toLower $ tail word)
ghci> let capitalize sentence = unwords $ map capWord $ words sentence
ghci> capitalize "the quick brown fox jUMPS OVER thE LaZY DOG"
"The Quick Brown Fox Jumps Over The Lazy Dog"

head and tail are partial functions, meaning that they are only defined for non-empty lists and will fail on empty ones. Because of this your capWord function is also partial and its strongly considered a bad practice to define such functions. In fact, the head and tail are also considered a bad heritage and in some alternative preludes it's been deliberately decided not to export such functions. So, firstly, you can redefine your function with pattern matching and make it total.

Secondly, you're introducing a redundant list for the first letter, instead you can prepend it using the : (pronounced "cons") operator.

Summing up we get the following definition:

capWord [] = []
capWord (h:t) = toUpper h : map toLower t

Alternatively (essentially the same):

capWord word = case word of
  [] -> []
  (h:t) -> toUpper h : map toLower t

The capitalize function is fine.


Sorted insert on a doubly linked list (HackerRank) I've decided to do a simple linked list question to brush up my Java & CS.

This is the solution for

  Given a reference to the head of a doubly-linked list and an integer,
  \$data\$ , create a new DoublyLinkedListNode object having data value
  \$data\$
  and insert it into a sorted linked list while maintaining the sort.
  Full question

   /*
     * For your reference:
     *
     * DoublyLinkedListNode {
     *     int data;
     *     DoublyLinkedListNode next;
     *     DoublyLinkedListNode prev;
     * }
     *
     */
    static DoublyLinkedListNode sortedInsert(DoublyLinkedListNode head, int data) {
        DoublyLinkedListNode nodeToInsert = new DoublyLinkedListNode(data);
        if (head == null) return nodeToInsert;
        DoublyLinkedListNode current = head;
        while (current != null) {
            if (data < current.data && current.prev == null) {
                current.prev = nodeToInsert;
                nodeToInsert.next = current;
                return nodeToInsert;
            }
            if (data >= current.data && current.next == null) {
                current.next = nodeToInsert;
                nodeToInsert.prev = current;
                break;
            }
            if (data >= current.data && data <= current.next.data) {
                DoublyLinkedListNode temp = current.next;
                current.next = nodeToInsert;
                nodeToInsert.prev = current;
                temp.prev = nodeToInsert;
                nodeToInsert.next = temp;
                break;
            }
            current = current.next;
        }
        return head;
    }

This passes all test cases.

One thing I take issue with is that all of this code is in a static. If you're already drinking the Java kool-aid, you can have a brief static wrapper to do your if (head == null) shortcut, but the rest of the code should be one or more methods on DoublyLinkedListNode. Rather than continuing to reference your parameter data, the instance saved to nodeToInsert would, in the method you write, access its this.data.

Overall the question asks you to do a thing that sucks, so it isn't your fault: sorted insertion to a linked list is O(n), but you can do much better with a different choice of data structure, such as an AVL tree. If you're interested, here's an implementation of a sorted list in Java.


Sum of all Paths in Binary Tree Problem

  For the given binary tree return the list which has sum of every paths in a tree. i.e Every path from root to leaf.

I've written following solution.

void check()
    {
        List<Integer> out = new ArrayList<>();
        leafsum(root, 0, out);
        System.out.println(out);
    }

    void leafsum(TreeNode root, int curr , List<Integer> sum)
    {

        if(root != null)
        {
            leafsum(root.left, curr+root.data, sum);
            if(root.left == null && root.right == null ) sum.add(curr+root.data);
            leafsum(root.right, curr+root.data, sum);
        }

    }

  Inorder traversal of Tree

2 4 3 5 1 9 2 5 15

root = new TreeNode(5);
root.left = new TreeNode(4);
root.left.left = new TreeNode(2);
root.left.right = new TreeNode(3);
root.right = new TreeNode(9);
root.right.right = new TreeNode(5);
root.right.left = new TreeNode(1);
root.right.right.left = new TreeNode(2);
root.right.right.right = new TreeNode(15);

  Output

[11, 12, 15, 21, 34]

I would like review about improvements and suggestions.

Formatting
The formatting could be nicer and doesn't follow Java conventions.

The indention isn't correct (maybe a copy and paste error).
Opening braces belong on the same line as the method header/statement.
There are random superfluous spaces (after int curr and root.right == null) and missing spaces around the + operator in curr+root.data
There should be a space between keywords and opening brackets (if (...).
Braces should always be used around a conditional block, even if it only contains a single statement.
There shouldn't be more than a single blank line at a time and there shouldn't any at all inside leafsum in my opinion.

Names
The parameter names could be better:

root should be node.
There is no need to abbreviate curr. It should be current or maybe even currentSum.
The list should have a plural name: sums.

The method itself should also have a plural name such as leafSums.
Early return
In order to minimize indention depth return early out of leafsum instead of putting the complete method body inside the if block:
void leafsum(TreeNode root, int curr, List<Integer> sum) {
    if (root == null) {
      return;
    }
    // method body here.
}

DRY
You are repeating the sum curr + root.data three times.
Handling results
I'm not a big fan creating, carrying around and mutating a list for the results, however your way is probably the least convoluted way with Java's standard collection library. Personally I'd do something like:
static List<Integer> leafSums(TreeNode node, int currentSum) {
    if (node == null) {
        return Collections.emptyList();
    }

    int newSum = currentSum + node.data;

    List<Integer> leftSums = leafSums(node.left, newSum);
    List<Integer> rightSums = leafSums(node.right, newSum);

    List<Integer> sums = new ArrayList<>(leftSums);
    if (node.left == null && node.right == null) {
        sums.add(newSum);
    }
    sums.addAll(rightSums);

    return sums;
}

except I'd look for alternative to ArrayList that allows more efficient list concatenation with a nicer API.


String replace templating utility I am new to Python and I am writing my first utility as a way to learn about strings, files, etc.  I am writing a simple utility using string replacement to batch output HTML files. The program takes as inputs a CSV file and an HTML template file and will output an HTML file for each data row in the CSV file.

CSV Input File: test1.csv

The CSV file, which has  header row, contains some catalog data, one product per row, like below:

stockID,color,material,url
340,Blue and magenta,80% Wool / 20% Acrylic,http://placehold.it/400
275,Purple,100% Cotton,http://placehold.it/600
318,Blue,100% Polyester,http://placehold.it/400x600

HTML Template Input File: testTemplate.htm

The HTML template file is simply a copy of the desired output with string replace tags %s placed at the appropriate locations:

Stock ID: %s

%s

%s

The Python is pretty straight forward I think.  I open the template file and store it as a string.  I then open the CSV file using the csv.dictreader() command.  I then iterate through the rows of the CSV, build the file names and then write the output files using string replacement on the template string using the dictionary keys.

import csv

# Open template file and pass string to 'data'.  Should be in HTML format except with string replace tags.
with open('testTemplate.htm', 'r') as myTemplate:
    data = myTemplate.read()
    # print template for visual cue.
    print('Template passed:\n' + '-'*30 +'\n' + data)
    print('-'*30)

# open CSV file that contains the data and store to a dictyionary 'inputFile'.
with open('test1.csv') as csvfile:
    inputFile = csv.DictReader(csvfile)
    x = 0 # counter to display file count
    for row in inputFile:
        # create filenames for the output HTML files
        filename = 'listing'+row['stockID']+'.htm'
        # print filenames for visual cue.
        print(filename)
        x = x + 1
        # create output HTML file.
        with open(filename, 'w') as outputFile:
            # run string replace on the template file using items from the data dictionary
            # HELP--> this is where I get nervous because chaos will reign if the tags get mixed up
            # HELP--> is there a way to add identifiers to the tags?  like %s1 =row['stockID'], %s2=row['color'] ... ???
            outputFile.write(data %(row['stockID'], row['color'], row['material'], row['url']))

# print the number of files created as a cue program has finished.
print('-'*30 +'\n' + str(x) + ' files created.')

The program works as expected with the test files I have been using (which is why I am posting here and not on SO).  My concern is that it seems pretty fragile.  In 'production' the CSV file will contain many more columns (around 30-40) and the HTML will be much more complex, so the chances of one of the tags in the string replace getting mixed seems pretty high. is there a way to add identifiers to the tags?  like %s1 =row['stockID'], %s2=row['color'] ...? that could be placed either in the template file or in the write() statement (or both)?  Any method alternatives or improvements I could learn would be great (note I am well aware of the Makos and Mustaches of the world and plan to learn a couple of template packages soon.)

Python has a number of templating options, but the simplest to start is probably the string.Template one described in https://docs.python.org/3/library/string.html#template-strings

This supports targets such as $StockId and is used as below

>>> from string import Template
>>> s = Template('$who likes $what')
>>> s.substitute(who='tim', what='kung pao')
'tim likes kung pao'

If you need more output options, look at the string.format functionality, but this is probably best for starting with.


Text-based User Interface in Python I wanted to make a GUI-ish (a TUI) thing in the terminal as I have no experience with PyGame or the like. It works, but how can I improve this?

import os, colorama
from msvcrt import getch
from colorama import Fore, Back, Style

colorama.init()

def newGame():
    print("Sorry, currently not a feature")

def cont():
    print("We can't do that")

def highscores():
    print("Yep, again, no can do")

class comm:
    def __init__(self, comm):
        self.comm = comm

def getKeyInput():
    key = ord(getch())
    if key == 224:
        key = ord(getch())
    if key == 72: return 1 # Up arrow
    elif key == 80: return -1 # Down arrow
    elif key == 13: return 0 # Enter

class Menu:
    def __init__(self,
                 title,
                 options, # GUI Options (a list)
                 comms = []): # A list of actions corresponding to options
        self.options = options
        self.title = title
        self.comms = comms
        self.pointer = 0

    def show(self):
        os.system("cls")
        buffer = "-"*len(self.title)
        print("{}\n{}\n{}".format(buffer, self.title, buffer))

        for i in self.options:
            colour = Fore.WHITE + Back.BLACK
            if self.pointer == self.options.index(i): # This is why self.options cannot be a dictionary
                colour = Back.WHITE + Fore.BLACK
            print("{}{}".format(colour, i))
            colour = Fore.WHITE + Back.BLACK
            print(colour, end = '')

        key = getKeyInput()

        if key == 0:
            os.system("cls")
            if self.comms:
                self.comms[self.pointer].comm()
                return 0
            print('Picking {}'.format(self.options[self.pointer]))
            return 0

        self.pointer -= key
        self.pointer = max(min(self.pointer, len(self.options)-1), 0)
        self.show()

if __name__ == "__main__":
    m = Menu("Main Menu",

             ['Start New Game',
              'Continue',
              'Highscores',
              'Quit'],

             [comm(lambda : newGame()),
              comm(lambda : cont()),
              comm(lambda : highscores()),
              comm(lambda : quit())])
    m.show()

General

Try to comply with PEP-8. PEP-8 is the name of the official Python style guide. Complying with it makes your code easier to read. Ultimately, developers can more easily collaborate on a project.

Every import-statement should be on a separate line. The only exception is importing 'from' a module / package. Furthermore, imports should be grouped:

standard library imports
third-party imports
local project imports

... Where the groups are separated by a single empty line.1

Putting this together:

from msvcrt import getch
import os
# Sort alphabetically

import colorama
from colorama import Fore, Back, Style

Functions should follow the snake_case naming convention.2 Classes should follow the PascalCase naming convention.3
Top-level function and class definitions should be preceded by two empty lines.4
Keyword arguments should separate their name and default value only by an equals-sign ('='), not by any spaces.5
Don't mix single and double quotes. They're functionally the same, but when you start mixing them, this can become confusing to people who are used to other languages, like C, where there is a difference.

In getKeyInput(), you use 7 magic numbers.

Instead of adding comments to explain what these numbers represent, you can give them a name. For example, by declaring these constants at the top of the file:

NEXT_KEY_IS_ARROW_KEY = 224
UP_ARROW_KEY = 72
DOWN_ARROW_KEY = 80
ENTER_KEY = 13

POINTER_UP = 1
POINTER_DOWN = -1
PRESS_BUTTON = 0

... no additional explanation is needed in getKeyInput():

def getKeyInput():
    key = ord(getch())
    if key == NEXT_KEY_IS_ARROW_KEY:
        key = ord(getch())
    if key == UP_ARROW_KEY:
        return POINTER_UP
    elif key == DOWN_ARROW_KEY:
        return POINTER_DOWN
    elif key == ENTER_KEY:
        return PRESS_BUTTON

(Even better would be to use an enum).

Bugs

You forgot to define quit().
I'm not sure if this is a quirk of my terminal emulator / MacOS Sierra, or if it's present regardless of setup, but the bounds checking in Menu.show() doesn't do its job: I can keep moving the pointer down forever, which causes interesting screen glitches.6, 7
This isn't strictly a bug, but if a user presses anything other than △, ▽, or return, the program crashes.

Public API

I know you used the windows-tag, but you might reach a broader audience by making this platform-independent.

msvcrt and os.system("cls") only work on Windows platforms. If I run the program on MacOS, I get a ModuleNotFoundError. Even if I could patch that, the os.system("cls") call would print out an error message instead of clearing the screen. There's three partially satisfying solutions:

msvcrt.getch() returns a single keypress. By default, the terminal is in 'cooked' mode. (I'm using UNIX terminology, not sure how much of this applies to Windows) so sys.stdin.read(1) won't work, as the user must press enter first. By putting the terminal in 'cbreak' mode, we can read one keypress at a time. The standard library tty.setcbreak() function does exactly that. One small issue is that there is no standard library function for resetting the terminal mode. Luckily, it's as simple as saving the terminal options, putting the terminal in cbreak mode, and restoring the saved options when done.
On Windows, the command for clearing the screen is '$ cls'. On most other systems, the command is '$ clear'. Keeping that in mind (pseudocode):

if platform is Windows then
    $ cls
else
    $ clear

If your terminal supports ANSI escape codes, you don't even need to make a system call to clear the screen. Instead, you print the 'Erase in Display' code (source, modified for formatting):

  CSI n J

  ED – Erase in Display

  Clears part of the screen. If n is 0 (or missing), clear from cursor to end of screen. If n is 1, clear from cursor to beginning of the screen. If n is 2, clear entire screen (and moves cursor to upper left on DOS ANSI.SYS). If n is 3, clear entire screen and delete all lines saved in the scrollback buffer (this feature was added for xterm and is supported by other terminal applications).

A major drawback of this approach is that there is no straightforward way of testing whether or not a terminal supports these codes. Most modern terminals do, but there are some exceptions.8

Unless you plan on extending the class with methods, comm is superfluous. A lambda is enough.
Move colorama.init() to the if __name__ == "__main__" body.
Try abbreviating names less. While abbreviating is customary in other languages, in Python, developers should be as verbose as possible (within reason).
Make functions and attributes private when it makes sense. Especially when you're designing a public API, you want to hide away all the unimportant / difficult to comprehend stuff. For example, in Menu.__init__(), all the arguments should be private attributes, because developers should not be allowed to modify them after initialization.
Add docstrings to public functions and classes.

Names

getKeyInput() is a misleading name. It doesn't return user input, it returns an action (move the cursor up, move the cursor down, press a button) based on the input. I suggest get_action_from_user_input().
In this context, the variable name buffer suggests it has to do with I/O buffering, which is not true. I suggest decorative_line (?).
Menu. Well, sure. But what kind of menu? It's a menu where you make a selection → SelectionMenu.
Menu.show() tells only part of the story. It doesn't just output stuff, it also waits for user input: it is waiting for the user to press return. How about Menu.await_selection()?

My take on it

import enum
import os
import sys

import colorama as cla

try:
    import msvcrt
except ImportError:
    # POSIX
    import termios
    import tty

class Terminal:
    """A `Terminal` instance represents a terminal (emulator)."""

    _ANSI_CODE_CLEAR_SCREEN = "\033[2J"
    _ANSI_CODE_MOVE_CURSOR_TOP_LEFT = "\033[1;1H"

    def __init__(self, ansi_escape_codes_supported=False):
        """Initialize the terminal.
        If this method is called on a POSIX system, set the terminal
        mode to 'cbreak'.

        Arguments:
            - ansi_escape_codes_supported [bool]: Whether or not to
            assume ANSI escape codes are supported. Defaults to
            `False`. When in doubt, don't set to `True`.
        Returns:
            - `None`.
        """
        if os.name == "posix":
            self.posix = True
        elif os.name == "nt":
            self.posix = False
        else:
            raise RuntimeError("Only POSIX and NT systems are supported.")
        self._ansi_escape_codes_supported = ansi_escape_codes_supported
        self._prepare()

    def _prepare(self):
        """Prepare the terminal.
        On NT systems, this is a no-op. On POSIX systems, save the
        terminal attributes, then set the terminal mode to 'cbreak'
        using `tty.setcbreak()`.

        Returns:
            - `None`.
        """
        if not self.posix:
            return
        self._saved_terminal_options = termios.tcgetattr(sys.stdin)
        tty.setcbreak(sys.stdin)

    def clear_screen(self):
        """Clear the screen.
        If ANSI escape codes are supported, print ED (CSI 2 J), then
        print CUP (CSI 1 ; 1 H). Otherwise: on POSIX systems, call
        '$ clear'. On NT systems, call '$ cls'.

        Returns:
            - `None`.
        """
        if self._ansi_escape_codes_supported:
            print(self._ANSI_CODE_CLEAR_SCREEN)
            print(self._ANSI_CODE_MOVE_CURSOR_TOP_LEFT, end="")
        elif self.posix:
            os.system("clear")
        else:
            os.system("cls")

    def get_character(self):
        """Get a single character from stdin.

        Returns:
            - [str] A single character.
        """
        if self.posix:
            return sys.stdin.read(1)
        return msvcrt.getch()

    def finalize(self):
        """Finalize the terminal.
        On NT systems, this is a no-op. On POSIX systems, reset the
        saved terminal attributes.

        Returns:
            - `None`.
        """
        if not self.posix:
            return
        termios.tcsetattr(
            sys.stdin,
            termios.TCSANOW,
            self._saved_terminal_options
        )

class POSIXCodePointValues:
    """
    MacOS Sierra code point values.
    NOTE: Don't rely on this!
    There are no standardized code points for arrow keys, so these are
    the values for my machine. They are most likely *not* the same for
    you.
    """
    NEXT_KEY_IS_ARROW_KEY_SEQ = (27, 91)
    UP_ARROW_KEY = 65
    DOWN_ARROW_KEY = 66
    ENTER_KEY_SEQ = (10, 13)
    # Either 10 or 13

class NTCodePointValues:
    """
    NT code point values.
    NOTE: These are most likely not very portable either.
    """
    NEXT_KEY_IS_ARROW_KEY = 224
    UP_ARROW_KEY = 72
    DOWN_ARROW_KEY = 80
    ENTER_KEY = 13

class UIAction(enum.Enum):
    """Values:
        - POINTER_UP (1): Move the cursor up one row.
        - POINTER_DOWN (2): Move the pointer down one row.
        - PRESS_BUTTON (3): Press the currently selected button.
        - NONE (4): Do nothing.
    """

    POINTER_UP = 1
    POINTER_DOWN = 2
    PRESS_BUTTON = 3
    NONE = 4

def _get_action_from_user_input_nt(terminal):
    code_point = ord(terminal.get_character())
    if code_point == NTCodePointValues.ENTER_KEY:
        return UIAction.PRESS_BUTTON
    elif code_point != NTCodePointValues.NEXT_KEY_IS_ARROW_KEY:
        return UIAction.NONE

    # Definitely an arrow key
    code_point = ord(terminal.get_character())
    if code_point == NTCodePointValues.UP_ARROW_KEY:
        return UIAction.POINTER_UP
    elif code_point == NTCodePointValues.DOWN_ARROW_KEY:
        return UIAction.POINTER_DOWN
    # Left or right arrow key
    return UIAction.NONE

def _get_action_from_user_input_posix(terminal):
    code_point = ord(terminal.get_character())
    if code_point in POSIXCodePointValues.ENTER_KEY_SEQ:
        return UIAction.PRESS_BUTTON
    elif code_point != POSIXCodePointValues.NEXT_KEY_IS_ARROW_KEY_SEQ[0]:
        return UIAction.NONE

    code_point = ord(terminal.get_character())
    if code_point != POSIXCodePointValues.NEXT_KEY_IS_ARROW_KEY_SEQ[1]:
        return UIAction.NONE

    code_point = ord(terminal.get_character())
    # Definitely an arrow key
    if code_point == POSIXCodePointValues.UP_ARROW_KEY:
        return UIAction.POINTER_UP
    elif code_point == POSIXCodePointValues.DOWN_ARROW_KEY:
        return UIAction.POINTER_DOWN
    # Left or right arrow key
    return UIAction.NONE

def get_action_from_user_input(terminal):
    """Get a UI action from user input.
    Read characters from stdin until a relevant UI action can be
    constructed. See `UIAction` for the possible actions.

    Arguments:
        - terminal [Terminal]: A terminal instance, used for reading
        single characters.

    Returns:
        - [UIAction] A UI action.
    """
    if terminal.posix:
        return _get_action_from_user_input_posix(terminal=terminal)
    return _get_action_from_user_input_nt(terminal=terminal)

class SelectionMenu:
    """A `SelectionMenu` represents a group of options,
    from which the user can pick only one.
    """

    _RESET_COLOUR = cla.Fore.WHITE + cla.Back.BLACK

    def __init__(
        self,
        title,
        options,
        callbacks,
        terminal,
    ):
        """Initialize the menu.

        Arguments:
            - title [str]: The text to display as a title.
            - options [list]: A list of options the user can pick from.
              The elements must be `str` instances.
            - callbacks [list]: A list of functions to call when the user
              makes a selection. The length of this list must be equal to
              the amount of options provided.
            - terminal [Terminal]: A terminal instance to use for I/O.

        Returns:
            - `None`.
        """
        self._title = title
        self._options = options
        self._callbacks = callbacks
        self._terminal = terminal
        self._pointer = 0

    def await_selection(self):
        """Wait for the user to make a selection.
        Handle user input, draw the menu, and wait until the user
        presses enter.

        Returns:
            - `None`.
        """
        self._terminal.clear_screen()
        decorative_line = "-" * len(self._title)
        print("{}\n{}\n{}".format(
            decorative_line,
            self._title,
            decorative_line
            )
        )

        for option in self._options:
            colour = cla.Fore.WHITE + cla.Back.BLACK
            if self._pointer == self._options.index(option):
                # i.e. if this is the currently selected button
                colour = cla.Fore.BLACK + cla.Back.WHITE
            print("{}{}\n{}".format(colour, option, self._RESET_COLOUR), end="")

        action = get_action_from_user_input(self._terminal)

        if action == UIAction.PRESS_BUTTON:
            self._terminal.clear_screen()
            if self._callbacks:
                self._callbacks[self._pointer]()
            else:
                print("Picking {}".format(self._options[self._pointer]))
            self._terminal.finalize()
            return None
        elif action == UIAction.POINTER_DOWN:
            self._pointer += 1
        elif action == UIAction.POINTER_UP:
            self._pointer -= 1

        self._pointer = max(min(self._pointer, len(self._options) - 1), 0)
        # Assure the pointer stays within bounds
        self.await_selection()

def margherita():
    print("Margherita it is!")

def pepperoni():
    print("Pepperoni it is!")

def four_cheese():
    print("Four Cheese it is!")

def no_pizza():
    print("Curious.")

if __name__ == "__main__":
    cla.init()

    TITLE = "Pizza"
    BUTTONS = [
        "Margherita",
        "Pepperoni",
        "Four Cheese",
        "Ew! No pizza for me..."
    ]
    ACTIONS = [
        margherita,
        pepperoni,
        four_cheese,
        no_pizza
    ]

    terminal = Terminal(True)

    select_a_pizza = SelectionMenu(TITLE, BUTTONS, ACTIONS, terminal=terminal)
    select_a_pizza.await_selection()

References

1 PEP-8: Imports

2 PEP-8: Function and variable names

3 PEP-8: Class Names

4 PEP-8: Blank Lines

5 PEP-8: Other Recommendations

6 Here's what I mean.

7 Using ANSI escape codes fixes the problem for me, which suggests the bug is related to '$ clear'.

8 Wikipedia: ANSI escape code: Platform support

Appendix: My reasoning

With most of the points I wanted to make out of the way, I want to explain some of the changes I made.

I started out by rewriting getKeyInput() to handle both POSIX and NT systems. After having found out I was also going to need prepare_terminal(), reset_terminal(), clear_screen() and get_character() functions, I decided to extract all terminal-related stuff into a class of its own.
I originally let the Menu constructor initialize a new Terminal instance. That's a pretty bad design choice: it means every time a UI widget is created, a new Terminal instance is created as well; and developers can't interact with the terminal directly. Therefore, I made it a parameter.
Since code point values associated with arrow keys differ per system, I created a class to hold the values for my laptop. From the little research I've done, it seems the values are the same for most recent Windows releases.
Instead of crashing, the UI should ignore meaningless input. That's why I added UIAction.NONE to the UIAction enum.

Appendix: Design

API design

Since you plan on matching PyGame's functionality, you should take the time to think about how you want to design your API. You've taken an event-driven (callback functions) approach, which is probably the most sensible thing to do for user interfaces in general. As it stands, though, the caller can't retrieve the return value of the callback functions, which means in more complex cases, you'd have to modify a shared variable, which can get really messy, really fast. And I haven't even covered threading yet.

Interrupt handling

If I press Ctrl-C, I get a bunch of traceback lines, but more importantly, my terminal is still in cbreak mode. Once your codebase starts growing, it becomes tiresome to wrap every terminal.get_character() call in a try / except-block. You could put the block in terminal.get_character(), but that would mean a forceful exit any time the user presses Ctrl-C. Unfortunately, there's no trivial solution.

Backend

I have my doubts about the UI backend:

Using recursion in Menu.show(). There's really no need to, you might as well use a while-loop.

The following two points only apply if you want your UIs to be dynamic, and you need to create reasonably complex applications.

There's no parent-child relationship. You can create two instances of Menu, but they won't be related to one another. The second instance simply waits for the first one to complete, and then, completely independently, does its thing. If you want to add other types of UI widgets (frames, text labels, input fields, multiline text boxes, sliders, ...), you need a widget hierarchy.
Text is left-aligned, and there's no way of placing it dynamically. Once you've implemented a widget hierarchy, you need a way to map widgets anywhere on-screen. This is where ANSI escape sequences come in handy, because they allow you to move the cursor anywhere you want, and even save the cursor position and restore it later.

Getting this right is possible, but you must be willing to spend time and effort to do research, lay out an API, and document it.


Generating a list of menus with AngularJS Some days ago, I decided to start learning a JS framework in order to gain skills and become more useful at work. I choose Angular for the purpose, and started learning it by building up my "professional" personal website.

In this site, one of the biggest obstacles I could encounter was the extremely "bogging" way of adding new stuff with plain HTML. Thus, after some readings, I made an Angular app that generates an ordered list from a Javascript array of JSON descriptors.

For transparency, I made a snippet being as short as possible, while keeping it functionally identical.

<!DOCTYPE html>

                 {{ x.content }}

Is it OK this way? That final call after body is slightly annoying for me, but I can't do anything against it.

Are there maybe any additional point to improve this code?

Simple answer, no it's not OK this way, you are totally misusing the angular.js purpose.

Anugular.js is a framework in which follows a modular pattern, it was designed by the time web wasn't so rich as nowadays so lots of things in the framework are native behavior today's days, it starts always with a module, like so:

angular.module('MyApp', []);

As you have your module declared, it can be triggered by the ng-app="MyApp" directive, which bootstraps your module in the element you embedded the ng-app attribute into (i.e., it starts an angular.js app in that element) or you can bootstrap it manually using angular.bootstrap(document, ['MyApp']) when the DOM is ready.

I've made an acceptable way of using angular.js with your example and you can check this in the example bellow.

angular.module('MyApp', [])
  .controller('MainCtrl', function($scope) {
    $scope.links = [
      { ref: "./link1.html", content: "First link of the menu" },
      { ref: "./link2.html", content: "Second link of the menu" },
      { ref: "./link3.html", content: "Third link of the menu" }
    ];
  });

       {{ x.content }}

However, the modern way of doing modern web apps these days is using the concept of WebComponents, concept in which angular.js supports since version 1.5.0.

In the snippet bellow, I provided the component approach of your example.

  Disclaimer: You seem not to have understood angular.js' concept properly, so, I don't know where you've been studying angular.js, but I really recommend the angular.js guide which covers pretty much everything you need to know about concepts, architecture, features, etc.

angular.module('MyApp', [])
  .component('app', {
    template: `

           {{ x.content }}

    `,
    controller: function() {
      this.links = [{
          ref: "./link1.html",
          content: "First link of the menu"
        },
        {
          ref: "./link2.html",
          content: "Second link of the menu"
        },
        {
          ref: "./link3.html",
          content: "Third link of the menu"
        }
      ];
    }
  });

  <app />


Add as newest ID to SQLite Database At our company we use an SQLite Database to manage our Drawing IDs (CAD Drawings). The code is written in Python and I need to update it. I think the code I have has some problems, but I am quite unsure so I hope to get feedback and criticism from you all!

1. What my code does:

It saves the ID, Title and Project Number of the Drawing in a SQLite Database
Therefore it has a function GetID to get the latest ID and create a new one from it

Our Drawing IDs look like this "DR0000001". So I need to split the string, convert it to an Integer and add +1. Then rebuild the ID and return it to the OnSave function.

Then if false is not returned I start the save process.

2. Problems I think I have with this code:

I think the time between getting the recent ID and saving the new one to the DB is susceptible to errors like double usage of IDs.
I really think this isn't a professional way to do this and I want to learn the correct way.

3. Here is the Code we used up to this point:

def GetID(self):
    try:
        conn = sqlite3.connect(SQL_PATH)
        c = conn.cursor()
        c.execute("SELECT DrawingID FROM Drawing LIMIT 1 OFFSET (SELECT COUNT(*) FROM Drawing)-1")
        recent_drawing_id = c.fetchall()

        new_drawing_id = "DR"+str(int(recent_drawing_id[0][0][2:])+1).zfill(len(recent_drawing_id[0][0][2:]))

        return new_drawing_id
    except:
        wx.MessageBox("Couldn't get new Drawing ID!", "Info", wx.OK | wx.ICON_INFORMATION)
        return False

def OnSave(self, event, mode):
    drawingID = self.GetID()

    if drawingID != False:
        conn = sqlite3.connect(SQL_PATH)
        c = conn.cursor()

        titel = self.tc2.GetValue()
        project_number = self.tc3.GetValue()

        try:
            c.execute("INSERT into Drawing (DrawingID,Titel,ProjectNumber) values (?,?,?)", (userID,titel,project_number))
            conn.commit()
            c.close()
            return
        except:
            wx.MessageBox("Couldn't create new Drawing entry!", "Info", wx.OK | wx.ICON_INFORMATION)
            return
    else:
        return

Really you don't need DR to be stored as it's a user convenience. That is you can add DR when extracting the value. You also don't need to check calculate the next number as SQlite can do this on your behalf.

To further elaborate if DrawingID is defined using INTEGER PRIMARY KEY the column is then an alias of the normally hidden rowid (if not a WITHOUT ROWID table (rarely used)).

When inserting a row and you don't sepcify a value for the column then a unique value is supplied. This is typically 1 greater than the highest with the first value as 1.

However, there is no guarantee that the unique value will be 1 greater or that if when (if) you reached the maximum number of 9223372036854775807 (basically impossible due to storage device capacity) that a lower (but still unique) number will be assigned.

Using an alias of the rowid will be more efficient not only as the value stored is shorter (maximum of 8 bytes), it is stored in the most efficient (up to twice as fast as other indexes) index. Of course you then don't have the additional inefficiency of trying to replicate this inbuilt behaviour by So I need to split the string, convert it to an Integer and add +1. Then rebuild the ID and return it to the OnSave function.

As an example consider the following :-

/* Cleanup in case existing Environment exists */
DROP TABLE IF EXISTS Drawing;
/* The suggested Schema */
CREATE TABLE IF NOT EXISTS Drawing (DrawingID INTEGER PRIMARY KEY, Title TEXT, ProjectNumber INTEGER);

/* Add various drawings */
INSERT INTO Drawing (Title,ProjectNumber) VALUES
    ('Room1',1),('Room2',1),('Room1',3),('Room3',3),('Room3',1),('Room2',3),('Room1',2);
/* Extracting Project, Drawing Title and Drawing ID */
SELECT 'Project - '||ProjectNumber AS ProjectNumber, Title, 'DR'||printf('%07d',DrawingID) AS DrawingID FROM Drawing ORDER BY ProjectNumber,DrawingId;
/* Cleanup testing Environment */
DROP TABLE IF EXISTS Drawing;

The above results in :-

The actual data in the table being

sorted as above i.e. extracted using SELECT * FROM Drawing ORDER BY  ProjectNumber,DrawingId;) :-

You may wish to have a read of

SQL As Understood By SQLite - CREATE TABLE - ROWIDs and the INTEGER PRIMARY KEY
SQLite Autoincrement

(Noting that althogh this is a link to AUTOINCREMENT it explains why AUTOINCREMENT is best not used in addition to adding information)


isPrefixOf & isSuffixOf Please review and suggest improvements for my isPrefixOf implementation.

isPrefixOf' :: (Eq a) => [a] -> [a] -> Bool
isPrefixOf' [] _ = False
isPrefixOf' _ [] = False
isPrefixOf' (x:xs) (y:ys)
  | null xs && x == y = True
  | x == y            = isPrefixOf' xs ys
  | otherwise         = False

Also, I wrote isSuffixOf baesd on isPrefixOf. It seems debatable to me whether it's clear to understand. Is it idiomatic?

isSuffixOf' :: (Eq a) => [a] -> [a] -> Bool
isSuffixOf' xs ys = isPrefixOf' (reverse xs) (reverse ys)

--isSuffixOf "foobar" "bar" == isPrefixOf "raboof" "rab"

That is very idiomatic Haskell code. The only thing I'd mention is that the empty list is a prefix of everything, so your first case should be

isPrefixOf' [] _ = True

With that change, you can remove

  | null xs && x == y = True

since it will be correctly handled by

  | x == y = isPrefixOf' xs ys

As for implementing isSuffixOf in terms of inPrefixOf, you're in good company; that's how Data.List does it.


Search for string in many WBs and post results into another WB I want to search for a string inserted in TextBox1 on Mainwb within a range of data located in Workbook officerA Worksheet DATA!(A2,G2000), and then paste results found into Workbook Mainwb.sheet("MAIN SCREEN").Range (A5,G500)

I am totally new to VBA and wrote this code quoting from many sources. I'd like to know how it can be improved.

This is used to search within the same workbook:

Sub CommandButton1_Click()
Dim wb1 As Workbook, Wb2 As Workbook
Dim ws1 As Worksheet, ws2 As Worksheet
Dim strSearch As String
Dim f As Variant
Dim fAddress As String
Dim fRow As Long
Dim cellA As Variant
Dim cellB As Variant

 Set Wb2 = Workbooks.Open("C:\Users\elhayani\Desktop\development\AML db\OfficerA.xlsx")
  Set wb1 = Workbooks.Open("C:\Users\elhayani\Desktop\development\AML db\Mainwb.xlsm")
   Set ws2 = wb1.Sheets("MAIN SCREEN").Range("A5:G2000")
   Set ws1 = Wb2.Worksheets("DATA")

strSearch = TextBox1.Value

ws1.Range("A5:G2000").ClearContents
Set dmr = Workbooks.Open("C:\Users\aselhayani\Desktop\Excel Reports\OfficerA.xlsx")
Set dmr = Worksheets("DATA")
strSearch = InputBox("Please enter T24 ID:", "Search Value")

pasteRowIndex = 5

If strSearch = vbNullString Then
    MsgBox ("User canceled, or did not enter a value.")
    Exit Sub
End If

With ws1.Range("A2:G2000")

    Set f = .Find(strSearch, LookIn:=xlValues)
    If Not f Is Nothing Then
        fAddress = f.Address
        Do
            fRow = f.Row
            cellA = ws2.Cells(fRow, 1).Value
            cellB = ws2.Cells(fRow, 2).Value
            cellC = ws2.Cells(fRow, 3).Value
            cellD = ws2.Cells(fRow, 4).Value
            cellE = ws2.Cells(fRow, 5).Value
            cellF = ws2.Cells(fRow, 6).Value
            cellG = ws2.Cells(fRow, 7).Value

            ws1.Cells(pasteRowIndex, 1) = cellA
            ws1.Cells(pasteRowIndex, 2) = cellB
            ws1.Cells(pasteRowIndex, 3) = cellC
            ws1.Cells(pasteRowIndex, 4) = cellD
            ws1.Cells(pasteRowIndex, 5) = cellE
            ws1.Cells(pasteRowIndex, 6) = cellF
            ws1.Cells(pasteRowIndex, 7) = cellG

            pasteRowIndex = pasteRowIndex + 1

            Set f = .FindNext(f)

        Loop While Not f Is Nothing And f.Address <> fAddress
    End If
End With
MsgBox "Search Done"
End Sub

Variables

Good job is making sure to declare all variables with a type:

 Dim wb1 As Workbook, Wb2 As Workbook

However, there's no real reason to group your variables like that; as you see with your strings it would get too long.

More on variables - give them some meaningful names. And avoid that hungarian naming:

Dim sourceWorkbook As Workbook
Dim targetWorkbook As Workbook
Dim sourceSheet As Worksheet
Dim targetSheet As Worksheet
Dim searchValue As String
Dim foundAddress As String
Dim foundRow As Long
Dim searchArray As Variant

Oh no, you didn't declare these variables:

  cellA = ws2.Cells(fRow, 1).Value
  cellB = ws2.Cells(fRow, 2).Value
  cellC = ws2.Cells(fRow, 3).Value
  cellD = ws2.Cells(fRow, 4).Value
  cellE = ws2.Cells(fRow, 5).Value
  cellF = ws2.Cells(fRow, 6).Value
  cellG = ws2.Cells(fRow, 7).Value

Why not use a range or an array? Seems like it will always be columns 1:7. Something like

For Each f In searchRange 'or your specific range
    Set f = searchRange.Find(searchValue, LookIn:=xlValues)
    If Not f Is Nothing And f.Address <> fAddress Then
        ws1.Range(Cells(pasterowindex, 1), (Cells(pasterowindex, 7))) = ws2.Range(Cells(fRow, 1), (Cells(fRow, 7)))
    End If
Next

But with better variables. I also dislike Do loops and stick to For Each loops, but that's my preference. It also can avoid the With block - you're nesting too many things, I think.

You also missed declaring these: dmr, pasteRowIndex

But I don't see you ever actually using dmr?

When you don't define your variable, VBA will declare it as a Variant, which are objects:

  Performance. A variable you declare with the Object type is flexible
  enough to contain a reference to any object. However, when you invoke
  a method or property on such a variable, you always incur late binding
  (at run time). To force early binding (at compile time) and better
  performance, declare the variable with a specific class name, or cast
  it to the specific data type.

By not declaring variables, you could possibly be paying a penalty.

Always turn on Option Explicit. You can have it automatically by going to Tools -> Options in the VBE and checking the Require Variable Declaration option. This way if you have any variables not defined, the compiler will let you know.

Additionally, I have no way of telling what dmr is. Plus first it's a workbook and then it's a range - the only reason it's working is because you didn't declare it and it's inherently a variant. Get two variables with good names in there!

Plus, what's this?

dim ws2 as worksheet
Set ws2 = wb1.Sheets("MAIN SCREEN").Range("A5:G2000")

Setting a worksheet variable to a range? Also why does ws2 reference wb1 and vice-versa? I got really confused on what the source and target actually are.

Praise

Great job using vbNullString instead of " ".
Good job using a Long instead of Integer. Integers - integers are obsolete. According to msdn VBA silently converts all integers to long.
Good job following Standard VBA naming conventions

Misc

When you do this

 ws1.Range("A5:G2000").ClearContents

You already gave the book and sheet a variable, just give this a range variable - it will make the code look cleaner. Additionally if you're using 2000 because you don't know how big the range is,
there is a standard way to find lastRow and lastColumn. That post explains why.

What is this spacing about -

 Set Wb2 = Workbooks.Open("C:\Users\elhayani\Desktop\development\AML db\OfficerA.xlsx")
  Set wb1 = Workbooks.Open("C:\Users\elhayani\Desktop\development\AML db\Mainwb.xlsm")
   Set ws2 = wb1.Sheets("MAIN SCREEN").Range("A5:G2000")
   Set ws1 = Wb2.Worksheets("DATA")

Be sure you align your levels:

Set Wb2 = Workbooks.Open("C:\Users\elhayani\Desktop\development\AML db\OfficerA.xlsx")
Set wb1 = Workbooks.Open("C:\Users\elhayani\Desktop\development\AML db\Mainwb.xlsm")
Set ws2 = wb1.Sheets("MAIN SCREEN").Range("A5:G2000")
Set ws1 = Wb2.Worksheets("DATA")

Also, what happens if the book isn't found or the sheet doesn't exist? Handle those errors!

Same goes for a space in the inputbox - " " <> vbNullString

Also why are you doing this -

strSearch = TextBox1.Value
...
strSearch = InputBox("Please enter T24 ID:", "Search Value")

I don't see that TextBox1 ever being used.

My version

Would look something like this -

Option Explicit

Sub CommandButton1_Click()

Dim sourceWorkbook As Workbook
Dim targetWorkbook As Workbook
Dim sourceSheet As Worksheet
Dim targetSheet As Worksheet
Dim sourceRange As Range
Dim targetRange As Range
Dim searchCell As Range
Dim searchValue As String
Dim targetRow As Long
targetRow = 5
Dim lastRow As Long

On Error GoTo ErrHandler

Set sourceWorkbook = Workbooks.Open("C:\Users\elhayani\Desktop\development\AML db\OfficerA.xlsx")
Set sourceSheet = sourceWorkbook.Worksheets("DATA")
lastRow = sourceSheet.Cells(Rows.Count, 1).End(xlUp).Row
Set sourceRange = sourceSheet.Range("A5:G" & lastRow)

Set targetWorkbook = Workbooks.Open("C:\Users\elhayani\Desktop\development\AML db\Mainwb.xlsm")
Set targetSheet = targetWorkbook.Sheets("MAIN SCREEN")
Set targetRange = targetSheet.Range("A5:G" & lastRow)
targetRange.ClearContents

searchValue = InputBox("Please enter T24 ID:", "Search Value")
If searchValue = vbNullString Or searchValue = " " Then Exit Sub

For Each searchCell In sourceRange 'or your specific range
    Set searchCell = sourceRange.Find(searchValue, LookIn:=xlValues)
    If Not searchCell Is Nothing Then
        targetSheet.Range(Cells(targetRow, 1), (Cells(targetRow, 7))) = sourceSheet.Range(Cells(searchCell.Row, 1), (Cells(searchCell.Row, 7)))
        targetRow = targetRow + 1
    End If
Next
Exit Sub

ErrHandler:
    MsgBox "WHOOPS"

End Sub

Advanced

Your way of doing the loop to .Find the search value is pretty complicated considering you can just .FindNext and avoid the loop altogether, but I know that's a lot to take in right now.


JavaScript Object.defineProperty I'm curious about Object.defineProperty.  It seems awfully verbose for not doing much more than what you can already do in the constructor.  What's the point of it?  Would it be 'bad code' to not use it?  I know you have more control over read/write and enumerability with defineProperty, is that enough to use this verbose syntax?

function Person(age, name) {
  this.name = name;
  this.age  = age;
};

Object.defineProperty(Person.prototype, "age", {
  get: function() {
    return this._age;
  },

  // Added a few things to demonstrate additional logic on the setter
  set: function(num) {
    num = parseInt(num, 10);
    if(num > 0) {
      this._age = num;
    }
  }
});

As requested by tomdemuyt. I'm posting my comments as an answeR:

You usually only use defineProperty in very specific cases. (let's say you wanted validation on age?) - in your case - I would not use it.

Op then asked:

  offhand can you think of anything outside of validation were it might be useful to use defineProperty?

Plenty of reasons:

non-enumerable properties,
frozen properties,
computed properties,
proxies (Create an object based on another object and mirror calls + observe).

There are a lot of good use cases - they're just not very common in the web because of IE8 yet. In your case (age) I would not use Object.defineProperty.


Multi-level image slider Here is my code I'm working with:

$('#slider_1').click( function change_slider(){
  $('#slider_1').addClass('current_tab').siblings('.current_tab').removeClass('current_tab')
  $('#cart_slider_1').show().siblings().hide();
  $('#cart_slider_1').addClass('current').siblings('.current').removeClass('current');
  numItems = $('.current .cart_slide').length;
  sliderwidth = numItems * swidth;
  $('.cart_slides').css({'left': 0, 'width': sliderwidth});
  numloop = numItems-1;
  last_slide = sliderwidth - swidth;
  i=0;
});

$('#slider_2').click( function change_slider(){
  $('#slider_2').addClass('current_tab').siblings('.current_tab').removeClass('current_tab')
  $('#cart_slider_2').show().siblings().hide();
  $('#cart_slider_2').addClass('current').siblings('.current').removeClass('current');
  numItems = $('.current .cart_slide').length;
  sliderwidth = numItems * swidth;
  $('.cart_slides').css({'left': 0, 'width': sliderwidth});
  numloop = numItems-1;
  last_slide = sliderwidth - swidth;
  i=0;
});

It repeats 6 more times, with incrementing #slider numbers and #cart_slider numbers.

I want to make this into one set, and run the variable sets through it. #slider_1, #slider_2, #slider_3, etc... and #cart_slider_1, #cart_slider_2, #cart_slider_3, etc...

The current code works fine; I just want to simplify / condense it.

$('#slider_1').click( function change_slider(){

You don't have to name the function when you pass it to an event handler.

$('#slider_1').click( function() {

An anonymous function is sufficient.

$('#slider_1').addClass('current_tab').siblings('.current_tab').removeClass('current_tab')

This seems to be missing a ; at the end.

But neither of those is what you asked:

var change_slider = function() {
  $(this).addClass('current_tab').siblings('.current_tab').removeClass('current_tab');
  $('#cart_'+$(this).attr('id')).show().siblings().hide();
  $('#cart_'+$(this).attr('id')).addClass('current').siblings('.current').removeClass('current');
  numItems = $('.current .cart_slide').length;
  sliderwidth = numItems * swidth;
  $('.cart_slides').css({'left': 0, 'width': sliderwidth});
  numloop = numItems-1;
  last_slide = sliderwidth - swidth;
  i=0;
};

$('#slider_1').click( change_slider );
$('#slider_2').click( change_slider );
$('#slider_3').click( change_slider );
$('#slider_4').click( change_slider );
$('#slider_5').click( change_slider );
$('#slider_6').click( change_slider );

The basic idea is that we assign an anonymous function to a variable and then pass that to the various click handlers.  In order to make this work, we make use of the fact that when the function is called, it knows what object called it:  this.

  $(this).addClass('current_tab').siblings('.current_tab').removeClass('current_tab');
  $('#cart_'+$(this).attr('id')).show().siblings().hide();
  $('#cart_'+$(this).attr('id')).addClass('current').siblings('.current').removeClass('current');

In the first line, we can access our slider object directly.  The cart_slider is a little more complicated, so we rely on the fact that it just adds a prefix to the id of the slider object.  We get the id by calling $(this).attr('id') and use string concatenation to form the id of the cart_slider.

Note:  I haven't tried to run this.  Apologies in advance if I missed the syntax somewhere.


HackerRank: ACM ICPC Team I made a solution to this problem from HackerRank :

  You are given a list of N people who are attending ACM-ICPC World
  Finals. Each of them are either well versed in a topic or they are
  not. Find out the maximum number of topics a 2-person team can know.
  And also find out how many teams can know that maximum number of
  topics.

  Note Suppose a, b, and c are three different people, then (a,b) and
  (b,c) are counted as two different teams.

  Input Format

  The first line contains two integers, N and M, separated by a single
  space, where N represents the number of people, and M represents the
  number of topics. N lines follow. Each line contains a binary string
  of length M. If the ith line's jth character is 1, then the ith person
  knows the jth topic; otherwise, he doesn't know the topic.

My code is considered too slow (I think I am allowed 10 seconds, the code below takes more than 20 seconds when N and M are both 500):

import random
import itertools
import time

# n number of people
# m number of topics

n = 500
m = 500

"""
binary_string(n) and random_list(n_people, n_topic) just help
to generate test cases, irrelevant otherwise.
"""

def binary_string(n):
    my_string = "".join(str(random.randint(0, 1)) for _ in range(n))
    return my_string

def random_list(n_people, n_topic):
    my_list = [binary_string(n_topic) for _ in range(n_people)]
    return my_list

"""
the essential part is item_value(e1, e2)
and find_couples(comb_list)
"""

def item_value(e1, e2):
    c = bin(int(e1, 2) | int(e2, 2))
    return sum(int(i) for i in c[2:])

def find_couples(comb_list):
    max_value = 0
    counter = 0

    for pair in itertools.combinations(comb_list, 2):
        value = item_value(pair[0], pair[1])
        if value == max_value:
            counter += 1
        elif value > max_value:
            max_value = value
            counter = 1

    print(max_value)
    print(counter)
    return

topic = random_list(n, m)

print(topic)

start = time.time()
find_couples(topic)
end = time.time()
print(end - start)

In the context of the above algorithm, in what ways can I make it more efficient?
Is there a better algorithm?

You're spending a lot of time in item_value converting numbers to a binary string back to an int to get the sum. It would be a lot more expedient to just use str.count to get the number of 1s in the string. That saves you a lot of type conversions as well as a call to sum with a generator.

def item_value(e1, e2):
    c = bin(int(e1, 2) | int(e2, 2))
    return c[2:].count('1')

My results from this caused a reduction from 58.9s to 0.91s.


Random value excluding both limits The javascript method Math.random() returns a (pseudo-)random float between 0 (inclusive) and 1 (exclusive).

In some use cases, I need both limits to be excluded, a random float between 0 (exclusive) and 1 (exclusive).

This is what I currently use to generate said random float:

do {
    var random = Math.random();
} while(random === 0);

But I have a feeling that the (possibly) multiple draws from the random pool somehow mess up the distribution of my random floats.

Is there any downside to using this snippet and if so, how could I generate random floats while excluding both the lower and the upper limits?

Too long for a comment:
Please do not use the approach given in @Yann's answer:

var random = Math.random() * (1 - Number.MIN_VALUE) + Number.MIN_VALUE;

The reason is that Number.MIN_VALUE == 5E-324. As numbers in javascript are doubles with 52 bit mantissa and the largest number smaller than 1.0 that can be represented is somewhere around 1.0 - 2^(-52). The expression (1 - 5e-324)  is not representable and will truncate to exactly 1.0.
The addition of Number.MIN_VALUE to the random value will also truncate for large parts of the number range, skewing your distribution.
The correct (and fastest) way to do this is by using the original code in OP's question (as also pointed out by @200_success's answer).
Addendum:
Even if we let: almost_one be the largest value smaller than 1.0 and then did:
var random = Math.random()*almost_one + Number.MIN_VALUE;

then the value would still be wrong.
To achieve a uniform distribution Math.random() must only return values with the same exponent (i.e. only the mantissa differs). Otherwise you'd have many more numbers between [0, 0.01]  than you would between [0.01, 1.0].
So for any return value of Math.random() besides 0.0 the addition of Number.MIN_VALUE will truncate. However if Math.random() returns 0.0 you will get the smallest value larger than 0.0 that is representable (5e-324) as your result. But then there will be a huge gap to the next number that you can get as a result, so your distribution will not be uniform any more.
The following is also wrong:
var random = Math.random()*almost_one + (1.0 - almost_one);

This can easily be seen by assuming that Math.random() returns its maximal value which is almost_one, then you would have:
var random = almost_one*almost_one + (1.0 - almost_one);

which is not equal to almost_one (it's close, but not correct).