Simple math facts game I've made a simple game for testing math facts (think practice for elementary school-age kids). It displays a random addition, subtraction, multiplication, or division problem, and feedback on if the right answer was entered.

For now it works interacts with the user via prompt(), but I plan to add a GUI later. You can exit out of the infinite prompt() loop by clicking cancel.

I'd like feedback on how to improve the code, particularly if there's a better way of doing the getCorrectAns() function, and if I'm using best practices.

var maxNum = 12,
  stillPlaying = true,
  ans, isCorrect = '',
  correctAns = '',
  sign, num1 = 0,
  num2 = 0,
  prob, correctAns;

function randInt(max) {
  return Math.floor(Math.random() * max);
}
function getCorrectAns(n1, sign, n2) {
  switch (sign) {
    case '+':
      return n1 + n2;
    case '\u2212':
      return n1 - n2;
    case '\xD7':
      return n1 * n2;
    case '\xF7':
      return n1 / n2;
    default:
      return false;
  }
}
while (stillPlaying) {
  sign = ['+', '\u2212', '\xD7', '\xF7'][randInt(4)];
  num1 = randInt(maxNum + 1);
  num2 = randInt(maxNum + 1);
  if (sign == '\u2212') num1 += num2;
  if (sign == '\xF7') {
    num2 = randInt(maxNum) + 1;
    num1 *= num2;
  }
  prob = num1 + ' ' + sign + ' ' + num2;
  ans = prompt(isCorrect + prob);
  if (!ans) stillPlaying = false;
  correctAns = getCorrectAns(num1, sign, num2);
  isCorrect = (correctAns == ans) ? 'Correct! ' : 'Incorrect, ' + prob + ' = ' + correctAns + '.\n';
}

From a once over:

You are using lowerCamelCase, that is good
Variables like ans and prob deserve to be written out to answer and problem
You are using functions, but I think it can be split up a bit more from a best practices. I would go for something like this:

while( stillPlaying ){
  problem = generateNewProblem();
  answer = getAnswer( problem );
  processAnswer( answer );
}

['+', '\u2212', '\xD7', '\xF7'][randInt(4)]; I wonder why you could not simply use ['+','-','*','/']. Regardless you should have 4 constants for those values since you use those values repeatedly in other places

var plusSign = '+',
  minusSign = '-',
  timesSign = '*,
  divideSign = '/',
  signs = [plusSign , minusSign, timesSign, divideSign];

  if (sign == divideSign) num1 *= num2;

This seems funny: num1 = randInt(maxNum + 1);, if you were trying to avoid zero, you really need num1 = randInt(maxNum) + 1;
Avoid dropping the curly braces and newlines, even when if statements are short like here: if (sign == '\u2212') num1 += num2;

Here's my attempt for fun and giggles.

var maxNum = 12,
    signWidt = 12,
    question = document.getElementById( 'question' ),
    input = document.getElementById( 'input' ),
    errorMessage = document.getElementById( 'errorMessage' ),
    greatMessage = document.getElementById( 'greatMessage' ),
    answer;
//Some of these signs are closer to what children use in school
var plusSign = '+',
    minusSign = '\u2212',
    timesSign = '\u2212',
    divideSign = '\xF7',
    signs = [plusSign , minusSign, timesSign, divideSign];

function randInt(max) {
  return Math.floor(Math.random() * max);
}

function calculateAnswer(n1, sign, n2) {
  switch (sign) {
    case plusSign:
      return n1 + n2;
    case minusSign:
      return n1 - n2;
    case timesSign:
      return n1 * n2;
    case divideSign:
      return n1 / n2;
    default:
      return false;
  }
}

function displayQuestion( s ){
  //We know the question will never surpass 12 chars..
  var extraneousSpace = 12 - s.length, spaceLeft, spaceRight;
  //Compare last bit to 1, if that bit is 1, the number is odd
  //So we subtract 1 before we evenly divide by 2 and 1 space on the right
  if( extraneousSpace & 1 ){
    extraneousSpace--;
    spaceLeft = extraneousSpace / 2;
    spaceRight = spaceLeft + 1;
  } else {
    spaceLeft = spaceRight = extraneousSpace / 2;
  }
  //Mildly evil re-use of variable and magical space repeating routine
  //http://stackoverflow.com/questions/202605/repeat-string-javascript
  spaceLeft = new Array( spaceLeft + 1 ).join( ' ' );
  spaceRight = new Array( spaceRight + 1 ).join( ' ' );
  question.innerText = spaceLeft + s + spaceRight;
}

function generateQuestion(){
  var sign = signs[randInt(4)],
      num1 = randInt(maxNum + 1),
      num2 = randInt(maxNum + 1);

  if (sign == minusSign){
    num1 += num2;
  }
  if (sign == divideSign) {
    num2 = randInt(maxNum) + 1;
    num1 *= num2;
  }
  answer = calculateAnswer( num1 , sign, num2 );
  displayQuestion( num1 + ' ' + sign + ' ' + num2 )
}

generateQuestion();
input.focus();

input.addEventListener( 'keypress' , function(e){
  console.log( this );
  if( e.which == 13 || e.keyCode == 13 ){
    if( input.value != answer ){
      errorMessage.innerText = 'Wrong, ' + question.innerText.trim() + ' = ' + answer;
      greatMessage.innerText = '';
    }else {
      errorMessage.innerText = '';
      greatMessage.innerText = 'That is right, here\'s another question';
    }
    input.value = '';
    generateQuestion();
  }
});
#errorMessage { color: red }
#greatMessage { color: green }

|￣￣￣￣￣￣ |
|   What is  |
|            |
|      ?     |
| ＿＿＿＿＿__|
(\__/) ||
(•ㅅ•) ||
/ 　 づ

<input type='text' id='input'>


Autocomplete and CancellationToken In the mobile application I have a search page. As user types search keywords I display autocomplete entries.
On each keystroke I initiate autocomplete lookup, and I am using CancellationToken to cancel lookups if user keeps typing.

I am new to using CancellationTokens, so please review my code and recommend changes as needed.

// ViewModel code is below.

// Search service is injected in ctor.
private readonly ISearchService _searchService;
private CancellationTokenSource _cancellationTokenSource;

// From the UI accept user input and get autocomplete search results.
public async Task GetAutocompleteSearchResultsAsync(string searchText)
{
    try
    {
        if (_cancellationTokenSource != null)
        {
            _cancellationTokenSource.Cancel();
            _cancellationTokenSource.Dispose();
            _cancellationTokenSource = new CancellationTokenSource();
        }
        else
        {
            _cancellationTokenSource = new CancellationTokenSource();
        }

        var autocomplete = await _searchService.GetAutocomplete(searchText, _cancellationTokenSource.Token);
        // Assign result to property to display results in UI.
        AutocompleteSearchResults = autocomplete;
    }
    finally
    {
        if (_cancellationTokenSource != null)
        {
            _cancellationTokenSource.Dispose();
            _cancellationTokenSource = null;
        }
    }
}

And inside the ISearchService implementation I have the following code.

public async Task<ObservableCollection<AutocompleteSearchResult>> GetAutocomplete(
string input, CancellationToken cancellationToken)
{
    try
    {
        string endpoint = $"/autocomplete/{input}";
        // _client is HttpClient instance. It's initialized in ctor.
        using (var response = await _client.GetAsync(endpoint, cancellationToken))
        {
            if (cancellationToken.IsCancellationRequested) return null;

            if (response.IsSuccessStatusCode)
            {
                string json = await response.Content.ReadAsStringAsync();
                var data = await Task.Run(() =>
                        JsonConvert.DeserializeObject<IEnumerable<AutocompleteSearchResult>>(json),
                    cancellationToken);
                return data.ToObservableCollection();
            }
        }

        return null;
    }
    catch (Exception)
    {
        return null;
    }
}

Just a small suggestion.

Typical typing rate is 40 WPM (with keyboard). It means more or less one character every 300ms. With mobile phone (and a good virtual keyboard) rate is typically slower (around 20/30 WPM).

Assuming:

You can start a request in few milliseconds.
User will have a useful result after few typed characters.

It means that for each user and for each typed word you will start few HTTP connections with your server (in the best case where user will pick something from the autocomplete). Multiply for all the searched words for all your user base and you will see that you add a useless load to your server (and you consume mobile bandwidth...)

Do not start lookup while typing but AFTER user stopped typing for a short period, let's say 300 ms for mobile devices but you may tune this value with some usage statistics. Network latency, server response time and UX fluidity need to be considered to find a trade-off.

Note that this may be done also with a simple Task.Delay(300) inside GetAutocomplete(), before calling _client.GetAsync(). It has the advantage to keep GetAutocompleteSearchResultsAsync() unchanged.

await Task.Delay(DebouncingDelayInMilliseconds, cancellationToken);


Extracting small word parts (quadgrams) from text using R I would like to extracts small word parts (quadgrams) from text. Example:

hello world

results in:

_hel, hell, ello, llo_, lo_w ...

This my basic attempt thus far:

text <- "hello world"

number_of_characters <- nchar(text)
quad_gram_list <- ""[-1]

for (i in 1:number_of_characters) {

    end <- i + 3
    if (end > number_of_characters) {
        end <- number_of_characters
    }

    temp <- substring(text, i, end)

    if (nchar(temp) == 4) {
        quad_gram_list <- append(quad_gram_list, temp)
    }
}

Any improvement suggestions would be very much appreciated.

Your loop approach works but seems a bit convoluted. For instance you could rewrite the for loop this way:

for (i in 1:number_of_characters) {

  end <- i + 3

  if (end <= number_of_characters) {
    temp <- substring(text, i, end)
    quad_gram_list <- append(quad_gram_list, temp)
  }
}

But anyway it's generally better to avoid loops in R.

Also note that:

you should use character(0) instead of ""[-1]
: is to be avoided because of undesired behaviour on edge cases. It's better to use seq_len().

Here are two alternative solutions:

1) Using base R:

unlist(lapply(seq_len(nchar(text) - 3), function(i) substring(text, i, i + 3)))
# [1] "hell" "ello" "llo " "lo w" "o wo" " wor" "worl" "orld"

2) Using tokenizers:

library(tokenizers)

tokenize_character_shingles(
  text,
  n = 4,
  strip_non_alphanum = FALSE,
  simplify = TRUE
)
# [1] "hell" "ello" "llo " "lo w" "o wo" " wor" "worl" "orld"


Expiring JWT tokens in Flask I've been using flask-jwt-extended for my application and one of the problems I had was logging a session out and making sure the token is not usable anymore.

I've based my solution on the Blacklist and Token Revoking documentation page with a custom RevokedToken model:

from flask_sqlalchemy import SQLAlchemy

db = SQLAlchemy()

class RevokedToken(db.Model):
    """
    Model is used as a storage to keep invalid/revoked tokens.
    Currently used for log out functionality.
    """
    __tablename__ = 'revoked_tokens'

    id = db.Column(db.Integer, primary_key=True)
    jti = db.Column(db.String(120))

    @classmethod
    def is_jti_blacklisted(cls, jti):
        query = cls.query.filter_by(jti=jti).first()
        return bool(query)

Logout resource:

class LogoutResource(Resource):
    @jwt_required
    def post(self):
        jti = get_raw_jwt()['jti']

        # invalidate access token
        revoked_token = RevokedToken(jti=jti)
        session.add(revoked_token)
        session.commit()

        return {}, 200

And the token_in_blacklist_loader() jwt function:

from flask_jwt_extended import JWTManager

jwt = JWTManager(app)

@jwt.token_in_blacklist_loader
def check_if_token_in_blacklist(decrypted_token):
    jti = decrypted_token['jti']
    return models.RevokedToken.is_jti_blacklisted(jti)

This looks straightforward enough, but, as we are talking about authentication, I thought I would ask if anyone sees any flaws or potential improvements to this approach?

As far as I can tell,

This looks like a pretty solid implementation of blacklisting.
I can't see any obvious code mistakes.

I you are worried about security it can be wise to check the corresponding OWASP documentation

This is for Java, but the security considerations should be similar


Replacing all occurrences of a substring in a string without using regex Problem Statement:

  Given two strings, base and remove, return a version of the base
  string where all instances of the remove string have been removed (not
  case sensitive). You may assume that the remove string is length 1 or
  more. Remove only non-overlapping instances, so with "xxx" removing
  "xx" leaves "x".

withoutString("Hello there", "llo") → "He there"

withoutString("Hello there", "e") → "Hllo thr"

withoutString("Hello there", "x") → "Hello there"

Please feel free to review the code below:

public String withoutString(String base, String remove) {
   final int rLen = remove.length();
   final int bLen = base.length();
   String op      = "";

   for(int i = 0; i < bLen;)
   {
        if( !(i + rLen > bLen) && base.substring(i, i + rLen).equalsIgnoreCase(remove) )
        {
            i += rLen;
            continue;
        }
        op += base.substring(i, i + 1);
        i++;
   }

   return op;

}

Using a StringBuilder should always be your preferred choice when doing String manipulation. It does not always make sense, but almost always.

Additionally, indexOf takes a 'from' location, and it makes the whole thing easier.... Convert both inputs to lower-case, then do the matching using the lower-case versions, but the substrings using the unknown-case.

The complicated assign-in-where-condition logic makes the loop conditions easier, but you do need to understand what this does:

while ((find = lcbase.indexOf(lcremove, previous)) >= 0) {

it searches for thelower-case remove from the previous position, saves the value to find, and checks that the result is 'positive'.... all in the condition of a while loop.

The full code is:

public static String withoutString(String base, String remove) {

    final String lcbase = base.toLowerCase();
    final String lcremove = remove.toLowerCase();
    int find = 0;
    int previous = 0;
    StringBuilder result = new StringBuilder(base.length());
    while ((find = lcbase.indexOf(lcremove, previous)) >= 0) {
        result.append(base.substring(previous, find));
        previous = find + remove.length();
    }
    result.append(base.substring(previous));
    return result.toString();

}

See this running inideone


MySQL PDO query to search by name and role I'm new to this PDO PHP and I have coded this spaghetti style. I hope you can give me idea how to improve using prepared statements.  I'm not a lazy person, but I really need your help to improve my coding style.

            try {
        $select = "SELECT users.id, users.users_id, username, firstname, lastname, password, position";
        $from = " FROM users
                  INNER JOIN users_role
                  ON users.id=users_role.users_id
                  INNER JOIN role
                  ON users_role.users_role=role.id";
        $where = " WHERE";
        $placeholders = array();
        if ($category !=''){
            if ($category == 'admin'){
                $where .= " position='admin'";
                if ($character_match !=''){
                    $where .= " AND ";
                }

            }
            else if($category == 'instructor'){
                $where .= " position='Instructor'";
                if ($character_match !=''){
                    $where .= " AND ";
                }
            }
            else if ($category=='student'){
                $where .= " position='student'";
                if ($character_match !=''){
                    $where .= " AND ";
                }
            }
            elseif ($category == 'Unverified'){
                $where .= " position='Unverified'";
            }
            else if ($category == 'view_all') {
                $where = "";
                if ($character_match != ''){
                    $where = " WHERE ";
                }
            }
            if($character_match !=''){
                $where .= "(username LIKE '" . $character_match . "%'"
                        . " OR firstname LIKE '"  . $character_match . "%'"
                        . " OR lastname LIKE '"  . $character_match . "%')";
                $placeholders[] = $character_match;
            }
            $where .= " ORDER BY position ASC";

            $sql = $select . $from . $where;
            $q = $this->db->prepare($sql);
            $q->execute($placeholders);
        }
    }
    catch(PDOException $e){
            $e->getMessage();
        }
       $rows = array();

       while($row = $q->fetch(PDO:: FETCH_ASSOC)){
           $rows[] = $row;
       }

There's different things about your code, the most important one is to start using PDO tools to facilitate our work. Use named parameters, and dissociate your query construct from setting correct values:

if the only thing you're going to catch is echo the error message, there's absolutely no reason to do it. setting $this->db->setAttribute(PDO::ATTR_ERRMODE,PDO::ERRMODE_EXCEPTION); will react in the exact same way.
for the LIKE portion of the code: named parameters must contain a full litteral string (or integer, or whatever) but you have to put wildcards % IN the string for the LIKE to work. exemple: $values[':character_match'] = '%value%'

$sql = 'SELECT users.id, users.users_id, username, firstname, lastname, password, position
            FROM users
            INNER JOIN users_role ON users.id=users_role.users_id
            INNER JOIN role ON users_role.users_role=role.id ';
if ($category != 'view_all') {
    $sql .= 'WHERE position=:position ';
}
if ($character_match != '') {
    $sql .= ($category == 'view_all' ? ' WHERE ' : ' AND ');
    $sql .= '(username LIKE :character_match OR firstname LIKE :character_match OR lastname LIKE :character_match)';
    $values[':character_match'] = $character_match;
}
$sql .= ' ORDER BY position';

switch ($category) {
    case 'admin':
        $values[':position'] = 'admin';
        break;
    case 'instructor':
        $values[':position'] = 'Instructor';
        break;
    case 'student':
        $values[':position'] = 'student';
        break;
    case 'Unverified':
        $values[':position'] = 'Unverified';
        break;
}

$q = $this->db->prepare($sql);
$q->execute($placeholders);

$rows = array();
while ($row = $q->fetch(PDO:: FETCH_ASSOC)) {
    $rows[] = $row;
}


Place arithmetic signs in correct order to get the number Given a list of numbers, place signs + or - in order to get the
  required number.

  If it's possible then return true, otherwise return false

  [1, 2, 3, 4, 5, 7]; 12 -> true, because 1 + 2 + 3 + 4 - 5 + 7 = 12

  [5, 3]; 7 -> false, because 5 + 3 != 7 and 5 - 3 != 7

I came up with the following brute force solution:

public static boolean trueOrNot(int number, List<Integer> numbers) {
        boolean found = false;
        int i = 0;
        while (!found && i < (1 << numbers.size())) {
            int tmpResult = 0;
            for (int j = 0; j < numbers.size(); j++) {
                int num = numbers.get(j);
                if ((i & (1 << j)) > 0) {
                    num = -num;
                }
                tmpResult += num;
            }
            found = tmpResult == number;
            i++;
        }
        return found;
}

Basically, I took for consideration binary representations of numbers 0000, 0001, 0010...2^n and iterate over every bit, thus getting all possible combinations. 0 is a + sign and 1 is - sign.
Then I calculate temporal result and once it is equal to the sought number I break out from loop.

Time complexity: O(2n)
Space complexity: O(1)

I just wondering is there any more effective solution with the help of Dynamic Programming, for example?

Feedback for the current solution?

There is one technique, of transforming a problem to a more comfortable one.
Here one could do it a couple of times:

First the original problem sum = term[0] ± term[1] ± ....

static boolean additive(int sum, int... terms) {
    if (terms.length == 0) {
        return sum == 0;
    }
    if (terms.length == 1) {
        return sum == terms[0];
    }
    int sum2 = sum - terms[0];
    int[] terms2 = IntStream.of(terms).skip(1).map(Math::abs).sorted().toArray();
    // Also an occasion to use Arrays.copyOfRange(terms, 1, terms.length);
    return additive2(sum2, terms2);
}

You can subtract the first term from the sum, and then have the possibly more regular problem sum == ∑i ±term[i]. All terms positive, and sorted.

/**
 * @param sum to be formed by the sum of every term * ± 1.
 * @param terms, all absolute, and sorted.
 */
static boolean additive2(int sum, int... terms) {
    int termsSum = IntStream.of(terms).sum();
    if (sum > termsSum) {
        return false;
    }
    int tooMuch = termsSum - sum;
    // Split the terms in added and subtracted ones.
    // The sum of the subtracted ones * 2 == tooMuch.
    if (tooMuch % 2 != 0) {
        return false;
    }
    if (tooMuch == 0) { // All positive terms.
        return true;
    }
    // Find subtracted terms:
    int subtractedSum = tooMuch / 2;
    return findSubtracted(subtractedSum, terms.length - 1, terms);
}

Then math properties appear: the difference between the sum of absolute terms and the requested sum must be twice the sum of the negative/subtracted terms.
An odd difference means failure. Nice.

Now one only needs to recursively find the subtracted terms, starting with the largest terms. O(2n) but with some cuts: if the term is too large, in the recursion sum < 0, and done: sum is decreased as fast as possible.

static boolean findSubtracted(int sum, int i, int[] terms) {
    if (sum <= 0) {
        return sum == 0;
    }
    if (i < 0) {
        return false;
    }
    return findSubtracted(sum - terms[i], i - 1, terms)
            || findSubtracted(sum, i - 1, terms);
}

public static void main(String[] args) throws XMLStreamException {
    System.out.println(additive(12, 1, 2, 3, 4, 5, 7)); // true, because 1 + 2 + 3 + 4 - 5 + 7 = 12
    System.out.println(additive(7, 5, 3)); // false, because neither 5 + 3 != 7 or 5 - 3 != 7
}

Take

Problem: 1 + 2 + 3 + 4 - 5 + 7 = 12
1 + 2 + 3 + 4 + 5 + 7 = 22; tooMuch = 10, subtractedSum = 5
-7? - false
-5? - true

This might not be the best solution, but it exposes some math intelligence.
For instance 1 + 2 + 3 + 4 - 5 + 7 = 13 will fail fast.

Dynamic programming, operations research and such are indeed worthwile in real life problems, especially if approximations / near solutions count too. Traffic optimisation and such.

A remark on iterating upto 2n with bit tests: nice and I did it myself on occasion. However here my code accomplishes the same with 2 recursive calls in the function. Which certainly is less effective, but more readable. Do this (micro-)optimisation last, as it hampers mental flexibility on the mathematical properties themselves. I think.


Implementation of Set data type in Haskell I'm an advanced beginner Haskell programmer. I implemented a set data type in Haskell and I wanted to find out if I made correct choices when writing it. Would there be a way to improve some of the algorithms like intersection etc.

data SET a = E | Add(a, SET a) deriving (Eq, Ord, Read, Show)

--test data

setx = listToSet[1,2,3,4]

setx2 = listToSet[3,4,2,1]

sety = listToSet[3,4,5,6]

setys = listToSet[3,4]

listToSet :: [a] -> SET a
listToSet [] = E
listToSet (s:x) = Add(s, listToSet x)

inS :: (Eq a) => a -> SET a -> Bool
inS a E = False
inS a (Add(value, set)) = if (value == a)
                        then True
                        else (inS a set)

nullSET :: (Eq a) => SET a -> Bool
nullSET s = s == E

headSET :: SET a -> a
headSET (Add(v, s)) = v

tailSET :: SET a -> SET a
tailSET (Add(v, s)) = s

addToSet :: (Eq a) => a -> SET a -> SET a
addToSet a (Add(value, set)) = if (inS a (Add(value, set)) == False)
                               then Add(a, Add(value, set))
                               else (Add(value, set))

setFilter :: (Eq a) => (a -> Bool) -> SET a -> SET a
setFilter f s
    | nullSET s    = E
    | f h == False = setFilter f t
    | otherwise    = Add(h, setFilter f t)
        where
            h = headSET s
            t = tailSET s

intersection :: (Eq a) => SET a -> SET a -> SET a
intersection s1 s2 = setFilter (\x -> inS x s1) s2

union :: (Eq a) => SET a -> SET a -> SET a
union set E = set
union set (Add(value2, set2)) = if (inS value2 set == False)
                                then Add(value2, (union set set2))
                                else union set set2

subSet :: (Eq a) => SET a -> SET a -> Bool
subSet set1 set2
    | set1 == E  = True
    | inS h set2 = subSet t set2
    | otherwise  = False
    where
        h = headSET set1
        t = tailSET set1

setEq :: (Eq a) => SET a -> SET a -> Bool
setEq s1 s2 = (subSet s1 s2) == (subSet s2 s1)

module Set where

Self-contained reusable chunks of code should be organized into modules.

data Set a = Empty | Set a (Set a)
  deriving (Read, Show)

All-caps types are not a thing, just Set is fine. Single letter constructors are uncommon too, so instead of E use a meaningful name like Empty. Constructors can take multiple arguments, so don't use tuples where you don't have to. I'll also point out that this data type you've defined is isomorphic to [a]. Besides the magic square brackets, Haskell lists are a data type like any other, i.e. data [a] = [] |  (:) a [a]. Notice the similarity? Is this a situation where you need a new datatype or might it make sense to use a newtype?

-- Test data
setx, setx2, sety, setys :: Set Int
setx = listToSet [1,2,3,4]

All top-level bindings should be given a type signature, get into the habit now and you'll save yourself the frustration of a lot of cryptic misplaced error messages later. These names are all over the place too, they should be more consistent, e.g. exampleSetA, exampleSetB, exampleSetC. Don't crowd together functions and their arguments, leave some space and let it breathe.

fromList :: [a] -> Set a
fromList []     = Empty
fromList (x:xs) = Set x (fromList xs)

fromList is a common name for converting lists to arbitrary datatypes. When it conflicts with other names from modules you have imported, you disambiguate with qualified imports, not a menagerie of globally unique names. (x:xs) is a common naming pattern for lists, that is, some variable x, y, e, or whatever, and then the plural form (xs, ys, es... it's a pun, get it?). Again, space out function application, tuples are a datatype in Haskell, not a method of passing arguments.

member :: (Eq a) => a -> Set a -> Bool
member _ Empty                  = False
member a (Set x xs) | a == x    = True
                    | otherwise = member a xs

A common function name for testing set membership is member. When you don't use an argument name in the body of a function, replace it with _. If you compile with -Wall (as you should), GHC will warn you about unused bindings. That is, names that you defined in a function body but didn't use. Often this can help you to notice errors before you ever get to running your code! Use guards (e.g., | a == x) instead of if statements where you can. Guards are like a multiway if, when you have multiple cases it becomes much clearer than chaining ifs, get into the habit now, there's no reason not to.

null :: Set a -> Bool
null Empty = True
null _     = False

Use pattern matching where you can, it's more explicit than equality testing, often faster, and helps you avoid unnecessary constraints. Notice how my version lacks the (Eq a) constraint?

-- Mourn headSet and tailSet here

These functions are meaningless on Sets. Being unordered collections, Sets do not have meaningful heads or tails, so you should not leak implementation details by exposing them.

insert :: (Eq a) => a -> Set a -> Set a
filter :: (a -> Bool) -> Set a -> Set a
intersection :: (Eq a) => Set a -> Set a -> Set a
union :: (Eq a) => Set a -> Set a -> Set a
isSubset :: (Eq a) => Set a -> Set a -> Bool

Around now you're starting to feel the pain of your choice of Set representation. Implementations are slow and unwieldy. For instance, your version of intersection is an \$O(n^2)\$ operation. You can improve running times by utilizing any of a number of standard algorithms, try using balanced binary trees to represent your sets instead of linked lists.

instance (Eq a) => Eq (Set a) where

Instead of making up another name, write your own instances for standard functions like equality. If the derived instance doesn't do what you want, don't use it, don't keep it around (because you'll be unable to keep it from being exported to users of your library), and don't let it clutter up good namespace.


Increase performance of Boyer Moore Here is my Boyer Moore code in Python:

def BoyerMoore(stringy, substring):
    if stringy == substring:
        return 0
    ASCIIcharset = [-1]*256
    for x in xrange(len(stringy)):
        ASCIIcharset[ord(stringy[x])] = x

    stringLen = len(stringy)
    substringLen = len(substring)
    for i in xrange(stringLen - substringLen):
        skip = 0
        for j in xrange(substringLen - 1 , 0, -1):
            if stringy[i + j] is not substring[j]:
                skip = max(1, j - ASCIIcharset[ord(stringy[i + j])])
                i += skip
                break
        if skip == 0:
            return i
    return -1

I was wondering what are some small tweaks here and there that I can do to increase the efficiency. Redesigning this code is fine too.

The following examples show that there is something wrong in your code :

print(BoyerMoore('azertyuiop', 'zertyuio' ))  #  1 - fair enough
print(BoyerMoore( 'zertyuio' , 'azertyuiop')) # -1 - so far so good
print(BoyerMoore('azertyuiop', 'azertyuiop')) #  0 - ok
print(BoyerMoore('abcdefgiop', 'iop'))        # -1 - looks wrong to me
print(BoyerMoore('iop'       , 'abcdefgiop')) # -1 - ok (just wanted to check that arguments were in the right order)


Binary Search in C++17 Question

Any way I can optimize this further with C++11 or C++17 features?

Would also like feedback on my variable naming, memory management, edge case handling (in this someone calling my function with an nullptr or int overflow with my rearranged equation to calculate the mid), and coding style. If there are other data structures I can use to implement this instead of basic arrays and raw pointers I'd like some feedback there too.

For my return type on the binary_search function, does it matter if I return a bool versus an int?

Code

#include <cassert>
#include <iostream>

bool binary_search(int* data, int num_elements, int target)
{
    int low = 0;
    int high = num_elements - 1;
    int mid;

    if(data == nullptr) { throw std::exception(); }

    while(low <= high) {
        mid = low + (high - low) / 2;
        if(data[mid] == target) {
            return 1;
        } else if(data[mid] > target) {
            high = mid - 1;
        } else {
            low = mid + 1;
        }
    }

    return 0;
}

int main()
{
    int num_elements = 6;

    int data[] = { 5, 8, 10, 15, 26, 30 };
    int target[] = { 5, 4, 12, 15, 35, 30 };
    int expected[] = { 1, 0, 0, 1, 0, 1 };

    for(int i=0; i < num_elements; ++i) {
        try {
            assert(expected[i] == binary_search(data, num_elements, target[i]));
            std::cout << expected[i] << " returned for search on " << target[i] << '\n';
        } catch(std::exception& e) {
            std::cout << "Exception " << e.what() << '\n';
        }
    }

    return 0;
}

An idiomatic approach

  to implement this instead of basic arrays and raw pointers

is to use iterators.
Returning bool is dubious. The situation where I only want to know if the element is present or not is very rare. Typically I want to know where exactly the element is (or, if absent, where it should be inserted to keep the collection sorted). Your function does compute this information, but immediately throws it away. Return an iterator.
All that said, consider the signature

template<typename It, typename T>
It binary_search(It first, It last, const T& target)

It is now suspiciously similar to the standard library's std::lower_bound. Follow the link for further insight and inspiration.


Class template, custom container So I wrote this class template for a custom container. Basically, it stores elements and their occurences. When an element is pushed into the container, if it already exists, all it does is increment its counter.

#pragma once

#include <map>
#include <string>

//
// Template class for a tocc, or Thing Occurence Counter Container
//
template <class T>
class tocc
{
public:
    tocc();

    //
    // Increments the count of that item or creates a new pair if it doesn't exist
    //
    void push(const T& item)
    {
        // If no the item isn't found, create new pair with a count of 1
        if(_tocc.find(item) == _tocc.end())
            _tocc.insert(std::pair<T, long int>(item, 1));
        else
            _tocc.at(item)++;
    }

    //
    // Decrements the count of that item and erases it if the count reaches 0
    //
    void pop(const T& item)
    {
        // If the item is found
        if(_tocc.find(item) != _tocc.end())
            (_tocc.at(item) <= 1) ? _tocc.erase(item) : _tocc.at(item)--;

        // Do nothing if the item isn't found
    }

    //
    // Gets the count of a particular item
    // Returns the count if the item exists, 0 otherwise
    //
    long int getCount(const T& item)
    {
        if(_tocc.find(item) == _tocc.end())
            return 0;
        return _tocc.at(item);
    }

    //
    // Returns true if the item is present in the map, false otherwise
    //
    bool contains(const T& item)
    {
        return _tocc.find(item) != _tocc.end();
    }

private:
    std::map<T, long int> _tocc;
};

Here I put all the whole code in a single "file" for simplicity's sake, though I know it is best practice to separate files into headers and source files (which I normally do).

I'd like to know what can be improved about this implementation.

The rules of five/three/zero

Usually, the existence of a single constructor indicates that the others should get implemented too (or explicitly forbidden/deleted). This is called the rule of five (or three, depending on the standard). However, there is only one member in your class, _tocc, and it has well-defined constructors for all the usual cases.

Here, we should follow the rule of zero: declare and define no constructors at all, as the default constructors will do the right thing. See also CppCoreGuidelines C.20.

Use already known data

While there is no bug in your code, there are some optimization flaws. Let's have a look at tocc<int>::push, compiled via gcc 9.2 -S -O3 (CompilerExplorer; I had to [[gnu::noinline]] to keep the assembly sane).

tocc<int>::push(int const&):
    sub     rsp, 24
    mov     rdx, QWORD PTR [rdi+16]
    mov     ecx, DWORD PTR [rsi]
    test    rdx, rdx
    je      .L31
    lea     r8, [rdi+8]
    mov     rax, rdx
    mov     rsi, r8
    jmp     .L32
.L48:
    mov     rsi, rax
    mov     rax, QWORD PTR [rax+16]
    test    rax, rax
    je      .L33
.L32:
    cmp     DWORD PTR [rax+32], ecx
    jge     .L48
    mov     rax, QWORD PTR [rax+24]
    test    rax, rax
    jne     .L32
.L33:
    cmp     rsi, r8
    je      .L31
    cmp     DWORD PTR [rsi+32], ecx
    jg      .L31
    mov     rax, r8
    jmp     .L36
.L50:
    mov     rax, rdx
    mov     rdx, QWORD PTR [rdx+16]
    test    rdx, rdx
    je      .L49
.L36:
    cmp     ecx, DWORD PTR [rdx+32]
    jle     .L50
    mov     rdx, QWORD PTR [rdx+24]
    test    rdx, rdx
    jne     .L36
.L49:
    cmp     r8, rax
    je      .L41
    cmp     ecx, DWORD PTR [rax+32]
    jl      .L41
    add     QWORD PTR [rax+40], 1
    add     rsp, 24
    ret
.L31:
    mov     rsi, rsp
    mov     DWORD PTR [rsp], ecx
    mov     QWORD PTR [rsp+8], 1
    call    std::pair<std::_Rb_tree_iterator<std::pair<int const, long> >, bool> std::_Rb_tree<int, std::pair<int const, long>, std::_Select1st<std::pair<int const, long> >, std::less<int>, std::allocator<std::pair<int const, long> > >::_M_emplace_unique<std::pair<int, long> >(std::pair<int, long>&&)
    add     rsp, 24
    ret
.L41:
    mov     edi, OFFSET FLAT:.LC0
    call    std::__throw_out_of_range(char const*)

That's a lot of conditional jumps. 11 jumps depend on test or cmp. However, we only have a single comparison. What happens here?

Well, first of all, a call to map<T>::at isn't free. It always has some additional boundary check, which prevents us from undefined behaviour but exchanges this boon for some additional code and an potential exception.

However, if we're in the second branch in push, then we already know that there is an element! After all, we found it beforehand:

void push(const T& item)
{
    if(_tocc.find(item) == _tocc.end())
        _tocc.insert(std::pair<T, long int>(item, 1));
    else
        _tocc.at(item)++;  // find() did not return end()!
}

Instead of at, we should use the iterator for several reasons:

we already have the element at hand,
we're guaranteed to have an element, so the boundary check in at() is not necessary,
we don't need to search the element a second time and therefore stay \$\mathcal O(1)\$ instead of \$\mathcal O(\log n)\$

So let's use the iterator instead and let's replace std::pair<T, long int> with std::make_pair while we're at it:

void push(const T& item)
{
    const auto it = _tocc.find(item);
    if(it == _tocc.end()) {
        _tocc.insert(std::make_pair(item, 1));
    } else {
        it->second++;
    }
}

What's the new assembly?

tocc<int>::push(int const&):
    mov     rax, QWORD PTR [rdi+16]
    mov     edx, DWORD PTR [rsi]
    test    rax, rax
    je      .L31
    lea     rsi, [rdi+8]
    mov     rcx, rsi
    jmp     .L32
.L44:
    mov     rcx, rax
    mov     rax, QWORD PTR [rax+16]
    test    rax, rax
    je      .L33
.L32:
    cmp     DWORD PTR [rax+32], edx
    jge     .L44
    mov     rax, QWORD PTR [rax+24]
    test    rax, rax
    jne     .L32
.L33:
    cmp     rsi, rcx
    je      .L31
    cmp     DWORD PTR [rcx+32], edx
    jle     .L36
.L31:
    sub     rsp, 24
    lea     rsi, [rsp+8]
    mov     DWORD PTR [rsp+8], edx
    mov     DWORD PTR [rsp+12], 1
    call    std::pair<std::_Rb_tree_iterator<std::pair<int const, long> >, bool> std::_Rb_tree<int, std::pair<int const, long>, std::_Select1st<std::pair<int const, long> >, std::less<int>, std::allocator<std::pair<int const, long> > >::_M_emplace_unique<std::pair<int, int> >(std::pair<int, int>&&)
    add     rsp, 24
    ret
.L36:
    add     QWORD PTR [rcx+40], 1
    ret

Only 6 conditional jumps, only ~55% the original amount. However, keep in mind that the reduction of asm instructions was not the goal of this section. Instead, we re-used already known values and didn't repeat ourselves (only one find() call).

The same holds for pop()'s usage of map::erase(), which can also take an iterator instead of a Key, but that's left as an exercise.

Documentation and (internal) comments

Good job on the comments! However, keep in mind that Doxygen and other programs use special syntax to discern  documentation comments and implementation comments.

Naming

The sole member of our class almost has the same name as your class. This makes it somewhat confusing, as we use _tocc to actually implement tocc. Naming is hard, though, and I cannot come up with a nicer name; counter, key_counter or key_counter_map don't have the same ring to it, although the latter is the most descriptive variant.

Interface

At the moment, a user must know the name of all items in the tocc to check their count afterwards. An iterator interface would be tremendously helpful.

We could even reuse std::map::const_iterator, if you only want constant iteration:

using iterator_type = std::map<T, long int>::iterator_type;

iterator_type begin() { return _tocc.begin(); }
...


Enhance the performance of the source code in Java I need help to enhance the performance of java source code. The code take txt file that has name, amount and date in form (name) (amount) (date yyyy-mm-dd) separated by spaces. After reading the file the data will be (name) (amount) (date dd/MM/yyyy).

    public static void main(String[] args) throws IOException {

    //we have an lang.ArrayIndexOutOfBoundsException because there is no index 0 in args
    long start = System.currentTimeMillis();
    File inputFile;
    int read;
    char[] charBuffer = new char[1024];
    String token = "";
    int tokenIndex = 0;
    String name = null;
    int year = -1;
    int month = -1;
    int day = -1;
    String amount = null;

    try {
        inputFile = new File(args[0]);
        FileReader reader = new FileReader(inputFile);
        while((read = reader.read(charBuffer)) != -1) {
            for (int i = 0; i < read; ++i) {
                char c = charBuffer[i];
                if (c == '\r' || c == '\n') {
                    if (token.length() != 0) {
                        // read date
                        String date = token;
                        year = Integer.parseInt(date.substring(0, 4));
                        month = Integer.parseInt(date.substring(5, 7));
                        day = Integer.parseInt(date.substring(8, 10));
                        // flush line
                        System.out.print(name.toUpperCase());
                        System.out.print(" ");
                        System.out.print(amount.indexOf('.') == -1 ? true : false);
                        System.out.print(" ");
                        System.out.print(day < 10 ? "0" + day : day);
                        System.out.print("/");
                        System.out.print(month < 10 ? "0" + month : month);
                        System.out.print("/");
                        System.out.print(year);
                        System.out.println();
                        name = null;
                        year = -1;
                        month = -1;
                        day = -1;
                        amount = null;
                        token = "";
                        tokenIndex = 0;
                    }
                }
                else if (c == ' ') {
                    // flush token
                    if (tokenIndex == 0) {
                        // read name
                        name = token;
                        token = "";
                        tokenIndex++;
                    }
                    else if (tokenIndex == 1) {
                        // read amount
                        amount = token;
                        token = "";
                        tokenIndex++;
                    }
                }
                else token = token + c;
            }
        }
    } catch (ArrayIndexOutOfBoundsException e) {System.out.println (e);}
    System.out.println ("Time taken for String in if/else if :"+ (System.currentTimeMillis() - start));
}

I was thinking to separate the nested if-statements because I think its a bad OO code. Any ideas to enhance the code?

Exceptions used for control flow

inputFile = new File(args[0]);

Using exceptions for the control flow in your program is harmful for the speed of executing. When making a exception, much of the time is spent on constructing the stack trace.

if (args.length == 0) {
    System.out.println("Usage: java -jar program.jar <filename>");
    return;
}

Using unbuffered IO

FileReader reader = new FileReader(inputFile);
while((read = reader.read(charBuffer)) != -1) {

You are using unbuffered IO, this means for every call to the read() method, a call will be dispatched to the underlying file system to read 1 byte. Wrapping the FileReader in a BufferedReader increases the performance, and gives you access to prebuild methods such as nextLine

BufferedReader reader = new BufferedReader(new FileReader(inputFile));

You are trying to reimplement the wheel with your loop

while((read = reader.read(charBuffer)) != -1) {
        for (int i = 0; i < read; ++i) {
            char c = charBuffer[i];
            if (c == '\r' || c == '\n') {
                if (token.length() != 0) {
                    // read date
 ...
                }
            }
            else if (c == ' ') {
                // flush token
                if (tokenIndex == 0) {
                    // read name
                    name = token;
                    token = "";
                    tokenIndex++;
                }
                else if (tokenIndex == 1) {
                    // read amount
                    amount = token;
                    token = "";
                    tokenIndex++;
                }
            }
            else token = token + c;
        }

The above code is pretty inefficient duo to the large amount of stringbuilder and string object created, a better way is reading a whole line using BufferedReader.readLine() and splitting that on spaces:

String line;
while((line = reader.readLine()) != null) {
    int firstSpace = line.indexOf(' ');
    int secondSpace = line.indexOf(' ', firstSpace + 1);

    String name = line.subString(0, firstSpace);
    String amount = line.subString(firstSpace + 1, secondSpace);
    String date = line.subString(secondSpace + 1, line.length());

    year = Integer.parseInt(date.substring(0, 4));
    ...
}

This also decreases the size of the actual code

Don't declare variables until you actually need them

File inputFile;
...
int read;
String token = "";
...
String name = null;
int year = -1;
int month = -1;
int day = -1;
String amount = null;

While the performance impact of this isn't that high, it makes your code look better when seeing the variables declared on the point where it is used.

Not closing your IO streams

CLosing your IO streams is important to prevent the java program from getting errors for too many open files, closing can be done by calling reader.close(), or by using a try-with-resources block.

Fixed code

After applying all the steps above, you should get a code similar to the following:

public static void main(String[] args) throws IOException {
    if (args.length == 0) {
        System.out.println("Usage: java -jar program.jar <filename>");
        return;
    }

    File inputFile = new File(args[0]);
    long start = System.currentTimeMillis();

    try (BufferedReader reader = new BufferedReader(new FileReader(inputFile))) {
        String line;
        while((line = reader.readLine()) != null) {
            int firstSpace = line.indexOf(' ');
            int secondSpace = line.indexOf(' ', firstSpace + 1);

            String name = line.subString(0, firstSpace);
            String amount = line.subString(firstSpace + 1, secondSpace);
            String date = line.subString(secondSpace + 1, line.length());

            int year = Integer.parseInt(date.substring(0, 4));
            int month = Integer.parseInt(date.substring(5, 7));
            int day = Integer.parseInt(date.substring(8, 10));

            // flush line
            System.out.print(name.toUpperCase());
            System.out.print(" ");
            System.out.print(amount.indexOf('.') == -1 ? true : false);
            System.out.print(" ");
            System.out.print(day < 10 ? "0" + day : day);
            System.out.print("/");
            System.out.print(month < 10 ? "0" + month : month);
            System.out.print("/");
            System.out.print(year);
            System.out.println();
        }
    }
    System.out.println ("Time taken for String in if/else if :"+ (System.currentTimeMillis() - start));
}


C pointer based growable stack This is my first attempt at writing a C program, it a generic stack that can grow accordingly. It appears to work correctly, however I am worried that is just a fluke and I could be doing something very wrong.

I would greatly appreciate any feedback/criticism.

I am aware I should split this out into a .h and .c file but for demonstration purposes I have listed it as one.

#include <stdio.h>
#include <stdlib.h>
#include <stdbool.h>

typedef struct Stack
{
    void** data;
    int capacity;
    int count;
} Stack;

void stack_init(Stack *stack, int capacity)
{
    stack->data = (void**)malloc(capacity * sizeof(void*));
    stack->capacity = capacity;
    stack->count = 0;
}

void stack_push(Stack *stack, void* entry)
{
    if (stack->count >= stack->capacity)
    {
        stack->capacity *= 2;
        stack->data = (void**)realloc(stack->data, stack->capacity * sizeof(void*));
    }

    stack->data[stack->count] = entry;
    stack->count++;
}

void* stack_pop(Stack *stack)
{
    stack->count--;
    return stack->data[stack->count];
}

bool stack_is_empty(Stack *stack)
{
    return (stack->count == 0);
}

int main()
{
    Stack myStack;

    stack_init(&myStack, 2);

    for (int i = 0; i < 2; i++)
    {
        stack_push(&myStack, i);
    }

    while (!stack_is_empty(&myStack))
    {
        printf("%d\n", (int)stack_pop(&myStack));
    }

    return 0;
}

I suggest that you do more error checking:

malloc or realloc may fail.
I can initialize the stack with negative capacity.
I can pop more elements than I pushed.

In all these cases, your implementation silently swallows the errors and accesses invalid addresses.

As πάντα ῥεῖ said in comments, if you want to store generic elements that exceed void* in size, you need to allocate memory for them and let the void* point to them, e.g.:

typedef struct {
    // ...
} BigStruct;

// pushing
BigStruct* data = malloc(sizeof(BigStruct));
// initialize *data
stack_push(&stack, data);

// popping
BigStruct* data = stack_pop(&stack);
// use *data
free(data);

Some small issues:

It is not common to cast the result of malloc or realloc in C.
If the stack is initialized to capacity 0, then the stack will never grow and elements will be stored at invalid positions.  A solution is to adjust the capacity to 1 automatically.


Pre-commit hook to prevent large file commits I've written the below bash script to run as a pre-commit hook. The intention is to check the git staging area for any files larger than 1mb, and prevent the commit if any are present.

#!/bin/sh
too_big() {
    bytez=$(cat "$(git rev-parse --show-toplevel)/$1" | wc -c)
    if [ "$bytez" -gt 1000000 ] ; then
        cat <<EOF
Error: Attempting to commit a file larger than approximately 1mb.
Commiting large files slows jenkins builds, clones, and other operations we'd rather not slow down.
Consider generating, downloading, zipping, etc these files.
Offending file - $1
EOF
        exit 1
    fi
}

# If you want to allow large files to be committed set this variable to true.
allowbigfiles=$(git config --bool hooks.allowbigfiles)

# Redirect output to stderr.
exec 1>&2

if [ "$allowbigfiles" != "true" ]
then
  set -e
  git diff --name-only --cached $1 | while read x; do too_big $x; done
fi

Edit:
The final script ended up as part of a library of client side Git Hooks

Although described as a Bash script, this appears to be a portable shell script that can be run by any POSIX-conformant shell.  That's a good thing, as it means we can use a much smaller, leaner shell such as Dash.

If you haven't yet installed shellcheck, I recommend you do so (there's also a web version you can try).  It highlights the following:

Useless cat here:

bytez=$(cat "$(git rev-parse --show-toplevel)/$1" | wc -c)

That can be simplified to

bytez=$(<"$(git rev-parse --show-toplevel)/$1" wc -c)

Unquoted expansion of $1 - we really wanted to write "$1" there.
Unsafe read x ought to be read -r x
$x is unquoted

Piping the file into wc isn't an efficient way to measure size of a file; we could simply use stat:

bytez=$(stat -c %s "$(git rev-parse --show-toplevel)/$1")

And instead of running git rev-parse for every file in the changeset, run it once and remember the value in a variable.

The error message should go to the standard error stream (I see the whole script is redirected to &2)

It's not obvious why set -e is right down inside the if - I'd normally put that immediately after the shebang.

Consider also set -u to help detect a likely cause of errors.

Spelling: unless you really mean "1 millibit", that should be "1MB".

A suggestion that might fall into the "too cute" category: since git config --bool always produces true or false as output, we can simply execute that as a command:

if ! $(git config --bool hooks.allowbigfiles)
then

Line-based reading (i.e. git diff --name-only | while read) isn't totally robust; there's a -z option provided to produce NUL-separated output.  This will require Bash, though, in order to read -d.

Improved code

#!/bin/bash

set -e

too_big() {
    if [ "$(stat -c %s "$toplevel/$1")" -gt 1000000 ] ; then
        cat <<EOF
Error: Attempting to commit a file larger than approximately 1MB.
Commiting large files slows jenkins builds, clones, and other
operations we would rather not slow down.
Consider generating, downloading, zipping, etc these files.
Offending file - $1
EOF
        exit 1
    fi
}

# If you want to allow large files to be committed set this variable to true.
allowbigfiles=$(git config --bool hooks.allowbigfiles)

# Redirect output to stderr.
exec >&2

if ! "$allowbigfiles"
then
    toplevel=$(git rev-parse --show-toplevel)
    git diff --name-only -z --cached "$1" |
        while IFS= read -d '' -r x; do too_big "$x"; done
fi


React hooks update array of object I have declared a react hook which will contain an array of object as follows:
const [rowDataTracker, setRowDataTracker] = useState([]);

Now I need to update the hook to basically create a payload as the following:
[
  {id: 100, payload: {field1: "value", field2: "value"}},
  {id: 105, payload: {field1: "value", field3: "value"}},
  {id: 109, payload: {field4: "value"}}
]

I need to check the following condition Before updating the hook:

Need to first check if the id exists in the array or not
if the id exists, then need to update/add the filed to the payload object
if the id does not exist, add a new object to the array with id and the field

const ifItemExist = (arr, item) => {
  return arr.findIndex((e) => e.id === item);
};
function storeEdit(rowId, field, newValue) {
  let itemIndex = ifItemExist(rowDataTracker, rowId);
  let newArr = [...rowDataTracker];
  if (itemIndex > -1) {
    newArr[itemIndex]["payload"][columnId] = newValue;
  } else {
    const obj = {
      itemNbr: rowId,
      payload: {
        [field]: newValue
      }
    };
    newArr.push(obj);
  }
  setRowDataTracker(newArr);
}

I am not sure if this is the correct way to update a hook.
Any review and suggestion will be appreciated.

Mutation You are mutating the existing state when you do
let newArr = [...rowDataTracker];
// ..
  newArr[itemIndex]["payload"][columnId] = newValue;

because spread syntax only creates a shallow copy of the array or object. This should never be done in React. (See end of answer for how to fix it - create the new obj no matter what, then either push it to the new array, or replace the existing object in the array, without mutating the existing object in the array)
Dot notation Most people prefer to read and write using dot notation in JS when possible - it's more concise and easier to read. ESLint rule: dot-notation. You can use .payload instead of ["payload"].
Property/variable naming The rowId refers to the same thing as the itemNbr, which looks to refer to the same thing as the id in the rowDataTracker array. It's good to be consistent with variable names and properties when possible - consider picking one and sticking with it. If you use the variable name itemNbr, you can use it  shorthand in the object literal:
const obj = {
  itemNbr,
  // ...

id or itemNbr or both? You check against the id property in rowDataTracker in ifItemExist, but you push to the same array with a property named itemNbr. This looks very likely to be  a typo. You probably meant to push an object with an id property, or you wanted to check against the itemNbr property instead.
Misleading function name ifItemExist doesn't return a boolean indicating if an item exists - it returns the index of the possibly-existing item. Maybe call it getItemIndex.
Prefer const over let when the variable name doesn't need to be reassigned.
In all:
const getItemIndex = (arr, item) => {
  return arr.findIndex((e) => e.id === item);
};

function storeEdit(id, field, newValue) {
  const itemIndex = getItemIndex(rowDataTracker, id);
  const obj = {
    id,
    payload: {
      ...rowDataTracker[itemIndex]?.payload,
      [field]: newValue
    }
  };
  if (itemIndex === -1) {
    setRowDataTracker([...rowDataTracker, obj]);
    return;
  }
  const newArr = [...rowDataTracker];
  newArr[itemIndex] = obj;
  setRowDataTracker(newArr);
}


A Python Tkinter GUI which you can flick between images I am new to Python and following tutorials.
This one I came across was for TKinter, and throughout I felt I re-wrote the same lines of code time and time again which I wondered if it could be better written.

I am interested to know how you would improve this code.

Thanks.

(First time posting, hope it's ok).

from tkinter import *
from PIL import ImageTk, Image

root = Tk()
root.title("Learn to code")
root.iconbitmap("C:\\palmTree.ico")

#random images
my_img1 = ImageTk.PhotoImage(Image.open("C:\\mountainLake.jpg"))
my_img2 = ImageTk.PhotoImage(Image.open("C:\\freedom.jpg"))
my_img3 = ImageTk.PhotoImage(Image.open("C:\\streetCoffee.jpg"))
my_img4 = ImageTk.PhotoImage(Image.open("C:\\surfBoarding.jpg"))
my_img5 = ImageTk.PhotoImage(Image.open("C:\\oldFence.jpg"))

image_list = [my_img1, my_img2, my_img3, my_img4, my_img5]

my_label = Label(image=my_img1)
my_label.grid(row=0,column=0,columnspan=3)

def forward(image_number):
    global my_label
    global button_forward
    global button_back

    my_label.grid_forget()
    my_label = Label(image=image_list[image_number-1])
    button_forward = Button(root, text=">>", command=lambda: forward(image_number+1))
    button_back = Button(root, text="<<", command=lambda: back(image_number-1))

    if image_number == 5:
        button_forward = Button(root, text = ">>", state=DISABLED)

    my_label.grid(row=0,column=0,columnspan=3)
    button_back.grid(row=1, column=0)
    button_forward.grid(row=1, column=2)

def back(image_number):
    global my_label
    global button_forward
    global button_back

    my_label.grid_forget()
    my_label = Label(image=image_list[image_number-1])
    button_forward = Button(root, text=">>", command=lambda: forward(image_number+1))
    button_back = Button(root, text="<<", command=lambda: back(image_number-1))

    if image_number == 1:
        button_back = Button(root, text = "<<", state=DISABLED)

    my_label.grid(row=0,column=0,columnspan=3)
    button_back.grid(row=1, column=0)
    button_forward.grid(row=1, column=2)

button_back = Button(root, text="<<", command=back, state=DISABLED)
button_quit = Button(root, text="Quit", command=root.quit)
button_forward = Button(root, text=">>", command=lambda: forward(2))

button_back.grid(row=1, column=0)
button_quit.grid(row=1, column=1)
button_forward.grid(row=1, column=2)

root.mainloop()
```

Specific suggestions

Usually when you find yourself numbering variables it's time to put all of them in an iterable of some sort. In your case I would simply inline my_img1 etc. into image_list.
Usually names like images would be used instead of image_list. Python is a duck typed language, so it doesn't really matter which type of iterable you use for the code which does the iterating.
Rather than running the main code like root.mainloop() the Pythonic way to do it is this:

if __name__ == "__main__":
    main()

This ensures that the code doesn't run when importing from your file, making it possible to import into a bigger program.
Importing everything from a module is considered an antipattern - it makes it easier to end up with name collisions and it makes it harder to see at a glance where something was imported from.
Refer to existing data structures whenever possible. One example is if image_number == 5, which would be better as if image_number == len(image_list).
Avoid doing unnecessary work. For example, you create an object and assign it to button_forward, then you check whether you have to create a different object to assign to the same variable, in which case the original work was wasted (or in the worst case, had detrimental side effects). I would instead use an if/else to assign button_forward only once.
globals are considered a big code smell these days. A preferable pattern is to pass around exactly the values which each function/method needs. That way the code is much easier to debug. In fact, this is pretty much the perfect example of something which should be encapsulated in one or more classes. That way you have a natural place to keep track of state like image_number, and won't have to pass it around between methods.
It doesn't seem like a stretch that the images would be the user input to this application. If so it would be better to provide them using argparse.

Tool support suggestions

black can automatically format your code to be more idiomatic. It'll do things like adjusting the vertical and horizontal spacing, while keeping the functionality of the code unchanged.
flake8 can give you hints to write idiomatic Python. I would start with this configuration:

[flake8]
max-complexity = 4
ignore = W503,E203


Maintaining information about people I would like advice if this is a good implementation of an application for maintaining information about people (first name, last name, and age).

The index.html contains an empty div and 2 templates (1 for listing people and the other for entering/editing a user):

<div id="app">
</div>

<script type="text/template" id="listTemplate">
<!-- Iterates over people and makes a table. -->
</script>

<script type="text/template" id="editTemplate">
<!-- Displays a form which if for existing user will fill the values. -->
</script>

In my starting point JS file I put into a namespace 'people.global' a bunch of things that I use throughout the application:

$(function() {

    people.global = {
        people: new people.collections.PeopleCollection()
    };

    people.global = $.extend(people.global, {
        listView: new people.views.ListView(),
        editView: new people.views.EditView(),
        router: new people.Router()
    });

    people.global.people.fetch();

    Backbone.history.start();

});

The router demonstrates the basic idea which is depending on the screen that should be shown it takes the View's el and draws it in the #app element:

var people = people || {};

people.Router = Backbone.Router.extend({

    $app: $('#app'),

    routes: {
        '': 'list',
        'edit': 'edit',
        'edit/:id': 'edit',
        'delete/:id': 'delete'
    },

    list: function() {
        this.$app.html(people.global.listView.render().el);
    },

    edit: function(id) {
        people.global.editView.setId(id);
        this.$app.html(people.global.editView.render().el);
    },

    delete: function(id) {
        var person = people.global.people.get(id);
        person.destroy();
        this.navigate('', {trigger: true, replace: true});
    }

});

Below is the view for editing/adding a person:

var people = people || {};
people.views = people.views || {};

people.views.EditView = Backbone.View.extend({

    template: _.template($('#editTemplate').html()),

    setId: function(id) {
        this.id = id;
    },

    edit: function() {
        var person;

        if (this.$("#id").val() !== '') {
            person = people.global.people.get(this.id);
            person.set({
                'first': this.$("#first").val(),
                'last': this.$("#last").val(),
                'age': this.$("#age").val()
            });

            person.save();
        } else {
            people.global.people.create({
                first: this.$('#first').val(),
                last: this.$('#last').val(),
                age: this.$('#age').val()
            });
        }

                people.global.router.navigate('', {trigger: true, replace: true});
    },

    render: function() {
        var person,
            that = this;

        if (this.id) {
            person = people.global.people.get(this.id);
            this.$el.html(this.template(person.toJSON()));
        } else {
            this.$el.html(this.template({}));
        }

        this.$('button').click(function() {
            that.edit();
        });

        return this;
    }

});

Is this a good design choice?

Is there something that can be done better?

All in all, this looks okay to me, proper naming, nothing bizarre, ( no commenting ).

The only thing that bothers me is that you call .setId(id); on the editView.

editView should have a data model with 1 person, and the .setId() should be called on that data model ( perhaps your function setId() does exactly that ? )

(Deleted race conditions, it not relevant)


Keyboard shortcuts Customization I have created keyboard shortcuts menu,same as like this stackoverflow keyboard shortcuts.In multilevel.

  Scenario

In first level i have assigned functions for only key A.

In the second level i have assigned functions for B & C keys

I have controlled key events B & c in the first level with a variable.

Its working fine.I would like to know is there any better way of doing it.

var level= 0;
$('body').keypress(function(e){
  level == 1 ?(
(((e.keyCode == 66 || e.keyCode == 98)  && level==1 )? (alert("B"),level=2,$('#screen3').show()) : ""),
(((e.keyCode == 67 || e.keyCode == 99)  && level==1 )? (alert("c"),level=2,$('#screen3').show()) : "")
):"";
level == 0 ?((e.keyCode == 65 || e.keyCode == 97) ? level = 1 : level =0):"";
level == 1 ?($('#screen1').hide(),$('#screen2').show()):(level =2 ?$('#screen2').hide():(level= 0));
});
#screen1,#screen2,#screen3 {
background-color: black;
background-color: rgba(0, 0, 0, .8);
position: fixed;
left: 10px;
top: 10px;
padding: 10px;
text-align: left;
border-radius: 6px;
z-index: 1000;
  color:white ;
}

:matches(#screen1,#screen2) pre kbd {
display: inline-block;
font-family: monospace;
}
kbd {
padding: .1em .6em;
border: 1px solid #ccc;
font-size: 11px;
  background-color: #f7f7f7;
color: #333;
  box-shadow: 0 1px 0 rgba(0,0,0,0.2),0 0 0 2px #fff inset;
  border-radius: 3px;
  margin: 0 .1em;
text-shadow: 0 1px 0 #fff;
line-height: 1.4;
white-space: nowrap;
  }

Keyboard shortcuts:
A alert

Keyboard shortcuts:
B alert Key - B
C alert Key - C

Thats it Folks!!

Right off the bat, your code needs some serious indentation. It's barely readable given its current state. If you happen to use Sublime Text, there's HTML-CSS-JS Prettify plugin.

Another thing I noticed is you're loading jQuery twice.

Next, your code is highly coupled. Your condition to check the buttons are highly coupled to the behavior it does. What would happen if I want to change the behavior to another key? Or what if you want to operate in another platform which uses different keyboard norms (Mac vs PC)? You can read Game Programming Patterns to get an idea of configurable keys.

Another thing to consider is what if the user wants to bail out of a level? What if the user accidentally pressed a button that progressed him a level, but wants to go back and use another combination instead? Usually Esc was reserved for this. For multi-press button combinations, releasing all buttons is an option.

jQuery also has event.which which is a normalized version of charCode and keyCode.

Your code lives in the global space, which means level is highly modifiable and pollutes the global namespace. You could put your code inside a closure, an IIFE to be exact, to prevent that.


Setting vars in jQuery / JavaScript I am setting a few vars in JavaScript. I typically like to make my code really readable so that it's easy to go back and edit.  I have a few questions about the code below:

1) Is this is a good way to break up the setting of the vars?  The comment seems redundant, but it seems to really help break up the code into relevant chunks.

2) When I set a var like this ~~ $IPPWrapper = $("div.iqPaginItemsPerPageWrapper") ~~ I always include the HTML element, in this case the "div".  Is this overkill? Is there a performance hit or benefit? I know in the CSS best practices, it's frowned upon.

3) I need to create an array, but I don't know the best type to use. Basically, I am just storing messages that I will use later in popups.  Is there a more common or better way to create this array?

Thanks in advance for any advice or helpful comments.

// GENERAL VARS
var UpRate = 250,
    DownRate = UpRate,
    OpenHeight = "25px";

// SET IPP VARS
var $IPPWrapper = $("div.iqPaginItemsPerPageWrapper"),
    $IPPDefault = $IPPWrapper.children("div.iqPaginDefault"),
    $IPPNonDefault = $IPPWrapper.children("div.iqPaginNonDefault"),
    $IPPOptions = $IPPWrapper.children("div.ItemsPerPage"),
    $IPPValue = $("span#ItemsPerPageValue");

// SET SB VARS
var $SBWrapper = $("div.iqPaginSortByWrapper"),
    $SBDefault = $SBWrapper.children("div.iqPaginDefault"),
    $SBNonDefault = $SBWrapper.children("div.iqPaginNonDefault"),
    $SBOptions = $SBWrapper.children("div.SortBy"),
    $SBValue = $("span#SortByValue");

// CREATE SB TEXT ARRAY
var SBTextArray = new Array();
    SBTextArray[1] = "Company Name A - Z";
    SBTextArray[2] = "Company Name Z - A";
    SBTextArray[3] = "Product Name A - Z";
    SBTextArray[4] = "Product Name Z - A";
    SBTextArray[5] = "Model Number A - Z";
    SBTextArray[6] = "Model Number Z - A";

EDIT ~~ Is this a better way of coding the above array?

// CREATE SB TEXT ARRAY
var SBTextArray = [
        "Company Name A - Z", "Company Name Z - A",
        "Product Name A - Z", "Product Name Z - A",
        "Model Number A - Z", "Model Number Z - A",
    ];

Your code is great so far.
The only tip I can think of is to wrap grouped variables inside a namespace / singleton.
This would make it easier to debug and perform operations of group of related elements.

    // GENERAL VARS
var UpRate = 250,
    DownRate = UpRate,
    OpenHeight = "25px";

// SET IPP VARS
var $IPP = {};
$IPP.Wrapper = $("div.iqPaginItemsPerPageWrapper");
$IPP.Default = $IPP.Wrapper.children("div.iqPaginDefault");
$IPP.NonDefault = $IPP.Wrapper.children("div.iqPaginNonDefault");
$IPP.Options = $IPP.Wrapper.children("div.ItemsPerPage");
$IPP.Value = $("span#ItemsPerPageValue");

// SET SB VARS
var $SB = {};
$SB.Wrapper = $("div.iqPaginSortByWrapper");
$SB.Default = $SB.Wrapper.children("div.iqPaginDefault");
$SB.NonDefault = $SB.Wrapper.children("div.iqPaginNonDefault");
$SB.Options = $SB.Wrapper.children("div.SortBy");
$SB.Value = $("span#SortByValue");

// CREATE SB TEXT ARRAY
var SBTextArray = [];
SBTextArray[1] = "Company Name A - Z";
SBTextArray[2] = "Company Name Z - A";
SBTextArray[3] = "Product Name A - Z";
SBTextArray[4] = "Product Name Z - A";
SBTextArray[5] = "Model Number A - Z";
SBTextArray[6] = "Model Number Z - A";

1) Use a namespace for related elements and functionality.

2) It depends. In most cases it doesn't matter unless if you different tags with the same classes. Like p.class1 and dd.class1. More info here

3) You could just use an object literal to store the message by a reference name.
Something like this.

var listing = { az: {}, za: {} };
listing.az.company = "Company Name A - Z";
listing.az.product = "Product Name A - Z";
listing.az.model = "Model Number A - Z";
listing.za.company = "Company Name Z - A";
listing.za.product = "Product Name Z - A";
listing.za.model = "Model Number Z - A";

I think your current code is good enough though.


small web scraper to read product highlight from given urls I have written a Python scraping program, using the Scrapy framework. The following code reads a list of urls that need to crawled and gets product highlights from the list. It also follows the next page button.
The resulting products are automatically written to an output file, as I have set FEED_URI in setting.py (which is scrap's setting file), as below:
FEED_URI = 'product_export__%(time)s.xml'
This is the code of my crawler/spider:
import scrapy

# list of wanted organisation is written in: organisation.txt
def get_wanted_organisations():
    org_file = open('config/organisation.txt', 'r')
    return org_file.read().splitlines()

# list of urls to crawl is writtem in url.txt
def get_start_urls():
    org_file = open('config/url.txt', 'r')
    return org_file.read().splitlines()

class ProductSpider(scrapy.Spider):
    name = 'product_spider'

    def __init__(self):
        self.product_highlight_dict = {}
        self.wanted_organisations = get_wanted_organisations() # ignore products listed by any other organisation

    def start_requests(self):
        org_file = open('config/url.txt', 'r')
        start_urls = get_start_urls()
        for url in start_urls:
            yield scrapy.Request(url=url, callback=self.parse)

    def parse(self, response):
        # get a list of product highlights in the page
        product_highlight = response.css('div > article[data-product-id]')

        for product_highlight in product_highlights:
            organisation = product_highlight.css('a[data-company="ListingCompany"]::text').get()

            if organisation and organisation.lower() in self.wanted_organisations:

                # we are interested in the following 4 fields, they will be written to output file, by setting FEED_URI setting of Scrapy framework
                yield {
                    'product_id': product_highlight.css('[data-id="ProductId"] span::text').get(),
                    'title': product_highlight.css('[data-title="ProductTitle"] h1::text').get(),
                    'price': response.css('[data-price="ProductPrice"] span span::text').get(),
                    'organisation': organisation,
                }

        # follow pagination link
        next_page = response.css('a[data-pager="NextPage"]::attr(href)').get()
        if next_page is not None:
            yield response.follow(next_page, self.parse)

I come from .NET background and this is my first Python program. Appreciate any feedback. Not sure if I am following the correct coding convention for naming variables, functions, etc.
Also, the entire program is in one file. Would it be better to move the first two functions into a different file and name the file something like settings_reader?

Splitting lines
Don't call splitlines() here:
org_file = open('config/organisation.txt', 'r')
return org_file.read().splitlines()

The file object itself is an iterator over its lines. Also, use a context manager to ensure file closure:
with open('config/organisation.txt', 'r') as f:
    return {line.rstrip() for line in f}

This is a set comprehension. You want a set because you're only ever checking for membership, and this will be more efficient.
Generator simplification
You don't really need to yield here:
for url in start_urls:
    yield scrapy.Request(url=url, callback=self.parse)

Instead,
return (
    scrapy.Request(url=url, callback=self.parse)
    for url in start_urls
)

Strongly-typed results
Since you're more experienced in .NET, consider how you would traditionally represent this:
yield {
    'product_id': product_highlight.css('[data-id="ProductId"] span::text').get(),
    'title': product_highlight.css('[data-title="ProductTitle"] h1::text').get(),
    'price': response.css('[data-price="ProductPrice"] span span::text').get(),
    'organisation': organisation,
}

Hint: it's not a dictionary. You would do better to make a class (perhaps a @dataclass) with well-defined members, and set your return type hint for this method to  -> Iterable[ResultType].


Optimize a Sql subquery containing multiple inner joins and aggregate functions I have a select statement which is infact a subquery within a larger select statement built up programmatically. The problem is if I elect to include this subquery it acts as a bottle neck and the whole query becomes painfully slow.

An example of the data is as follows:

Payment
.Receipt_no|.Person |.Payment_date|.Type|.Reversed|
          2|John    |01/02/2001   |PA   |         |
          1|John    |01/02/2001   |GX   |         |
          3|David   |15/04/2003   |PA   |         |
          6|Mike    |26/07/2002   |PA   |R        |
          5|John    |01/01/2001   |PA   |         |
          4|Mike    |13/05/2000   |GX   |         |
          8|Mike    |27/11/2004   |PA   |         |
          7|David   |05/12/2003   |PA   |R        |
          9|David   |15/04/2003   |PA   |         |

The subquery is as follows :

select Payment.Person,
Payment.amount
from Payment
inner join (Select min([min_Receipt].Person) 'Person',
   min([min_Receipt].Receipt_no) 'Receipt_no'
   from Payment [min_Receipt]
   inner join (select min(Person) 'Person',
      min(Payment_date) 'Payment_date'
      from Payment
      where Payment.reversed != 'R' and Payment.Type != 'GX'
      group by Payment.Person) [min_date]
   on [min_date].Person= [min_Receipt].Person and [min_date].Payment_date = [min_Receipt].Payment_date
   where [min_Receipt].reversed != 'R' and [min_Receipt].Type != 'GX'
   group by [min_Receipt].Person) [1stPayment]
on [1stPayment].Receipt_no = Payment.Receipt_no

This retrieves the first payment of each person by .Payment_date (ascending), .Receipt_no (ascending) where .type is not 'GX' and .Reversed is not 'R'. As Follows:

Payment
.Receipt_No|.Person|.Payment_date
          5|John   |01/01/2001
          3|David  |15/04/2003
          8|Mike   |27/11/2004

I am unable to move the subquery out to a temporary table as temporary tables are simply not supported within the programming language used by my application.

Edit : Incorrect statement. Temporary tables are supported and therefore this is a valid option.

Following a post on StackOverflow -

The Query was rewritten as the following.

Query 1.

select min(Payment.Person) 'Person',
    min(Payment.receipt_no) 'receipt_no'
from
    Payment a
where
    a.type<>'GX' and (a.reversed not in ('R') or a.reversed is null)
    and a.payment_date =
      (select min(payment_date) from Payment i
      where i.Person=a.Person and i.type <> 'GX'
      and (i.reversed not in ('R') or i.reversed is null))
group by a.Person

I added this as a subquery within my much larger query, however it still ran very slowly. So I tried rewriting the query whilst trying to avoid the use of aggregate functions and came up with the following.

Query 2.

SELECT
    receipt_no,
    person,
    payment_date,
    amount
FROM
    payment a
WHERE
    receipt_no IN
      (SELECT
           top 1 i.receipt_no
       FROM
           payment i
       WHERE
           (i.reversed NOT IN ('R') OR i.reversed IS NULL)
           AND i.type<>'GX'
           AND i.person = a.person
       ORDER BY i.payment_date DESC, i.receipt_no ASC)

Which I wouldn't necessarily think of as being more efficient. In fact if I run the two queries side by side on my larger data set Query 1. completes in a matter of milliseconds where as Query 2. takes several seconds.

However if I then add them as subqueries within a much larger query, the larger query completes in hours using Query 1. and completes in 40 seconds using Query 2.

I can only attribute this to the use of aggregate functions in one and not the other.

I see that in your question you said:

  "I am unable to move the subquery out to a temporary table as temporary tables are simply not supported within the programming language used by my application."

But, have you considered calling a stored procedure instead? Is this even an option, considering the limitations with the programming language?

If this is a viable option, you could simply have the results of your subquery inserted into a temp table transparently & encapsulate all the logic in the stored procedure.

Edit

I got to thinking about this some more, and perhaps the columns that you're using in your JOIN condition are of different collations. While this will usually result in a specific error message, there may be some implicit collation coversion occurring instead (see: MSDN: Collation Precedence (Transact-SQL)) between the sub-query & the data being joined.

Here are a few links about collation that might be useful to you:

Difference between collation SQL_Latin1_General_CP1_CI_AS and Latin1_General_CI_AS
Collation Hell (Part 1)
SQL SERVER – Find Collation of Database and Table Column Using T-SQL
SQL SERVER – Change Collation of Database Column – T-SQL Script

Also, you may be able to trick your programming language into using a temp table with syntax like this:

SELECT *
  FROM tempdb..#MyTempTable

Just keep in mind that sometimes the temp database has a different collation then the data you're working with too, in which case you'll need to explicitly convert the data to/from each collation.


Python: Insertion Sort I am a beginner in python and to learn python I have written this program. How can this code be improved? Should I use main method in this program? Python

lst = []

def insertion_sort(thelist):
    for index in range(1, len(thelist)):
        key = thelist[index]
        position = index - 1
        while position >= 0 and thelist[position] > key:
            thelist[position + 1] = thelist[position]
            position = position - 1
        thelist[position + 1] = key

n = int(input("Enter number of elements: "))
for i in range(0, n):
    ele = int(input())
    lst.append(ele)

insertion_sort(lst)
print(*lst, sep = ", ")

There is a much simpler method in Python to do an insertion sort using the
bisect.insort method

import bisect
array = [1,3,5,7]
bisect.insort_left(array, 2)

>>[1, 2, 3, 5, 7]

With regard to your coding style, I would recommend using the if __name__ == "__main__" guard.

You can also just say for i in range(n) with no need to say (0, n)

You can also use list-comprehension to build the list:

import bisect
def insertion_sort(thelist):
    new_list = []
    for item in thelist:
        bisect.insort_left(new_list, item)
    return new_list

if __name__ == "__main__":
    n = int(input("Enter number of elements: "))
    lst = [input("Please enter a number: ") for i in range(n)]
    new_list = insertion_sort(lst)
    print(*new_list, sep = ", ")


Which class is best to call .getResource on? I'm creating a Java Helper Library with helpful static methods. I find myself needing to call ...getResource(...) or ...getResourceAsStream(...) in several instances and I'm wondering what's the best practice for this. It seems to be working fine when I say: Helper.class.getResource(...), but I'm wondering whether that will bite me in the future.

I have a few alternatives I'm thinking of. Below is an example of when I need to use this and the alternatives I have thought of:

Alternative 1 (what I would prefer for the sake of users):

/**
* Takes the given resource and returns that as a string.
*
* @param location of the file
* @return the file as a string
* @throws IOException
*/
public static String resourceToString(String location) throws IOException {
  InputStream is = IOHelper.class.getResourceAsStream(location);
  InputStreamReader r = new InputStreamReader(is);
  return readerToString(r);
}

Alternative 2:

/**
* Takes the given resource (based on the given class) and returns that as a string.
*
* @param location of the file
* @param c class to get the resource from
* @return the file as a string
* @throws IOException
*/
public static String resourceToString(String location, Class c) throws IOException {
  InputStream is = c.getResourceAsStream(location);
  InputStreamReader r = new InputStreamReader(is);
  return readerToString(r);
}

Alternative 3 (should I just have the user give me the resource?):

/**
* Takes the given InputStream and returns that as a string.
*
* @param is the InputStream of the File to read
* @return the file as a string
* @throws IOException
*/
public static String resourceToString(InputStream is) throws IOException {
  InputStreamReader r = new InputStreamReader(is);
  return readerToString(r);
}

I would prefer the second one. The others will bite if the library runs inside an OSGi container.

Furthermore, I would

pass UTF-8 character encoding to the constructor of the InputStreamReader. Otherwise it will use the system encoding which vary from system to system. (Or let the user to set it with a third method parameter.)
use longer variable names. is, r, c are not too easy to read. I would call them resourceStream, clazz and resourceReader. It does not force readers to decode them.
reverse the order of String location, Class c parameters. It would be in the same order as the first line use them (c.getResourceAsStream(location)). I don't know if there is a rule of thumb for this, but it looks more natural to me. (Any pointers are welcome.)
remove the * @throws IOException javadoc line. It is meaningless in this form.


User defined string literals and namespace use I have a need to process URIs in one of my applications, sometimes from const char[] literals.

My API:

namespace net {
    class uri { ... }; // contains decomposed URI (path, parameters, etc)

    uri parse_uri(const std::string& uri_str); // parse string and return uri instance
}

// within global namespace (this is what the question is about):
uri operator "" _uri(const char*str, const unsigned long size)
{
    return net::parse_uri(std::string{str, str + size});
}

Client code:

auto x = "/static/path/here/image.jpg"_uri;

I tried defining the string conversion operator within the net namespace, but (disimilar to other operators) it seems I have to explicitly specify access to it in client code (with using namespace net; for example).

My question:

Does this clash with the "compiler searches for operators in the namespace of the object it applies to" rule (like it would for an operator+(uri&, uri&) for example)? (As a rule of thumb, I assume the problem is with my code, not the compiler).

What is the best practice or policy to use here? Import the namespace / operator in client code, or pollute the global namespace (like I did)?

I am using CLang 5.1 on OS X.

I would follow the way user-defined literals are used in the C++14 standard library: put them in a literals subnamespace that can be imported by the user. This subnamespace would be inline so that it can be imported alone, or automatically imorted when the namescape net is imported:

namespace net {
    class uri { ... }; // contains decomposed URI (path, parameters, etc)

    uri parse_uri(const std::string& uri_str); // parse string and return uri instance

    inline namespace literals
    {
        uri operator "" _uri(const char*str, const unsigned long size)
        {
            return net::parse_uri(std::string{str, str + size});
        }
    }
}

Then, a client code could use your literal with the following code:

int main()
{
    // Import only the literal, not the rest of net
    using namespace net::literals;

    auto x = "/static/path/here/image.jpg"_uri;
}

I actually slightly simplified the way user-defined literals are used in the standard library for your example: there are actually nested inline namespaces in namespace literals so that a more precise selection of related literals can be done if needed (for example, import only the std::chrono-related literals). Since you have only one literal, you don't need to have so many nested namespaces, but you can still add them later if needed.


Custom ViewModel to check property existance prior to update I have written an abstract View Model which all my other View Models inherit when I write any apps in Silverlight, WinRT, WPF etc. The class was written to attempt to address an issue I have run into frequently. That is, after refactoring code, all my property names are wrong and I often forget to rename them. Debugging by identifying views which don't reflect their view models is hard, thus this.

The idea is relatively simple. It provides an implementation of the INotifyPropertyChanged interface with the addition of some additional safety checking when debugging the project. This is done via reflection by checking if the property you are attempting to notify actually exists.

Thus:

class Foo : AbstractViewModel {
    public object Bar { get; set; }
}

Will throw an exception if I try to call NotifyPropertyChanged("Ponies") since Ponies is not a property in Foo.

The AbstractViewModel looks like this:

using System;
using System.ComponentModel;

#if DEBUG
// We need reflection in order to debug our Property Changed Notifications
using System.Reflection;
#endif

namespace WorldOfZero.Common.ViewModels
{
    public class AbstractViewModel : INotifyPropertyChanged
    {
        public event PropertyChangedEventHandler PropertyChanged;

        protected void NotifyPropertyChanged(string name)
        {
#if DEBUG
            // This will detect and throw an error if we attempt to Notify a change on a none existant property.
            // This helps eliminate stupid mistakes like spelling errors.
            // This code is only included in the debug builds of the app, this prevents us from slowing down production code.
            Type type = this.GetType();
            PropertyInfo property = type.GetRuntimeProperty(name);
            if (property == null)
            {
                throw new InvalidOperationException(String.Format("Attempting to Notify a change on a property ({0}) that does not exist.", name));
            }
#endif

            if (PropertyChanged != null)
            {
                PropertyChanged(this, new PropertyChangedEventArgs(name));
            }
        }
    }
}

I'm looking for ways to improve this. I'm also curious if there might be better ways to solve this than with Reflection.

#if DEBUG
// We need reflection in order to debug our Property Changed Notifications
using System.Reflection;
#endif

That #if directive is overkill. A using statement isn't like a C++ include, it doesn't clutter up your assembly with code you may not need. All it does, is facilitate coding by telling the parser that types in these namespaces don't need to be fully qualified; once the code is compiled, they're "inlined" anyway.

Now, the idea is nice, but as you noticed, you can give it just about any value and it happily passes the "name" to whoever is listening for that event.

That string parameter isn't refactoring-friendly. You've mitigated the issue by throwing an InvalidOperationException - if your ViewModel classes' setters are 100% covered with unit tests, you're probably good to go.

The thing that worries me is that the exception isn't thrown in a release build, so instead of blowing up and failing promptly, you're failing silently, and worse things could result from this. Of course that should not happen.. but the room for human error is there.

The best way to implement that interface, is in my opinion with a strongly-typed overload that takes a selector expression and passes the selected member's name to the interface's stringly-typed method.

Something like this for example, can turn your setters into something like this:

private int _foo;
public int Foo
{
    get { return _foo; }
    set
    {
        if (value != _foo)
        {
            _foo = value;
            OnPropertyChanged(() => this.Foo);
        }
    }
}


Custom Web Api asynchronous filter implementation Following the tutorial from Filip W's blog, I've ended up creating my own implementation for a custom ActionFilter.

This logs the execution name, action, parameters and elapsed time of every ApiController in my solution.

The post is a bit old (2013), that's why I'm still thinking it must be a better/simpler way to do this, any suggestion? improvements?

public sealed class LogActionFilter : FilterAttribute, IActionFilter
{
    private readonly Stopwatch _stopwatch = new Stopwatch();

    public bool AllowMultiple => false;

    public async Task<HttpResponseMessage> ExecuteActionFilterAsync(HttpActionContext actionContext,
        CancellationToken cancellationToken, Func<Task<HttpResponseMessage>> continuation)
    {
        await InternalActionExecuting(actionContext);

        if (actionContext.Response != null)
            return actionContext.Response;

        HttpActionExecutedContext executedContext;
        try
        {
            var response = await continuation();
            executedContext = new HttpActionExecutedContext(actionContext, null)
            {
                Response = response
            };
        }
        catch (Exception exception)
        {
            executedContext = new HttpActionExecutedContext(actionContext, exception);
        }

        await InternalActionExecuted(executedContext);
        _stopwatch.Reset();

        return executedContext.Response;
    }

    private Task InternalActionExecuting(HttpActionContext actionContext)
    {
        _stopwatch.Start();
        return Log("Executing", actionContext, 0);
    }

    private Task InternalActionExecuted(HttpActionExecutedContext actionExecutedContext)
    {
        _stopwatch.Stop();
        return Log("Executed", actionExecutedContext.ActionContext, _stopwatch.ElapsedMilliseconds);
    }

    private Task Log(string action, HttpActionContext actionContext, long elapsedTime)
    {
        return Task.Run(() =>
        {
            var controllerName = actionContext.ControllerContext.ControllerDescriptor.ControllerName;
            var actionName = actionContext.ActionDescriptor.ActionName;
            var parameters = string.Join(", ", actionContext.ActionArguments.Values.Select(x => x).ToArray());

            var message = $"{action}: ctrl: {controllerName}, act: {actionName}, params: {parameters}" +
                          $"{(elapsedTime > 0 ? "took (ms): " + elapsedTime : string.Empty) }";

            Trace.WriteLine(message, "Action filter log");
        });
    }
}

Here is couple of things that I could done;

Implement a logger class, Logging with "Trace.WriteLine" is not usefull for everycase.
Override allow multiple; public override bool AllowMultiple => false;
Log the error...

public sealed class LogActionFilter : FilterAttribute, IActionFilter
{
    private readonly ILogManager _logManager;

    public LogActionFilter(ILogManager logManager)
    {
        _logManager = logManager;
    }

    private readonly Stopwatch _stopwatch = new Stopwatch();

    public override bool AllowMultiple => false;

    public async Task<HttpResponseMessage> ExecuteActionFilterAsync(HttpActionContext actionContext,
                                                                    CancellationToken cancellationToken,
                                                                    Func<Task<HttpResponseMessage>> continuation)
    {
        await InternalActionExecuting(actionContext);

        if (actionContext.Response != null)
        {
            return actionContext.Response;
        }

        HttpActionExecutedContext executedContext;

        try
        {
            var response = await continuation();

            executedContext = new HttpActionExecutedContext(actionContext, null)
                              {
                                  Response = response
                              };
        }
        catch (Exception exception)
        {
            executedContext = new HttpActionExecutedContext(actionContext, exception);
            _logManager.Error(exception, "LogAction recorded an error");
        }

        await InternalActionExecuted(executedContext);
        _stopwatch.Reset();

        return executedContext.Response;
    }

    private Task InternalActionExecuting(HttpActionContext actionContext)
    {
        _stopwatch.Start();
        return Log("Executing", actionContext, 0);
    }

    private Task InternalActionExecuted(HttpActionExecutedContext actionExecutedContext)
    {
        _stopwatch.Stop();
        return Log("Executed", actionExecutedContext.ActionContext, _stopwatch.ElapsedMilliseconds);
    }

    private Task Log(string action, HttpActionContext actionContext, long elapsedTime)
    {
        return Task.Run(() =>
                        {
                            var controllerName = actionContext.ControllerContext.ControllerDescriptor.ControllerName;
                            var actionName = actionContext.ActionDescriptor.ActionName;
                            var parameters = string.Join(", ", actionContext.ActionArguments.Values.Select(x => x).ToArray());

                            var message = $"{action}: ctrl: {controllerName}, act: {actionName}, params: {parameters}" +
                                          $"{(elapsedTime > 0 ? "took (ms): " + elapsedTime : string.Empty)}";
#if (DEBUG)
                            Trace.WriteLine(message, "Action filter log");
#elif (RELEASE)
                            _logManager.Debug(null, message, "Action filter log");
#endif

                        });
    }
}