Prime Number Generator in Swift I would like to make (or improve my function) a function that returns an array of integers containing all the prime numbers from 2...n that can execute as quickly and efficiently as possible (ideally so that it will be able to execute n = 1,000,000,000 in a matter of seconds). I am using Swift 4.1 and Xcode 9.3.

Where I'm currently at

So far, I've used the Sieve of Eratosthenes to make this function that can calculate prime numbers under 1,000,000 in 0.28s, but when I try it with a number like 1 billion or 1 trillion, it takes too long.

func generatePrimes(to n: Int) -> [Int] {

    precondition(n > 5, "Input must be greater than 5")

    var arr : [Int] = Array<Int>(stride(from: 3, to: n, by: 2))

    for index in 0... {
        let num : Int = arr.remove(at: index)
        arr = arr.filter{ $0 % num != 0 }
        arr.insert(num, at: index)
        if arr[index + 1] >= Int(floor(sqrtf(Float(n)))) { break }
    }

    arr.insert(2, at: 0)

    //Print Statements
    print("Prime numbers under \(n):")
    _ = arr.enumerated().map { (index, element) in print("\t\(index + 1). \(element)") }

    return arr
}

Usage:

generatePrimes(to: 50)

Prints:

Prime numbers under 50:
    1. 2
    2. 3
    3. 5
    4. 7
    5. 11
    6. 13
    7. 17
    8. 19
    9. 23
    10. 29
    11. 31
    12. 37
    13. 41
    14. 43
    15. 47

That is not the Sieve of Eratosthenes

The Sieve of Eratosthenes computes multiples
of each found prime to mark subsequent composite numbers in the sieve.
Your algorithm computes the remainder of all subsequent numbers instead.
That makes a huge difference. I'll come back to that later, let's start
with a

Review of your current code

There is a inconsistency here:

func generatePrimes(to n: Int) -> [Int] {
    // ...
    var arr : [Int] = Array<Int>(stride(from: 3, to: n, by: 2))
    // ...
}

The function – as I understand the to parameter –
computes all primes up to and including n. On the other hand,
stride(from: 3, to: n, by: 2) does not include the upper bound,
and that is easily verified with

print(generatePrimes(to: 11)) // [2, 3, 5, 7]

So either rename the function to func generatePrimes(below n: Int)
or use stride(from: 3, through: n, by: 2) to include the upper bound.
I'll do the latter for this review.

It would also be a good idea to add a documentation comment which
unambiguously documents the function parameters.

The explicit type annotation is not needed, and the array creation
can be simplified to

var arr = Array(stride(from: 3, through: n, by: 2))

Why is the limit number expected to be greater than 5? That is an
unnecessary restriction and would be unexpected for a caller of your function. It may be that your implementation  does not work if
\$ n \le 5 \$, but it would be easy to handle that case separately:

if n <= 5 {
    return [2, 3, 5].filter { $0 <= n }
}

In

    let num : Int = arr.remove(at: index)

the type annotation on the left is again not needed.

(Remark: This

    arr = arr.filter { $0 % num != 0 }

can be replaced by the more efficient

    arr.removeAll(where: { $0 % num == 0 })

in a future version of Swift, when SE-0197 Adding in-place removeAll(where:) to the Standard Library is implemented in Swift 4.2.)

Here

    if arr[index + 1] >= Int(floor(sqrtf(Float(n)))) { break }

is a hidden bug: As

print(generatePrimes(to: 26)) // [2, 3, 5, 7, 11, 13, 17, 19, 23, 25]

demonstrates, you are leaving the loop to early.
I would also compute the square root only once, and use double-precision
arithmetic: The 24 bit significand of a Float cannot represent large
integers correctly (compare Computing the integer square root of large numbers).

Finally, the function should compute the result, but not print it, i.e. this

//Print Statements
print("Prime numbers under \(n):")
_ = arr.enumerated().map { (index, element) in print("\t\(index + 1). \(element)") }

should be removed. It is generally a good habit to separate computation from I/O.
In addition, this distorts the benchmark results, because you measure also the
time for converting the numbers to strings, and the time to print these strings
(which depends on the output device, printing to a file is faster than printing
to the Terminal or the Xcode console).

With all changes suggested so far, your function would look like this:

/// Compute list of primes
///
/// - Parameter n: The upper bound
/// - Returns: An array with all primes <= `n`
func generatePrimes(to n: Int) -> [Int] {

    if n <= 5 {
        return [2, 3, 5].filter { $0 <= n }
    }

    var arr = Array(stride(from: 3, through: n, by: 2))

    let squareRootN = Int(Double(n).squareRoot())
    for index in 0... {
        if arr[index] > squareRootN { break }
        let num = arr.remove(at: index)
        arr = arr.filter { $0 % num != 0 }
        arr.insert(num, at: index)
    }

    arr.insert(2, at: 0)
    return arr
}

Using the Sieve of Eratosthenes

As I said above, your algorithm is different from the Eratosthenes sieve.
Each time a prime number \$ p \$ is found, your code does trial divisions for
all remaining numbers to remove multiples of \$ p \$.
The Eratosthenes sieve computes multiples of \$ p \$ instead:

p*p, p*p + p, p*p + 2*p, p*p + 3*p, ...

and marks these as composite. Note that there are far less values to compute.

Also your algorithm removes and inserts array elements frequently.
That is slow because the remaining elements must be shifted to the left
or to the right.

The Eratosthenes sieve works with a fixed-sized boolean array instead.

Here is a very simple straight-forward implementation in Swift:

func eratosthenesSieve(to n: Int) -> [Int] {
    var composite = Array(repeating: false, count: n + 1) // The sieve
    var primes: [Int] = []

    if n >= 150 {
        // Upper bound for the number of primes up to and including `n`,
        // from https://en.wikipedia.org/wiki/Prime_number_theorem#Non-asymptotic_bounds_on_the_prime-counting_function :
        let d = Double(n)
        let upperBound = Int(d / (log(d) - 4))
        primes.reserveCapacity(upperBound)
    } else {
        primes.reserveCapacity(n)
    }

    let squareRootN = Int(Double(n).squareRoot())
    var p = 2
    while p <= squareRootN {
        if !composite[p] {
            primes.append(p)
            for q in stride(from: p * p, through: n, by: p) {
                composite[q] = true
            }
        }
        p += 1
    }
    while p <= n {
        if !composite[p] {
            primes.append(p)
        }
        p += 1
    }
    return primes
}

The only optimization I did was to reserve capacity for the result
array in order to avoid reallocations.

This is already considerably faster than your original code.
Here is my benchmarking code:

func benchmark(_ ident: String, _ function: (Int) -> [Int], to n: Int) {
    let start = Date()
    let primes = function(n)
    let elapsed = Date().timeIntervalSince(start)
    print(ident, primes.count, "primes up to", n, String(format: "time: %.3f", elapsed))
}

benchmark("original", generatePrimes, to:     100_000)
benchmark("original", generatePrimes, to:   1_000_000)
benchmark("original", generatePrimes, to:  10_000_000)

print()

benchmark("eratosthenes", eratosthenesSieve, to:       100_000)
benchmark("eratosthenes", eratosthenesSieve, to:     1_000_000)
benchmark("eratosthenes", eratosthenesSieve, to:    10_000_000)
benchmark("eratosthenes", eratosthenesSieve, to:   100_000_000)
benchmark("eratosthenes", eratosthenesSieve, to: 1_000_000_000)

On a 1.2 GHz Intel Core m5 MacBook (Retina, 12-inch, Early 2016)
with 8GM RAM I get the following results (slightly reformatted to
increase the legibility):

original   9592 primes up to   100000 time: 0.038
original  78498 primes up to  1000000 time: 0.418
original 664579 primes up to 10000000 time: 9.739

eratosthenes     9592 primes up to     100000 time:  0.001
eratosthenes    78498 primes up to    1000000 time:  0.007
eratosthenes   664579 primes up to   10000000 time:  0.096
eratosthenes  5761455 primes up to  100000000 time:  1.223
eratosthenes 50847534 primes up to 1000000000 time: 15.034

This can surely be further improved, a first step would be to handle the case p=2
separately and to use the sieve only for odd numbers (which however complicates the
index calculations a bit).

But, as you see, computing the primes up to 1 billion is feasible with the sieve
of Eratosthenes (and the numbers are correct, you can compare them with
https://primes.utm.edu/howmany.html).

Going for the trillion

You were also thinking of computing all primes up to one trillion. That might be
a problem, no matter which algorithm you choose. According to
https://primes.utm.edu/howmany.html, there are 37,607,912,018 prime numbers
below 1 trillion. Even if we use 32-bit bit integers to save space, that would
still require approx 140 GB memory, which makes returning an array with all that
numbers difficult.

So that would require a different approach which allows to compute prime numbers in
some given range, instead of all prime numbers up to a limit.


How secure is my PHP registration script? I coded a registration form and was wondering if anyone could give me feedback on whether I'm lacking any major security mechanisms. I've done loads of reading around the topic and feel it should be secure but expert feedback would be great.
Registration script
<?php

// User/pass only used locally
$host = 'localhost';
$db   = 'new_db';
$user = 'root';
$pass = '';

$options = [
    PDO::ATTR_ERRMODE            => PDO::ERRMODE_EXCEPTION,
    PDO::ATTR_DEFAULT_FETCH_MODE => PDO::FETCH_ASSOC,
    PDO::ATTR_EMULATE_PREPARES   => false,
];
$dsn = "mysql:host=$host;dbname=$db";
try {
     $pdo = new PDO($dsn, $user, $pass, $options);
} catch (\PDOException $e) {
     throw new \PDOException($e->getMessage(), (int)$e->getCode());
}

$errors = [];
if ( $_POST )
{

    // Get form field values
    $fname              = filter_input(INPUT_POST, 'fname', FILTER_SANITIZE_STRING);
    $email              = filter_input(INPUT_POST, 'email', FILTER_SANITIZE_EMAIL);
    $pwd                = filter_input(INPUT_POST, 'password', FILTER_SANITIZE_STRING);
    $pwd_confirm        = filter_input(INPUT_POST, 'confirm-password', FILTER_SANITIZE_STRING);

    // Generate activation key
    $activation_key     = md5(uniqid(mt_rand(), true));
    $activation_link    = 'https://www.example.com/activate?id='.$activation_key.'&name='.$fname;

    // Check if passwords match
    if ($pwd !== $pwd_confirm) {
        $errors[] = "Passwords don't match";
    }

    // Check if password is secure
    if (strlen($pwd) < 8) {
        $errors[] = "Password not long enough! Must be at least 8 characters long";
    }

    // Check if username equals password
    if ($fname === $pwd) {
        $errors[] = "Your name cannot be your password!";
    }

    // Check if email address exists in database
    $email_query = $pdo->prepare("SELECT * FROM users WHERE email = :email");
    $email_query->bindParam(':email', $email);
    $email_query->execute();
    $email_found = $email_query->fetchColumn();
    if ($email_found) {
        $errors[] = "Your email address is associated with another account.";
    }

    // If no errors, continue with user account creation
    if (!$errors)
    {
        // Hash password
        $hashed_password = password_hash($pwd, PASSWORD_DEFAULT);

        // Create database entry
        $create_account = $pdo->prepare("INSERT INTO users (first_name,email,password, activation_key) VALUES (:fname, :email, :password, :activation_key)");
        $create_account->bindParam(':fname', $fname);
        $create_account->bindParam(':email', $email);
        $create_account->bindParam(':password', $hashed_password);
        $create_account->bindParam(':activation_key', $activation_key);
        $create_account->execute();

        // Send out activation email
        $to=$email;
        $subject="Activate your account";
        $from = 'no-reply@example.com';
        $body='Thank you for creating your account, '.$fname.'. Please click on the following link to activate your account: '.$activation_link.'';
        $headers = "From:".$from;
        mail($to,$subject,$body,$headers);

        // Redirect user to the dashboard
        header("Location: /dashboard.php");
        exit;
    }

}

?>

I do want to use it on a live site at one point, so the end result should not just be a registration form that's somewhat secure but could actually be used in production.
EDIT
I did some more thinking and I guess before redirecting the newly registered user, I have to start a session to detect on the dashboard page whether the user is logged in. Is the following sufficient and secure to to add after I sent out the activation email?
session_start();

$_SESSION["loggedin"] = true;
$_SESSION["email"] = $email;

// Redirect user to the dashboard
header("Location: /dashboard.php");
exit;

You properly protect against SQL injection, that's good.
I worry about the filtering on the password. If I choose <script>' OR 1=1 ">>> as my password, the filtering should not reduce this to just ' OR 1=1 ", just because some part of it looks like HTML tags. A password is not supposed to be displayed anywhere, it is not supposed to be stored in plain text anywhere, the only valid use is to feed it to password_validate and password_hash, and these can cope with arbitrary input (well, except that they stop processing at the first byte that is 0, that is, the byte value 0, not the digit '0'). When I enter a password, I expect this password to be used exactly, unmodified, unnormalized, just as I entered it.
I dimly remember that uniqid is not very unique at all. You should double-check the documentation whether that function provides enough randomness to protect against guessing attacks.
Your code is sending out a mail containing <a href="...">, but nowhere in the code do I see a Content-type: text/html; charset=UTF-8 header.
Your code is sending out malformed HTML. The URL contains a plain &, which in HTML must be escaped as &amp;. To do this, call htmlspecialchars($url). Most browsers accept this malformed HTML, but it's better do know how to correctly convert between plain text and HTML. Just to avoid cross-site scripting from the beginning.
What if the $fname contains an ampersand? Some companies have that in their name. I don't know of actual people with these names, but who knows? An ampersand would make the generated $activation_link invalid.


Looks mostly fine, though there are a few issues:
Argument bug/typo You're referring to top-level variable entries inside the function instead of to the listAsString argument.
Match numbers? Even if it's said that the input will contain line-separated numbers, I'd feel safer matching numeric characters instead of splitting by newlines. It'll be more robust if the input happens to not be formatted perfectly correctly, or at least makes the intent (to extract an array of only digit characters from the input) a bit clearer, without relying on the description.
Maybe use Number? Once you have an array of digit characters, you turn them all into numbers with .map(s => s*1). I think the intent - to cast to numbers - would be a bit clearer by using .map(Number) instead.
i is conventionally used to indicate the index being iterated over. Here, i isn't being used as the index, but as the element being iterated over. Maybe use num instead?
target - i can be saved into a variable instead of repeating it twice, if you want.
Counting bug If the target sum happens to be a sum of two of the same number, eg: 1010 + 1010, those will be matched even if the input only contains one of those numbers, since you're using a Set which doesn't differentiate one occurrence from multiple.
To fix this, you could add an index check to the original array if the two numbers happen to be the same (as I've done below), or count up occurrences into an object, or something like that.

With findTargetSum3, in addition to the above:
Computational complexity is unnecessarily large The first approach is O(n), which is good. This second approach is O(n ^ 2), since you're checking every element against every other element. I'd ditch findTargetSum3 completely.
Prefer strict equality over loose equality - yes, the types are surely the same, so it doesn't make a difference in the logic, but strict equality with !== and === is easier to understand at a glance due to the lack of strange coercion rules. Whenever I see sloppy comparison (especially in professional code), I think: "Why isn't strict comparison being used, is there some possible type coercion weirdness going on that wasn't explicit?" Better to be consistent and use strict equality comparison everywhere.

function findTargetSum(listAsString, target){
  const numbers = listAsString.match(/\d+/g).map(Number);
  const set = new Set(numbers);
  for (const num of set) {
    const otherNum = target - num;
    if (
      set.has(otherNum) &&
      (num !== otherNum || numbers.indexOf(num) !== numbers.lastIndexOf(otherNum))
    ) {
       return num * otherNum;
    }
  }
}

const entries = `1780
1693
1830
1756
1858
1868
1968
1809
1996
1962
1800
1974
1805
1795
170
1684
1659
1713
1848
1749
1717
1734
956
1782
1834
1785
1786
1994
1652
1669
1812
1954
1984
1665
1987
1562
2004
2010
1551
961
1854
2005
1883
1965
475
1776
1791
262
1912
1227
1486
1989
1857
825
1683
1991
1875
1982
1654
1767
1673
1973
1886
1731
1745
1770
1995
1721
1662
1679
1783
1999
1889
1746
1902
2003
1698
1794
1798
1951
1953
2007
1899
1658
1705
62
1819
1708
1666
2006
1763
1732
1613
1841
1747
1489
1845
2008
1885
2002
1735
1656
1771
1950
1704
1737
1748
1759
1802
2000
1955
1738
1761
1765
1853
1900
1709
1979
1911
1775
1813
1949
1966
1774
1977
1757
1992
2009
1956
1840
1988
1985
1993
1718
1976
1078
1997
1897
1792
1790
1801
1871
1727
1700
1485
942
1686
1859
1676
802
1952
1998
1961
1844
1808
1703
1980
1766
1963
1849
1670
1716
1957
1660
1816
1762
1829
526
359
2001
1874
1778
1873
1511
1810
1699
1970
1690
1978
1892
1691
1781
1777
1975
1967
1694
1969
1959
1910
1826
1672
1655
1839
1986
1872
1983
1981
1972
1772
1760`;
console.log(findTargetSum(entries, 2020));
console.log(findTargetSum(`1
3
3
5`, 6));


Async/await and Task vs Task.Factory.StartNew and Result I am working on creating some libraries for a project at work and I wanted to make sure I have this pattern correct. Assuming that the GetWidgets method is what I am going to be exposing, which is the preferred method for this? Is there any fundamental difference in these two methods?

public async Task<List<Widget>> GetWidgets(DateTime date)
{
    using (var httpClient = new HttpClient())
    {
        var feed = "http://example.org/feed";
        var response = await httpClient.GetStringAsync(feed);
        var items = await SomeOtherAsyncMethod(response);

        return items.Where(item => item.StartDate.Date == date).ToList();
    }
}

public Task<List<Widget>> GetWidgets(DateTime date)
{
    return Task.Factory.StartNew(() =>
    {
        using (var httpClient = new HttpClient())
        {
            var feed = "http://example.org/feed";
            var response = httpClient.GetStringAsync(feed).Result;
            var items = SomeOtherAsyncMethod(response).Result;

            return items.Where(item => item.StartDate.Date == date).ToList();
        }
    });
}

And if the second example is correct, should I be naming it GetWidgetsAsync?

I'd prefer the first method because it's making more efficient use of resources (provided the async methods you call are really async and not just wrappers around synchronous methods) and it also looks cleaner.

To quote from this MSDN blog:

  ... the only asynchronous methods that should be exposed are those that have scalability benefits over their synchronous counterparts ...

What you are doing in your second method is basically calling the async methods in a synchronous way just to then in turn wrap it into a task. This will have scalability issues because it's using up a dedicated thread (at least in the current implementation of the task library) while the async methods you call might have more efficient means of achieving their asynchronicity (like IO completion ports or timer callbacks)


Find the common characters between two strings without using set operations Below are two implementations of the problem "find the characters in common between two strings" - in Clojure without using collections / set operations.

Are these approaches sufficiently idiomatic and could the be more concise? (aside from using collections functions).

Also, is there a Clojure library that wraps or provides an alternative to java.util.BitSet?

;; Approach #1 BitSets
;; - use a bitsets for each string,
;; - flip the bit for each character in each string
;; - then "and" the two bitsets. Theoretically O(N + M) performance

(defn string-to-charbits [s]
  (loop [[f & rest] s
          bits (java.util.BitSet.)]
    (if f (recur rest (do (.set bits (int f)) bits))
           bits)))

(defn process-bitset [bitset]
   (loop [bitset bitset
          count 0
          result '[]]
     (if (< count (.length bitset))
       (recur bitset
              (inc count)
              (if (.get bitset count) (conj result (char count)) result))
       result)))

(defn find-matching-chars [s1 s2]
  (let [b1 (string-to-charbits s1)
        b2 (string-to-charbits s2)]
    (do
      (.and b1 b2)
      (process-bitset b1))))

;; Approach #2 - Brute Force
;; For each character in string 1, linear search (via .indexOf) of string 2
;; Theoretically O(N * M) performance

(defn find-matching-chars-brute [s1 s2]
  (loop [[f & rest] s1
         s2 s2
         result []]
    (if f
      (if (> (.indexOf s2 (int f)) -1)
        (recur rest s2 (conj result f))
        (recur rest s2 result))
      (distinct result))))

;; Might need to sort the results of each function call to make this pass
(assert (= (find-matching-chars "abcd" "cde")
            (find-matching-chars-brute "abcd" "cde"))
         "expected [\\c \\d]")

With your restrictions, yes, that seems fairly idiomatic.  I am not aware of a good bitset implementation yet -- wrapping java.util.BitSet wouldn't work that well, since a clojure version would want to be persistent.  That said, your restrictions are not idiomatic at all.  Presumably, you don't want to use clojure.set/intersection (and completely trivializing the problem), but something like

(defn find-matching-chars [s1 s2]
  (distinct (filter (into #{} s1) s2)))

really would be far more idiomatic.  I could see using something like your BitSet code if find-matching-chars was actually causing significant performance issues, but short of that, some form of collections manipulation really is the idiomatic way to solve this issue.

In all honesty, my probable course in a situation like this would be to build filter and linked lists from scratch if I couldn't use the built-in versions, but at this point, I am probably straying away from idiomatic code myself.


Generator iterator with push back function In a compiler project for an LL(1/2) grammar, I have had a need for a generator iterator with a push back function. I have been surprised not to find any perfectly applicable solution (clean, simple, obvious, standard, etc.). On the other hand I was surprised of how easy it was to create exactly what I wanted after I decided to invest a little time.

Here is what I came up with:

class Back_pushable_iterator:
    """Class whose constructor takes an iterator as its only parameter, and
    returns an iterator that behaves in the same way, with added push back
    functionality.

    The idea is to be able to push back elements that need to be retrieved once
    more with the iterator semantics. This is particularly useful to implement
    LL(k) parsers that need k tokens of lookahead. Lookahead or push back is
    really a matter of perspective. The pushing back strategy allows a clean
    parser implementation based on recursive parser functions.

    The invoker of this class takes care of storing the elements that should be
    pushed back. A consequence of this is that any elements can be "pushed
    back", even elements that have never been retrieved from the iterator.
    The elements that are pushed back are then retrieved through the iterator
    interface in a LIFO-manner (as should logically be expected).

    This class works for any iterator but is especially meaningful for a
    generator iterator, which offers no obvious push back ability.

    In the LL(k) case mentioned above, the tokenizer can be implemented by a
    standard generator function (clean and simple), that is completed by this
    class for the needs of the actual parser.
    """
    def __init__(self, iterator):
        self.iterator = iterator
        self.pushed_back = []

    def __iter__(self):
        return self

    def __next__(self):
        if self.pushed_back:
            return self.pushed_back.pop()
        else:
            return next(self.iterator)

    def push_back(self, element):
        self.pushed_back.append(element)

def main():
    it = Back_pushable_iterator(x for x in range(10))

    x = next(it) # 0
    print(x)
    it.push_back(x)
    x = next(it) # 0
    print(x)
    x = next(it) # 1
    print(x)
    x = next(it) # 2
    y = next(it) # 3
    print(x)
    print(y)
    it.push_back(y)
    it.push_back(x)
    x = next(it) # 2
    y = next(it) # 3
    print(x)
    print(y)

    for x in it:
        print(x) # 4-9

    it.push_back(x)
    y = next(it) # 9
    print(x)

if __name__ == "__main__":
    main()

Any feedback is appreciated. A better name? ;-)

Note: I have already posted this code as a very late answer to https://stackoverflow.com/questions/2425270/how-to-look-ahead-one-element-in-a-python-generator, but got no feeback.

This is to expand on Austin Hastings comment.

According to Python Standard Library documentation,

  The intention of the protocol is that once an iterator’s next() method raises StopIteration, it will continue to do so on subsequent calls. Implementations that do not obey this property are deemed broken.

It means that once the underlying iterator raises StopIteration, your object shall not accept any more push backs.

I would call the list lookahead rather than pushed_back, but it is a matter of taste.

Otherwise, LGTM.


XLSX writer implementation We have multiple scripts which write into XLSX with the same format, into a different format or slide variation. I am trying to write an API which saves time for other programs, but I need input on restructuring the file.

import openpyxl
from xlsxwriter.workbook import Workbook

class xlsx_extend(object):
    """This class contain api of xlsx functionality"""
    def __init__(self, name):
        """create xls file"""
        self.xlsx_name = Workbook(name)
        self.create_format('blue', 12)

    def create_format(self, color, size):
        """write header"""
        self.xlsx_format = self.xlsx_name.add_format()
        self.xlsx_format.set_font_size(size)
        self.xlsx_format.set_bold()
        self.xlsx_format.set_color(color)
        self.xlsx_format.set_border()
        self.xlsx_format.set_center_across()
        self.xlsx_format.set_align('center')
        self.xlsx_format.set_rotation(90)

    def add_sheet(self, ws_name):
        """Adding worksheet name"""
        return self.xlsx_name.add_worksheet(ws_name)

    def rotate_text_format(self, angle):
        """rotate the text data"""
        self.xlsx_format.set_rotation(angle)

    def get_xlsx_name(self):
        """Returing xlsx name for derive class"""
        return self.xlsx_name

    def close_xlsx(self):
        """close xlsx File"""
        self.xlsx_name.close()

class xlsx_sheet:
    """This is xlsx worksheet class"""
    def __init__(self, excel_sheet, ws_name):
        """Initialze Base function and Create Worksheet"""
        self.excel_sheet = excel_sheet
        self.ws_name_worksheet = excel_sheet.add_sheet(ws_name)
        self.ws_name_worksheet.set_column(0, 0, 40)

    def write(self, row, col, data, format=None):
        """write Add in xlsx File"""
        self.ws_name_worksheet.write(row, col, data)

    def write_column(self, row, col, data, format=None):
        """write data in column than need pass data as list"""
        self.ws_name_worksheet.write_column(row, col, data)

    def write_row(self, row, col, data, format=None):
        """write data in row than need to pass as list"""
        self.ws_name_worksheet.write_row(row, col, data, self.excel_sheet.xlsx_format)

    def write_comment(self, row, col, data):
        """write comment in cell"""
        self.ws_name_worksheet.write_comment(row, col, data, {'width': 400, 'height': 400})

    def text_condition_format(self, first_row, first_col, last_row, last_col, value, format):
        """Adding text condition format"""
        self.ws_name_worksheet.conditional_format(first_row, first_col, last_row, last_col,
            {'type': 'text',
            'criteria': 'containing',
            'value': value,
            'format': format
            })

And another module with it's call API like:

report_xlsx = xlsxutils.xlsx_extend("report.xlsx")
red_format = report_xlsx.xlsx_name.add_format({'bg_color': '#FF0000'})
green_format = report_xlsx.xlsx_name.add_format({'bg_color':'#99CC32'})
gray_format = report_xlsx.xlsx_name.add_format({'bg_color':'#53868B'})
blue_format = report_xlsx.xlsx_name.add_format({'bg_color':'#3298FE'})
light_red_format = report_xlsx.xlsx_name.add_format({'bg_color':'#FF9999'})

report_worksheet = xlsxutils.xlsx_sheet(report_xlsx, "Report")

report_worksheet.text_condition_format(1, 1, row, col, 'P', green_format)
report_worksheet.text_condition_format(1, 1, row, col, 'F', red_format)
report_worksheet.text_condition_format(1, 1, row, col, 'AB', red_format)
report_worksheet.text_condition_format(1, 1, row, col, 'RU', light_red_format)
report_worksheet.text_condition_format(1, 1, row, col, 'QU', light_red_format)
report_worksheet.text_condition_format(1, 1, row, col, 'M', gray_format)
report_worksheet.text_condition_format(1, 1, row, col, 'UN', blue_format)
report_xlsx.close_xlsx()

Can you do code review and provide your input?

Naming Conventions

According to the official Python style guide, class names should be in PascalCase:

No:
    class my_class():
Yes:
    class MyClass():

In your case, I would capitalize the entire xlsx because it is an acroynm:

class XSLXExtend():

There also seems to be some redundancy in some of your names:

# worksheet_name_worksheet?
self.ws_name_worksheet

This next point may be speaking on an intentional design measure, but I would take a look at what properties are supposed to be 'public' and which are meant to be 'private'. Even though Python doens't truly have private variables, it is still convention to use a single underscore ('_') at the start of any intended private variables.

self._i_should_be_private = 'No touching!'

Some of your names are misleading:

# Wait. This stores the actual Workbook not its name?
self.xlsx_name = Workbook(name)

Structure

I am not a big fan of declaring instance variables outside of the __init__ method. In lieu of this, I would refactor your create_format() method as such:

def create_format(self, color, size):
    format = self.xlsx_name.addFormat()
    format.set_font_size(size)
    format.set_bold()
    format.set_color(color)
    format.set_border()
    format.set_center_across()
    format.set_align('center')
    format.set_rotation(90)

    return format

Then just put this line in the __init__ method: self.format = create_format('blue', 12).

Also, you have an accessor method (which is nice) for xlsx_name. However, in your code that actually uses your classes, you just directly access it:

# Why even have `get_xlsx_name()` if you don't use it?
red_format = report_xlsx.xlsx_name.add_format(...)

The same goes for your create_format() method. You use it once in __init__ but don't when you actually use your class. You skip straight to the Workbook:

red_format = report_xlsx.xlsx_name.add_format({'bg_color': '#FF0000'})

Essentially you are circumventing all the work you did wrapping the Workbook class in another class.

Since your xlsx_extend class is supposed to store data about and manipulate a Workbook why not store all of the formats there? Here is what I would do:

class XLSXExtension(object):
    def __init__(self, name):
        self.xlsx_name = Workbook(name)
        self.formats = {}

        self.create_format('blue', 'blue')

    def create_format(self, name, color, size=12):
        format = self.xlsx_name.addFormat()
        format.set_font_size(size)
        format.set_bold()
        format.set_color(color)
        format.set_border()
        format.set_center_across()
        format.set_align('center')
        format.set_rotation(90)

        self.formats[name] = format

    def __getattr__(self, key):
        try:
            return self.formats[key]
        except KeyError:
            print('Format "{}" not found.'.format(key))

report_xlsx = xlsxutils.xlsx_extend("report.xlsx")
report_xlsx.create_format('red', '#FF0000')
report_xlsx.create_format('green', '#99CC32', 24)

report_worksheet = xlsxutils.xlsx_sheet(report_xlsx, "Report")
report_worksheet.text_condition_format(1, 1, row, col, 'P', report_xlsx['green'])
report_worksheet.text_condition_format(1, 1, row, col, 'F', report_xlsx['red'])


MembershipProvider with Entity Framework and IoC I am using Castle Windsor as my IoC container and I registered it as a DependencyResolver to let the MVC framework know about it.

With the Entity Framework I have this DbContext:

public class MyDbContext : DbContext
{
    public DbSet<User> Users { get; set; }
}

In Castle Windsor it is registered as per web request:

container.Register(
    Component.For<DbContext, MyDbContext>()
             .ImplementedBy<MyDbContext>()
             .LifestylePerWebRequest()
);

Because I don't have control over creating the MyMembershipProvider object, this object is created once per web application so it is not possible to inject DbContext directly, because it will be disposed when the web request ends.

I wrote this as a solution:

public interface IDoInContext<TContext>
{
    void DoInContext(Action<TContext> action);
}

public class DoInMyDbContext : IDoInContext<MyDbContext>
{
    IKernel _kernel;

    public DoInMyDbContext(IKernel kernel)
    {
        _kernel = kernel;
    }

    public void DoInContext(Action<MyDbContext> action)
    {
        var context = _kernel.Resolve<MyDbContext>();
        action(context);
        _kernel.ReleaseComponent(context);
    }
}

And I registered like this:

container.Register(
    Component.For<IDoInContext<MyDbContext>>()
             .ImplementedBy<DoInMyDbContext>()
             .LifestyleSingleton()
);

Now I can create a MyMembershipProvider which will be able to interact with the current and correct DbContext every time it needs:

public class MyMembershipProvider : MembershipProvider
{
    IDoInContext<MyContext> db;

    public MyMembershipProvider()
        : this(DependencyResolver.Current.GetService<IDoInContext<MyContext>>())
    { }

    public MyMembershipProvider(IDoInContext<MyContext> db)
    {
        this.db = db;
    }

    public override bool ValidateUser(string username, string password)
    {
        bool result = false;
        db.DoInContext(x => {
            var encodedPassword = encodePassword(password);
            result = x.Users.Any(y => y.Login == username &&
                                      y.Password == encodedPassword);
        });
        return result;
    }

    ...
}

It seems to be stable and I can't see any memory leaks or any other problems. What do you think? Is the DoInContext(Action<TContext> action) thing any sort of pattern, anti-pattern or bad practice?

The preferred method of doing this is to use a factory dependency that creates the DbContext for you. So instead of the DbContext as the dependency you have a ContextFactory as the dependency and its consumers can get and manage a local DbContext by calling contextFactory.CreateContext();

If you insist on having your context injected directly, I believe it would be much better to have your MembershipProvider be registered per web request (even though you don't have control over creating it, you still have to register it right?). It shouldn't be a bottleneck. From a design and testing perspective this is much preferred to having a hard dependency on a service locator. How will you test the class by itself? Would you be incorporating your service locator into your tests?


2D convolution in matlab function    C = convolve_slow(A,B)
(file name is accordingly convolve_slow.m )
This routine performs convolution between an image A and a mask B.

Input:      A - a grayscale image (values in [0,255])
                    B - a grayscale image (values in [0,255]) serves as a mask in the convolution.
Output:     C - a grayscale image (values in [0,255]) - the output of the convolution.
                          C is the same size as A.

Method:  Convolve A with mask B using zero padding. Assume the origin of B is at
         floor(size(B)/2)+1.
         Do NOT use matlab convolution routines (conv,conv2,filter2 etc).
         Make the routine as efficient as possible: Restrict usage of for loops which
         are expensive (use matrix multiplications and matlab routines such as dot etc).

         To simplify and reduce ifs, you should pad the image with zeros before starting your convolution loop.
         Do not assume the size of A nor B (B might actually be larger than A sometimes).

here is my solution for this exercise. please elaborate on any change you have done or suggesting since i'm new to matlab and image processing.

function [ C ] = convolve_slow( A,B )

%This routine performs convolution between an image A and a mask B.
%
% Input:      A - a grayscale image (values in [0,255])
%             B - a grayscale image (values in [0,255]) serves as a mask in the convolution.
% Output:     C - a grayscale image (values in [0,255]) - the output of the    convolution.
%             C is the same size as A.
%
% Method:  Convolve A with mask B using zero padding. Assume the origin of B is at floor(size(B)/2)+1.

% init C to size A with zeros
C = zeros(size(A));

% make b xy-reflection and vector
vectB = reshape(flipdim(flipdim(B,1),2)' ,[] , 1);

% padding A with zeros
paddedA = padarray(A, [floor(size(B,1)/2) floor(size(B,2)/2)]);

% Loop over A matrix:
for i = 1:size(A,1)
    for j = 1:size(A,2)
        startAi = i;
        finishAi = i + size(B,1) - 1;
        startAj = j;
        finishAj = j + size(B,2) - 1;
        vectPaddedA = reshape(paddedA(startAi :finishAi,startAj:finishAj)',1,[]);
        C(i,j) = vectPaddedA* vectB;
    end
end

end

Rather than making four variables just to index the matrix, I would go for two in such a way:

paddedA(i :i_end,j:j_end)


Program that multiplies numbers over and over Would appreciate a review on this. The program does work as it is intended to. However I am unsure if there is a more elegant or better way to execute this process with less code.

def main():

    print ('This program takes two numbers and multiplies them.')
    print ('the result is multiplied by a third number')
    print ('entered by the user too')

    procedure()

def procedure():

    n = float(input('Enter a number: '))
    n2 = float (input('Enter a number: '))
    value = n * n2
    y = value
    print value

    n3 = float(input('Multiply by: '))

    while n3 != '0':

        op = n3 * y
        y = op
        print y

        n3 = float(input('Multiply by: '))

main()

Your program makes inconsistent use of Python 2 and 3. For 3.x you need print('Hello World!'), for 2.x you need print 'Hello World!'. I'm guessing that you are using 2x because you say your program works and print value (as above) only works in 2.x. Note that in 2.x, input(prompt) is equivalent to eval(raw_input(prompt)). In 3.x, input() now means raw_input() and raw_input() has been removed. Given 2.x, your code already eval()s input, no you don't need float(). However, float(raw_input()) is safer so I'll use that below. Also, the use of n3 != '0' never works because n3 is a float not a string. Finally, I've cleaned up the variable names and removed unnecessary assignments.

number1 = float(raw_input('Enter a number: '))
number2 = float(raw_input('Enter a number: '))
result = number1 * number2

print(result)

number3 = float(raw_input('Multiply by: '))

while not number3 == 0:
    result2 = number3 * result
    print result2

    number3 = float(raw_input('Multiply by: '))


Organize JavaScript for video player controls I have a small piece of code, and the problem is that I don't know how to organize it. There are tons of tutorials about JS code organization but I feel that those are for large scale apps. Also, I'm not sure if having the setInterval function where it is now is a big no no, can you come up with some critique/ideas?

YUI().use('node', function (Y) {
    // Access DOM nodes.

    var chapterTime = [0, 30, 49, 68, 90, 105, 127, 153, 182, 209],
        buttons = Y.one('ul'),
        currentChapter = 0,
        prevChapter = 0,
        myPlayer = videojs('example_video_1');

    buttons.delegate('click', function () {
        var button_name =  this.ancestor().get('id'),
            chapter = button_name.slice(1);

            playChapter(chapterTime[chapter-1], this);
    }, 'a');

    function playChapter(chTime, chButton) {
        myPlayer.play();
        setTimeout(function(){ myPlayer.currentTime(chTime); },100);
        Y.all('#nav a').setStyle('backgroundColor', "#999");
        chButton.setStyle('backgroundColor', '#666');
    }

    setInterval(function(){
        var playbackTime = myPlayer.currentTime(),
            i = 0,
            chapterFound = false;
            //console.log(chapterTime);

        while (i <= 9 && chapterFound == false){
            if (playbackTime >= chapterTime[i] && playbackTime < chapterTime[i+1]){
                currentChapter = i + 1;
                chapterFound = true;
            } else {
                i++;
            }
        }

        if (currentChapter != prevChapter){
            var button = Y.one('#c' + currentChapter);
            Y.all('#nav a').setStyle('backgroundColor', '#999');
            button.one('a').setStyle('backgroundColor', '#666');
            prevChapter = currentChapter;
        }
    },1000);
});

This looks just fine to me.

Some very minor nitpicks:

Remove //console.log(chapterTime);
Consider removing the blank line after var statements, it does not add much
chapterTime could use a comment as to what those numbers express (I assume seconds)
I would probably write this

buttons.delegate('click', function () {
    var button_name =  this.ancestor().get('id'),
        chapter = button_name.slice(1);

        playChapter(chapterTime[chapter-1], this);
}, 'a');

as

buttons.delegate('click', function () {
        var chapter = this.ancestor().get('id').slice(1) - 1;
        playChapter(chapterTime[chapter], this);
}, 'a');

The color strings #999 and #666 have a meaning, I assume enabled/disabled, you could use properly named constants for these, declared on top
Only seldom use double newlines, they are vertical overkill

Still, all in all I like your code.


Learning Ruby Caesar Cipher I am trying to learn Ruby and made a simple Caesar cipher program and I was wondering if the following code follows the 'ruby way' or the way most ruby users would do it

# Implements the classes caesar cipher encryption and decryption.
class CaesarCipher
  def initialize(shift)
    @shift = shift
  end

  def encrypt(str)
    str_array = str.split("")
    str_array.map! { |char| round_over(char) }.join
  end

  def decrypt(str)
    str_array = str.split("")
    str_array.map! { |char| round_down(char) }.join
  end

  private
  def round_over(char)
    if char.ord + @shift > "z".ord
      (char.ord + @shift - 26).chr
    elsif char.ord + @shift > "Z".ord && char.ord <= "Z".ord
      (char.ord + @shift - 26).chr
    else
      (char.ord + @shift).chr
    end
  end

  def round_down(char)
    if char.ord - @shift < "a".ord && /[[:lower:]]/.match(char) # checks for lowercase
      (char.ord - @shift + 26).chr
    elsif char.ord - @shift < "A".ord
      (char.ord - @shift + 26).chr
    else
      (char.ord - @shift).chr
    end
  end
end

That's a pretty good start, however I noticed that your code only works on letters. For non-alphabetic characters it returns confusing results e.g. encrypting and decrypting ! gives you ;. I can think of three good ways to handle non-alphabetic characters:

Ignore them
Use ASCII codes to shift them as well (so Z shifted right one becomes [)
Raise an error

Any of those approaches will ensure that decrypting and encrypting are inverses (which should be a property of any cipher!). That also leads you to a nice way to refactor your code: decrypting with a shift of 5 is the same thing as encrypting with a shift of -5. Try rewriting your code to look like this:

def encrypt(str)
  str.split('').map { |char| round(char, @shift) }.join
end

def decrypt(str)
  str.split('').map { |char| round(char, -@shift) }.join
end

private
def round(char, shift)
  # ...
end

Other than that, just a couple tips:

Ways to compare ranges of characters: /[a-z]/i =~ chr or (?A..?Z) === chr
What if @shift is greater than 26? You can use modulus % to account for shifts of any size


Determine if requested documents exist and if they do, whether they match requested revision number Is there a better way than using 2 for loops and 2 if statements deep?

jsfiddle:

https://jsfiddle.net/bobbyrne01/bmnk9bLz/

Code:

var documentsInDatabase = [{
  id: '1',
  revision: '1'
}, {
  id: '2',
  revision: '1'
}, {
  id: '3',
  revision: '1'
}];

// incorrect revision of document with id of 1, and non-existent document of id 4
var requestedDocuments = [{
  id: '1',
  revision: '2'
}, {
  id: '4',
  revision: '1'
}];

var result = {
  ids_not_found: [],
  revision_mismatches: []
};

// 4 nested blocks deep
for (var i = 0; i < documentsInDatabase.length; i++) {
  for (var j = 0; j < requestedDocuments.length; j++) {
    if (documentsInDatabase[i].id === requestedDocuments[j].id) {
      if (documentsInDatabase[i].revision !== requestedDocuments[j].revision) {
        result.revision_mismatches.push(requestedDocuments[j].id);
      }

      requestedDocuments.splice(j, 1);
      break;
    }
  }
}

for (var l = 0; l < requestedDocuments.length; l++) {
  result.ids_not_found.push(requestedDocuments[l].id);
}

document.getElementById('container').innerHTML = JSON.stringify(result, null, 2);

When you have code that starts to look like this, it's a sign that you need to write more functions to abstract some things away. You might put the code inside the two for loops into its own function, for example.


Ruby install script; packages+installs as a .deb or .rpm from source Is this bad practice?  Also, how can it be improved?

#!/usr/bin/env bash

RUBY_VERSION=2.1.0
printf "Installing Ruby $RUBY_VERSION\\n"

if [ -d ruby_build ]; then
    rm -Rf ruby_build
fi

if [[ `command -v ruby` && `ruby --version | colrm 11` == "ruby $RUBY_VERSION" ]] ; then
    echo "You already have this version of Ruby: $RUBY_VERSION"
    exit 113
fi

sudo apt-get build-dep -y ruby1.9.1

mkdir ruby_build && cd ruby_build
curl -O "http://ftp.ruby-lang.org/pub/ruby/2.1/ruby-$RUBY_VERSION.tar.gz"
tar xf "ruby-$RUBY_VERSION.tar.gz"
cd "ruby-$RUBY_VERSION"

./configure
make

sudo checkinstall -y --pkgversion "$RUBY_VERSION" --provides "ruby-interpreter" --replaces="ruby-1.9.2"
cd ../.. && rm -Rf ruby_build

First,I would like to point out that not all .deb-based distributions are as fond of sudo as Ubuntu is. For example, Debian doesn't use sudo out of the box.

It appears that you're trying to build a newer version of Ruby than is available in the stock package repository. In that case, why not make a proper .deb package with its .dsc and publish it in a private APT repository?  Then, everything integrates properly into the distribution the way package management is supposed to work, including the build process and subsequent updates.  Everyone will have a better experience when you work with the system instead of against it.


Reducing complexity of method I'm trying to reduce the complexity of some methods, and I'm not exactly sure what approach to take. I'm currently building a PHP wrapper for a REST API, and the main problematic class is here:

https://github.com/petrepatrasc/blizzard-starcraft-api/blob/master/Service/ApiService.php

In short, since data retrieved from the API is not always consistent, I have to do this:

$portrait = new Portrait();
$portrait->setXCoordinate(isset($apiData['portrait']['x']) ? $apiData['portrait']['x'] : null)
    ->setYCoordinate(isset($apiData['portrait']['y']) ? $apiData['portrait']['y'] : null)
    ->setWidth(isset($apiData['portrait']['w']) ? $apiData['portrait']['w'] : null)
    ->setHeight(isset($apiData['portrait']['h']) ? $apiData['portrait']['h'] : null)
    ->setOffset(isset($apiData['portrait']['offset']) ? $apiData['portrait']['offset'] : null)
    ->setUrl(isset($apiData['portrait']['url']) ? $apiData['portrait']['url'] : null);
return $portrait;

What would be a better way of handling the problem, so that the method complexity goes down? I suppose that I can try and catch an "undefined index exception", but I kind of wanted to see if there's something a bit more elegant that I can use.

You could remove some duplication with a getPortraitValue function:

function getPortraitValue($apiData, $key) {
    $portrait = $apiData['portrait'];
    if (isset($portrait[$key])) {
        return $portrait[$key];
    }
    return null;
}

$portrait = new Portrait();
$portrait->setXCoordinate(getPortraitValue($apiData, 'x')
    ->setYCoordinate(getPortraitValue($apiData, 'y'))
    ->setWidth(getPortraitValue($apiData, 'w'))
    ->setHeight(getPortraitValue($apiData, 'h'))
    ->setOffset(getPortraitValue($apiData, 'offset')
    ->setUrl(getPortraitValue($apiData, 'url');
return $portrait;


Distinguishing between people who have likes to posts and those who have dislikes I want to distinguish between people who have likes to posts and those who have dislikes. I want to find a way to improve my code without thinking it is efficient.

<% if current_user.voted_up_on? @post %>
  <%= link_to "LIKE #{@post.get_upvotes.size}", like_post_path(@post), method: :post, class: "btn btn-outline-primary" %>
  <%= link_to "DISLIKE #{@post.get_downvotes.size}", dislike_post_path(@post), method: :post, class: "btn btn-outline-dark" %>
<% elsif current_user.voted_down_on? @post %>
  <%= link_to "LIKE #{@post.get_upvotes.size}", like_post_path(@post), method: :post, class: "btn btn-outline-dark" %>
  <%= link_to "DISLIKE #{@post.get_downvotes.size}", dislike_post_path(@post), method: :post, class: "btn btn-outline-danger" %>
<% else %>
  <%= link_to "LIKE #{@post.get_upvotes.size}", like_post_path(@post), method: :post, class: "btn btn-outline-dark" %>
  <%= link_to "DISLIKE #{@post.get_downvotes.size}", dislike_post_path(@post), method: :post, class: "btn btn-outline-dark" %>
<% end %>

As  you can easily identify from your code that link_to code for Like and Dislike  are same for all conditions only the class value is changed
so you can use below code it looks more clean:

<% like_class = current_user.voted_up_on? @post ? 'btn-outline-primary' : 'btn-outline-dark' %>
<% dislike_class = current_user.voted_down_on? @post ? 'btn-outline-danger' : 'btn-outline-dark' %>
<%= link_to "LIKE #{@post.get_upvotes.size}", like_post_path(@post), method: :post, class: "btn #{like_class}" %>
<%= link_to "DISLIKE #{@post.get_downvotes.size}", dislike_post_path(@post), method: :post, class: "btn #{dislike_class}" %>


Words Counter using the revealing module pattern Is the following a good way to use the revealing module pattern?

$(document).ready(function() {

  var wordsCounter = (function() {

    function init() {
      bindEvents();
    }

    // cache dom
    var $textArea = $('#textArea');
    var $words = $('#words'); //a span
    var $chars = $('#chars'); //a span

    function bindEvents() {
      $textArea.on('keyup', updateWordsCount);
    }

    // $textArea keyup callback function
    function updateWordsCount() {
      var text =  $textArea.val();
      var numOfChars = text.length;
      var numOfWords = countWords(text);
      $chars.html(numOfChars);
      $words.html(numOfWords);
    }

    // counts words for a given string
    function countWords(str){
      var cnt = 0;
      var flag = true;
      for(var i = 0; i < str.length; i++){
        if(str[i] !== " " && flag){
          flag = false;
          cnt++;
        }else if(str[i] === " "){
          flag = true;
        }
      }
      return cnt;
    };

    return {
      init: init
    }

  }());

  wordsCounter.init();

});

What are other good ways I can structure this code?

Also if i'm working on a larger project, should it all be in a one IIFE like this or should I divide it into more than one IIFE each one doing related things?

function bindEvents() {
  $textArea.on('keyup', updateWordsCount);
}

// $textArea keyup callback function
function updateWordsCount() {
  var text =  $textArea.val();
  var numOfChars = text.length;
  var numOfWords = countWords(text);
  $chars.html(numOfChars);
  $words.html(numOfWords);
}

The handler is unnecessarily split from the code that binds it. It's not often you see an event handler split off from it's binding. In most cases, the handler code is very specific to the binding, it cannot be reused and therefore makes no sense splitting off. Inline the handler instead.

var $textArea = $('#textArea');
var $words = $('#words'); //a span
var $chars = $('#chars'); //a span

I would discourage the use of $ indicating that a variable is a jQuery instance. Whether it's a jQuery instance, or some other constructors instance, a variable is a variable regardless of what it's bound to.

Another problem is that this piece of code isn't reusable. The selector uses an ID. IDs are supposed to be unique. That means this code can only bind to 3 unique elements, and no more. If you want to make this word counter reusable, use a class.

var flag = true;

flag is a poorly named variable. It's not telling me what it's supposed to be, what it is for, and why it is there.

// counts words for a given string
function countWords(str){
  var cnt = 0;
  var flag = true;
  for(var i = 0; i < str.length; i++){
    if(str[i] !== " " && flag){
      flag = false;
      cnt++;
    }else if(str[i] === " "){
      flag = true;
    }
  }
  return cnt;
};

I think this is overcomplicated. A simple word count can be done by exploding the string into array by spaces, and getting the length of the resulting array. return str.split(' ').length what you are looking for.

For this simple task, a jQuery "plugin" would do. I would expect the plugin to accept an element containing your input and output elements, and a set of options asking for which elements are the input and output inside that group.

jQuery.fn.wordCounter = function(options){

  // For each found group
  return this.each(function(){

    // Look for the input and output under that group
    var input = $(options.input, this);
    var output = $(options.output, this);

    // Counter event and handler
    input.on('keyup', function(){
      output.text(input.val().split(' ').filter(text => text !== '').length);
    });

  });
};

$('.input-group').wordCounter({
  input: '.input-group__input',
  output: '.input-group__count',
});

  <input class="input-group__input">

In larger scale projects, consider using a small framework. vue.js, Ractive.js and riot.js are good candidates for quick-and-easy UI-related operations.


mapping & loops - counting up or counting down I am building a time tracking application and my requirement says that any hours past 40 total should be counted as overtime and not regular hours.

Which function is more clear? What can be done to clarify the function? Even the second one at ~ 10 lines seems wordy

Option #1

function calculateOvertime(days) {
    var totalHours = 0;
    var over40 = false;
    return $.map(days, function(hours) {
        var ot = 0;
        totalHours += hours;
        if (totalHours > 40 && !over40) {
            ot = totalHours - 40;
            hours = hours - ot;
            over40 = true;
        } else if (over40) {
            ot = hours;
            hours = 0;
        }
        return { reg: hours, ot: ot};
    })
}
calculateOvertime([16,16,16,16]);

Option #2

function calculateOvertime(days) {
    var hoursUntilOT = 40;
    return $.map(days, function(hours) {
        var ot = 0;
        hoursUntilOT -= hours;
        if (hoursUntilOT < 0) {
            ot = hoursUntilOT * -1;
            hours = hours - ot;
            hoursUntilOT = 0;
        }
        return { reg: hours, ot: ot};
    })
}
calculateOvertime([16,16,16,16]);

You can accomplish your task using only core Javascript, with no reference to JQuery code, in about that many lines. JQuery is overkill for this task.

I applied the following changes to make your code more readable:

Reduced knowledge required to understand it (removed JQuery).
Reduced the complex portions (see iCurrentOvertime).
Added Hungarian notation; i.e. prepending integers with "i", arrays with "a", etc.
Encased added values together within parentheses (e.g. iCurrentHours+iTotalHours becomes (iCurrentHours+iTotalHours)).

The new code is below:

function calculateOvertime(days) {
  var aHours = [];
  var iTotalHours = 0;
  for(var i=0;i<days.length;i++){
    var iCurrentHours = days[i];
    var iNewHours = (iCurrentHours+iTotalHours);
    var iCurrentOvertime = Math.max(0,(iNewHours - Math.max(iTotalHours,40)));
    iTotalHours = iNewHours;
    aHours.push({
      reg: (iCurrentHours - iCurrentOvertime),
      ot: iCurrentOvertime
    });
  }
  return aHours;
}
calculateOvertime([16,16,16,16]);


increase speed for 'for loop' I have files containing data set which contain 11,000 rows. I have to run a loop through each row, this is taking me 25minutes for each file.
I am using following code:

z <- read.zoo("Title.txt", tz = "")
for(i in seq_along(z[,1])) {
  if(is.na(coredata(z[i,1]))) next
  ctr <- i
  while(ctr < length(z[,1])) {
    if(abs(coredata(z[i,1])-coredata(z[ctr+1,1])) > std_d) {
      z[ctr+1,1] <- NA
      ctr <- ctr + 1
    } else {
      break
    }
  }
}

Where "Title.txt" is file containing 11,000 rows. It looks like (first five rows):

"timestamp" "mesured_distance" "IFC_Code" "from_sensor_to_river_bottom"
 "1" "2012-06-03 12:30:07-05" 3188 1005 3500
 "2" "2012-06-03 12:15:16-05" 3189 1005 3500
 "3" "2012-06-03 12:00:08-05" 3185 1005 3500
 "4" "2012-06-03 11:45:11-05" 3191 1005 3500
 "5" "2012-06-03 11:30:15-05" 3188 1005 3500

I wish to receive help on how should I increase the speed for this code?

Here is how the code works:

    set.seed(100)
    x=rnorm(15)
    std_d=sd(x)

afer running code. It gives this:

    m
  [1] -0.50219235          NA -0.07891709          NA  0.11697127  0.31863009            NA
  [8]  0.71453271          NA          NA          NA          NA          NA  0.73984050
 [15]          NA

It replaces the next element(e2) with NA if the subtraction(e1-e2)  is > std_d and then checks e1 with e3 if (e1-e3) is > std_d then it replaces e3, if it was < std_d then it
would check e3-e4 and so on.

Revised: ...I wonder if this could replace those two loops? Here are the results with the test vector offered in your comments:

set.seed(100);  m=rnorm(15); std_d <- sd(m)
Reduce(function(y,xx){ if( abs(tail(y[!is.na(y)], 1) - xx) > std_d ) {c(y,NA)}else{
                                                                        c(y,xx)} },
          m )
 [1] -0.50219235          NA -0.07891709          NA  0.11697127  0.31863009          NA  0.71453271
 [9]          NA          NA          NA          NA          NA  0.73984050          NA

Here's original test case with revised results. It is now comparing the last non-NA value to the current one and making NA if it exceeds the standard value. That other solution looked at the running sd() but your code did not do that so I just used a global sd value:

> z <- read.zoo(text=' "timestamp" "mesured_distance" "IFC_Code" "from_sensor_to_river_bottom"
 "1" "2012-06-03 12:30:07-05" 3188 1005 3500
 "2" "2012-06-03 12:15:16-05" 3189 1005 3500
 "3" "2012-06-03 12:00:08-05" 3185 1005 3500
 "4" "2012-06-03 11:45:11-05" 3191 1005 3500
 "5" "2012-06-03 11:30:15-05" 3188 1005 3500', tz = "")
 std_d <- sd(coredata(z[,1]) )
 zx <- as.numeric(coredata(z[,1]))
 coredata(z[,1]) <- Reduce(function(y,xx){
                              if( abs(tail(y[!is.na(y)], 1) - xx) > std_d ) {
                                        c(y,NA)} else {
                                        c(y,xx)} },
                                  zx )
 z
                    mesured_distance IFC_Code from_sensor_to_river_bottom
2012-06-03 11:30:15             3188     1005                        3500
2012-06-03 11:45:11               NA     1005                        3500
2012-06-03 12:00:08               NA     1005                        3500
2012-06-03 12:15:16             3189     1005                        3500
2012-06-03 12:30:07             3188     1005                        3500


Binary search optimization: Kernighan & Ritchie 3-1 The problem I am trying to solve can be described as take binarysearch0 and rewrite it as binarysearch1 such that binarysearch1 uses a single test inside the loop instead of two. The code I used to do this is seen below. Please let me know if you think I accomplished the task, and what I can do better.

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

void printdata(int *v, int n)
{
    int i = 0;
    for (i = 0; i < n; ++i) {
        printf("%3d", v[i]);
    }
    putchar('\n');
}

int binarysearch0(int x, int *v, int n)
{
    int low, high, mid;

    low = 0;
    high = n - 1;
    while (low <= high) {
        mid = (low + high) / 2;
        if ( x < v[mid])
            high = mid -1;
        else if (x > v[mid])
            low = mid + 1;
        else
            return mid;
    }
    return -1;
}

int binarysearch1(int x, int *v, int n)
{
    int low, high, mid;

    low = 0;
    high = n - 1;
    while (low <= high && v[(mid = (low + high) / 2)] != x ) {
        mid = (low + high) / 2;
        if ( x < v[mid])
            high = mid -1;
        else
            low = mid + 1;
    }
    return (v[mid] == x) ? mid : -1;
}

int main(int argc, char *argv[])
{
    int v[10][10] = { { 0, 1, 2, 3, 4, 5, 6, 7, 8, 9},
                      {10,11,12,13,14,15,16,17,18,19},
                      {20,21,22,23,24,25,26,27,28,29},
                      {30,31,32,33,34,35,36,37,38,39},
                      {40,41,42,43,44,45,46,47,48,49},
                      {50,51,52,53,54,55,56,57,58,59},
                      {60,61,62,63,64,65,66,67,68,69},
                      {70,71,72,73,74,75,76,77,78,79},
                      {80,81,82,83,84,85,86,87,88,89},
                      {90,91,92,93,94,95,96,97,97,99}
                    };

    int i = 0;
    for (i = 0; i < 10; ++i) {
        int search = 0;
        (i == 0) ? (search = 1000) : (search = v[i][0] + rand()%10);
        if(binarysearch1(search, v[i], 10) > -1) {
            printf("%d was found in the data: %14s", search, " ");
            printdata(v[i], 10);
        } else {

            printf("%d was NOT found in the data: %10s", search, " ");
            printdata(v[i], 10);
        }
        putchar('\n');
    }
    return 0;
}

You're not really changing the number of comparisons inside the loop here: all you've done is move it from the loop body to the loop termination condition. Note that in many situations, comparisons on the array elements (which could be strings or more complex objects) are a lot costlier than comparisons on the indices.

You can find several solutions to this exercise on the clc-wiki. I think one of them (Paul Griffith's) misses the point (it's pretty much the same as yours); I'm going to show  explain the reasoning behind Andrew Tesker and Colin Barker's solutions.

In the original function binsearch, inside the loop, there are three possible situations: x<v[mid], x==v[mid] and x>v[mid]. Sometimes, a comparison function has three outcomes: less than, equal or greater than; in such cases, the approach in the original function is best. For example, if we were comparing strings with strcmp:

    int cmp = strcmp(x, v[mid]);
    if (cmp < 0) high = mid - 1;
    else if (cmp > 0) low = mid + 1;
    else return mid;

In other cases, the only primitive you have is a less-than or less-or-equal-to, and to test for equality requires a second comparison. Thus the method presented in K&R requires two calls to the comparison function per iteration through the loop. It is more efficient to require a single comparison. Now we absolutely need to distinguish between the less-than and the greater-than case, so this means we're not going to be able to deal with the equality case inside the loop. So we'll keep going either up or down in the equality case. Since we haven't vetted the value v[mid] in one of the cases, we need to keep it inside the search space.

    if (x < v[mid]) high = mid - 1;
    else low = mid;

What happens to the loop termination? Now, we can get stuck with low == high and x >= v[low]. However, this is easily resolved by ending the loop when low == high.

while (low < high) {
    mid = (low + high + 1) / 2;
    if (x < v[mid]) high = mid - 1;
    else low = mid;
}

Note that I set mid to low + high + 1. In the case where low + 1 == high, (low + high) / 2 is low, and if x is still too small we'd set mid to low and loop forever (thanks to Elyasin for noticing that my original answer had this bug). Instead in this case we arrange for mid to be set to high. If low <= high we exit the loop anyway, and in other cases we'll continue looping and this change only causes the middle to be rounded up instead of down.

At the exit of the loop, if there is a matching element in the array, then v[low] is equal to it. So we perform a final equality test:

return x == v[low] ? low : -1;

The number of element comparisons performed by the original version is anywhere from 1 to 2*ceil(log₂(n)+1) where n is the number of elements in the array. The number of element comparisons in the version with a single comparison within the loop is always ceil(log₂(n))+2. That's a smaller maximum, but not necessarily a gain, because the original version will terminate earlier if it finds the element. The original version is preferable when there is a good chance that the element will be in the array. The modified version is preferable when the element is rarely present.


Compare strings by custom sort order Each letter represent a category. Example the group with "M" Code is greater Than "I" and "C" group. "F" is the higher group

Here is what I did but I wonder if there is a better way to do it.

 Private Function CompareCode(ByVal code As String, ByVal otherCode As String) As Boolean
    Dim orderedCode As New List(Of String)
    orderedCode.Add("I")
    orderedCode.Add("C")
    orderedCode.Add("M")
    orderedCode.Add("F")

    Return orderedCode.IndexOf(code) < orderedCode.IndexOf(otherCode)

End Function

Naming

The name CompareCode does not express ll what this method is doing. Try to name it more meaningful.

General

You don't need a List(Of String) here. Add a private const string like

Private Const orderedCode As String = "ICMF"
Private Function CompareCode(ByVal code As String, ByVal otherCode As String) As Boolean
    Return orderedCode.IndexOf(code) < orderedCode.IndexOf(otherCode)
End Function


Number guessing game in Python I'm learning python and created a number guessing game on which I would like to get your feedback on the structure and style of the program.

import random

LOWER_NUMBER = 1
HIGHEST_NUMBER = 10

def print_header():
    print("-" * 40)
    print("Welcome to the number guessing game")
    print("-" * 40)

def user_input():
    """Collect and validate the entries made by the users

    Returns
    -------
    int
        Gives back the validated number entered by the user.
    """
    while True:
        try:
            guess = int(
                input(f'Enter your Guess of a number between '
                      f'{LOWER_NUMBER} - {HIGHEST_NUMBER}: '))
            if guess < LOWER_NUMBER or guess > HIGHEST_NUMBER:
                print("The entered number is out of range, try again.")
                continue
        except ValueError:
            print('you did not enter a number, please try again.')
            continue
        else:
            break
    return guess

def generate_number_to_guess(lower_number, higher_number):
    """Generates a random number between the given lowest and highest number

    Parameters
    ----------
    lower_number : int
        Lowest number for the generator

    higher_number : int
        highest number for the generator

    Returns
    -------
    int
        returns the generated random number that is in the given range.
    """
    return random.randint(lower_number, higher_number)

def play_again():
    """Perform and validate entry for a next game

    Returns
    -------
    string
        returns the validated user entry for a next game.
    """
    while True:
        new_game = input(
            "Would you like to play again? [y]es/[n]o: ")
        if new_game.lower() not in ["n", "y"]:
            print('Wrong entry please use y or n')
            continue
        else:
            break

    return new_game.lower()

def start_game():
    """This is the main loop that runs the app.
    """
    highscore = 0

    while True:
        print_header()
        number_to_guess = generate_number_to_guess(LOWER_NUMBER,
                                                   HIGHEST_NUMBER)
        guess = 0
        count = 0

        while guess != number_to_guess:
            guess = user_input()
            count += 1
            if guess < number_to_guess:
                print("It's higher")
            elif guess > number_to_guess:
                print("It's lower")
        else:
            print(
                f'\nYou geussed the right number and needed {count} tries')
            if count < highscore or highscore == 0:
                highscore = count

            # validate the input for a new game
            another_game = play_again()
            if another_game == 'y':
                print(f"\n\nThe HIGHSCORE is {highscore}")
                continue
            elif another_game == 'n':
                print("\n Thanks for playing, see you next time!")
                break

if __name__ == "__main__":
    start_game()

There is a lot of things done right in your program. The code is structured into functions, there are docstrings, you validate user input and you even have a main guard. Still there is room for improvement.

code duplication

This is a minor issue in your code but I do not want to skip it. In

def print_header():
    print("-" * 40)
    print("Welcome to the number guessing game")
    print("-" * 40)

which could read

def print_header():
    dashed_line = "-" * 40
    print(dashed_line)
    print("Welcome to the number guessing game")
    print(dashed_line)

The reason for not duplicate code (even if it is less code) is maintenance. If you ever chose to increase the line length there is a single place to edit.
In your case the chance to miss the other number is low. But keep that in mind.

naming of functions and variables

user_input() is not the best name as there is many user interaction. A better name would be get_user_guess().
Also play_again() does not play but getting user input again. Might be get_user_play_again().

overdone design

Your function

def generate_number_to_guess(lower_number, higher_number):
    """Generates a random number between the given lowest and highest number

    Parameters
    ----------
    lower_number : int
        Lowest number for the generator

    higher_number : int
        highest number for the generator

    Returns
    -------
    int
        returns the generated random number that is in the given range.
    """
    return random.randint(lower_number, higher_number)

does nothing but calling a standard(!) function with the same parameter signature. Instead call the standard function directly.
Such detors are not good in terms of maintainability. Every reasonably experienced python programmer does know randint().
If reading that in a code he exactly knows what's happening. If he stumbles onto a call of your function he must immediately look upon your implementation to get to know what it does.
He must not trust your docstring (between?!) but read the code. He is even forced to write unittests if they are not existing and there are some lines of implementation.
In your case it is a pure delegation to a standard function, so the reviewer is happy to find a reliable implementation. But he lost time for no reason.
Do not do this.

Beside that issues your code is very maintainable! It is structured and readable. The complexity of your functions is reasonable. Be very careful with docstrigs.
Your docstring starts with

"""Generates a random number between the given lowest and highest number

while randint.__doc__ returns

'Return random integer in range [a, b], including both end points.\n        '

This is 'between' vs 'including both endpoints'.

global constants

Simply try to avoid them, especially if they are not naturally constant. You nearly did it right in the superfluous function generate_number_to_guess where you decided to pass tarameters instead of accessing the globals directly.
However you stopped early on the process of passing. If you also pass the variables to start_game()

def start_game(lower_number, higher_number):

and call the generator

    number_to_guess = generate_number_to_guess(lower_number,
                                               higher_number)

you are done with that path. Now you also pass these numbers to user_input()

def user_input(lower_number, higher_number):

and eliminate the globals in there.

You end up with shouting variable names (no global constants)

LOWER_NUMBER = 1
HIGHEST_NUMBER = 10

# [...]

if __name__ == "__main__":
    start_game()

which you change to

if __name__ == "__main__":
    start_game(1, 10)

If a value is not globally constant for a function - pass it as a parameter.
The function is then loosly coupled to the rest and is perfectly testable.


Finding the minimum in a sorted, rotated array Given an array that is first sorted non-decreasing and then rotated
  right by an unspecified number of times, find the index of its minimal
  element efficiently. If multiple such minimal elements exist, return
  the index of any one.

  Idea: Conceptually, divide the array into two parts: the "larger"
  subpart (to the left) which consists of large numbers brought here
  from the extreme right by rotation, and the "smaller" subpart which
  starts with the smallest element. We can always tell in which part we
  are, and move left/right accordingly.

Notes: When the array has multiple minimal elements, the index of the leftmost one in the "right" subpart is returned.

int getMinimIndex (const int *const a, size_t left, size_t right)
{
        assert(left<= right);

        // Special cases:
        // 1 If left & right are same , return
        if (left ==right)
                return left;

        // 2 For an array of size 2, return the index of minimal element
        if (left == right - 1)
                return a[left]<=a[right]?left:right;

        // 3 If the array wasn't rotated at all,
        if (a[left] < a[right])
                return left;

        // General case
        // Go to the middle of the array
        size_t mid = (left + right) >> 1;

        // If we stepped into the "larger" subarray, we came too far,
        // hence search the right subpart
        if (a[left] <= a[mid] )
                return getMinimIndex(a, mid, right);
        else
        // We're still in the "smaller" subarray, hence search left subpart
                return getMinimIndex(a,left, mid);
}

Unit tests:

\#define lastIndex(a)  ((sizeof(a)/sizeof(a[0]))-1)

int main()
{
        int a1[] = {7,8,9,10,11,3};
        int a2[] = {1};
        int a3[] = {2,3,1};
        int a4[] = {2,1};
        int a5[] = {2,2,2,2,2};
        int a6[] = {6,7,7,7,8,8,6,6,6};
        int a7[] = {1,2,3,4};

        printf("\n%d", getMinimIndex(a1,0, lastIndex(a1))); // 5
        printf("\n%d", getMinimIndex(a2,0, lastIndex(a2))); // 0
        printf("\n%d", getMinimIndex(a3,0, lastIndex(a3))); // 2
        printf("\n%d", getMinimIndex(a4,0, lastIndex(a4))); // 1
        printf("\n%d", getMinimIndex(a5,0, lastIndex(a5))); // 3
        printf("\n%d", getMinimIndex(a6,0, lastIndex(a6))); // 6
        printf("\n%d", getMinimIndex(a7,0, lastIndex(a7))); // 0

        return 0;

}

The algorithm does not work correctly.

It fails on the following:

int a1[] = {10,1,10,10,10,10,10};

printf("\n%d", getMinimIndex(a1,0, lastIndex(a1))); // 5

It prints 5 instead of 1.

The problem is in case the elements can repeat, you assumption that we can recurse on the left or right half is wrong.

In fact, if elements can repeat, any algorithm will be in the worst case Omega(n), while yours is always O(logn), so it is incorrect.


Securely Delete All Drive Data When giving away or selling a PC, one should be careful not to send personal data with it. I'm in such a situation and was using dd if=/dev/urandom of=/dev/sdb bs=8M, but it was only averaging 22MB/s. Way too slow since I have to drop it at the post office next morning. So I decided to write my own and see what kinds of results I would get.

First I dropped /dev/urandom in favor of hashing a secret key and feeding back the output as state. The second thing I did was to use O_DIRECT to avoid buffering.

My program reports an average of 48MiB/s, much better than what I got with plain old dd unless it has a bug.

#define _GNU_SOURCE 1

#include <stdio.h>
#include <stdlib.h>
#include <sys/types.h>
#include <sys/stat.h>
#include <fcntl.h>
#include <time.h>
#include <stdint.h>
#include <string.h>
#include <unistd.h>
#include <openssl/sha.h>

#define BLOCK_SIZE (1024 * 1024 * 8)
#define ALIGNMENT 512

typedef struct {
  unsigned char state[64];
  unsigned char key[32];
  __uint128_t counter;
} RandomCtx;

static void _xor64(uint64_t* a, uint64_t* b)
{
  for(int i = 0; i < 8; ++i)
    *a++ ^= *b++;
}

static void _cipher(RandomCtx* ctx, unsigned char* data, unsigned int len)
{
  unsigned int runs = len >> 6;
  while(runs){
    // get value to XOR
    unsigned char x[64];
    SHA512((unsigned char const*)ctx, sizeof(RandomCtx), x);

    // XOR with data
    _xor64((uint64_t*)data, (uint64_t*)x);

    // feed the XORed result back as state and update counter
    memcpy(ctx->state, data, 64);
    ++ctx->counter;

    // move on
    data += 64;
    --runs;
  }
}

int main(int argc, char* argv[])
{
  if(2 != argc){
    fprintf(stderr, "Usage: %s /dev/sdb\n", argv[0]);
    return EXIT_FAILURE;
  }

  // open disk
  int disk = open(argv[1], O_WRONLY|O_DIRECT);
  if(-1 == disk){
    perror("open()");
    return EXIT_FAILURE;
  }

  // allocate memory enough
  char* raw = malloc(BLOCK_SIZE + ALIGNMENT - 1);
  if(!raw){
    perror("malloc()");
    return EXIT_FAILURE;
  }

  // align
  unsigned char* aligned = (unsigned char*)
                           ((size_t)(raw + (ALIGNMENT-1)) & ~(ALIGNMENT-1));

  // open random source
  int urandom = open("/dev/urandom", O_RDONLY);
  if(-1 == urandom){
    perror("open()");
    return EXIT_FAILURE;
  }

  // init random ctx
  RandomCtx ctx;
  if(sizeof ctx != read(urandom, &ctx, sizeof ctx)){
    perror("read()");
    return EXIT_FAILURE;
  }

  // init buffer
  if(BLOCK_SIZE != read(urandom, aligned, BLOCK_SIZE)){
    perror("read()");
    return EXIT_FAILURE;
  }

  // we are done with urandom
  close(urandom);

  // now run until there's no more space
  time_t ti0 = time(NULL);
  size_t to0 = 0;
  size_t to1 = 0;
  int gib = 0;
  while(BLOCK_SIZE == write(disk, aligned, BLOCK_SIZE)){
    to1 += BLOCK_SIZE;
    if((to1 - to0) >= (1024*1024*1024)){
      to0 = to1;
      ++gib;
      size_t secs = time(NULL) - ti0;
      printf("Total: %d GiB in %zu secs. %.2f MiB/s\n", gib, secs,
             (double)gib*1024/secs);
    }

    _cipher(&ctx, aligned, BLOCK_SIZE);
  }
}

If you actually need to securely wipe your non-SSD drive consider using
shred, or look at some
of the linked pages too.

Also I don't see much point in the pseudo-random data generation given
that you just want to make the previous content unrecoverable.  If you
a want faster program you could probably use a faster hash too.

For the code I only want to note that write might write less than the
specified number of bytes and that's not immediately an error.


Manually writing a function matrixpower() in R Problem 1 - Matrix powers in R

  R does not have a built-in command for taking matrix powers.

  Write a function matrixpower() with two arguments mat and k that  will
  take integer powers k of a matrix mat.

My attempted solution.

matrixMul <- function(mat1, mat2)
{
    rows <- nrow(mat1)
    cols <- ncol(mat2)

    if(rows == cols)
    {
        matOut <- matrix(nrow = rows, ncol = cols) # empty matrix

        for (i in 1:rows)
        {
            for(j in 1:cols)
            {
                vec1 <- mat1[i,]
                vec2 <- mat2[,j]

                mult1 <- vec1 * vec2

                matOut[i,j] <- sum(mult1)
            }
        }

        return(matOut)
    }
    else
    {
        return (matrix( rep( 0, len=25), nrow = 5))
    }
}

matrixpower<-function(mat1, k)
{
    pow1 <- abs(k)

    rows <- nrow(mat1)
    cols <- ncol(mat1)

    matOut <- diag(1, rows, cols)

    if(pow1 == 0)
    {
        return(matout)
    }
    if (pow1 == 1)
    {
        matOut <- mat1
    }

    if(pow1 > 1)
    {
        for (i in 1:pow1)
        {
            matOut <- matrixMul(matOut, mat1)
        }
    }

    if(k < 0)
    {
        matOut <- solve(matOut)
    }

    return (matOut)
}

mat1 <- matrix(c(1,2,3,4), nrow = 2, ncol=2)
pow1 <- matrixpower(mat1, -2)
pow1

I have a few questions here.

Is there any room for improvement in this code?
Does this problem ask for manual implementation of multiplication? Or, should the use of %*% suffice?

It would be more efficient to use binary exponentiation.

Suppose that you want to raise an n by n matrix to the \$k^{th}\$ power.

The current method requires that you do a normal matrix multiplication \$k-1\$ times.

This of course has complexity \$k\$ multiplied by \$f(n)\$, Where \$f(n)\$ is the complexity of a matrix multiplication ( This is usually \$n^ 3\$ unless you write some fancy shmancy code that no one writes)

If you use binary exponentiation you reduce the number of multiplications to \$log(k)\$. Specifically the number of multiplications is the ceiling of \$log_2(k)\$ plus the number of 1 bits in the binary representation of \$k\$.

This is of course much more efficient for large values of \$k\$.


Blackjack game - follow-up This is an updated version of my previously posted "Blackjack game made in Python 3". I think I did almost everything @Austin Hastings recommend me to do and I hope you like it.

from random import shuffle
import os

def shuffled_shoe():
    shoe = ['2', '3', '4', '5', '6', '7', '8', '9', '10', 'A', 'J', 'Q', 'K']*4
    shuffle(shoe)
    return shoe

def deal_card(shoe, person, number):
    for _ in range(number):
        person.append(shoe.pop())

def deal_hand(shoe, player, dealer):
    deal_card(shoe, player, 2)
    deal_card(shoe, dealer, 2)

def score(person):
    non_aces = [c for c in person if c != 'A']
    aces = [c for c in person if c == 'A']
    total = 0
    for card in non_aces:
        if card in 'JQK':
            total += 10
        else:
            total += int(card)
    for card in aces:
        if total <= 10:
            total += 11
        else:
            total += 1
    return total

def display_info(player, dealer, player_stands):
    os.system('cls' if os.name == 'nt' else 'clear')
    print("Your cards:   [{}] ({})".format("][".join(player), score(player)))
    if player_stands:
        print("Dealer cards: [{}] ({})".format("][".join(dealer), score(dealer)))
    else:
        print(f"Dealer cards: [{dealer[0]}][?]")

def hit_or_stand():
    while True:
        print("What do you choose?")
        print("[1] Hit")
        print("[2] Stand")
        ans = input("> ")
        if ans in '12':
            return ans

def player_play(shoe, player, dealer, player_plays, dealer_plays, player_stands):
    while not player_stands:
        if hit_or_stand() == '2':
            player_plays = False
            dealer_plays = True
            player_stands = True
            display_info(player, dealer, True)
        elif not player_stands:
            deal_card(shoe, player, 1)
            display_info(player, dealer, False)
            if score(player) >= 21:
                player_plays = False
                break
    return (player_plays, dealer_plays, player_stands)

def dealer_play(shoe, dealer, DEALER_MINIMUM_SCORE, player):
    while score(dealer) <= DEALER_MINIMUM_SCORE:
        deal_card(shoe, dealer, 1)
    display_info(player, dealer, True)
    return False

def results(player, dealer, player_stands, first_hand, still_playing):
    if score(player) == 21:
        print("Blackjack! You won")
        still_playing = False
    elif first_hand and score(dealer) == 21:
        print("Dealer got a blackjack. You lost!")
        still_playing = False
    elif score(player) > 21:
        print("Busted! You lost!")
        still_playing = False
    if player_stands:
        if score(dealer) > 21:
            print("Dealer busted! You won")
        elif score(player) > score(dealer):
            print("You beat the dealer! You won!")
        elif score(player) < score(dealer):
            print("Dealer has beaten you. You lost!")
        else:
            print("Push. Nobody wins or losses.")
        still_playing = False
    return still_playing

def main():
    shoe = shuffled_shoe()
    player = []
    dealer = []
    player_plays = True
    still_playing = True
    dealer_plays = False
    player_stands = False
    deal_hand(shoe, player, dealer)
    display_info(player, dealer, player_stands)
    still_playing = results(player, dealer, player_stands, True, still_playing)
    while still_playing:
        while player_plays:
            (player_plays, dealer_plays, player_stands) = player_play(shoe, player, dealer, player_plays, dealer_plays, player_stands)
        while dealer_plays:
            dealer_plays = dealer_play(shoe, dealer, 17, player)
        still_playing = results(player, dealer, player_stands, False, still_playing)

if __name__ == '__main__':
    main()

First; I agree with the points made by @Austin. I like it that your code is written such that it is easy to read (which I find important).
However, I find the structure of the program a bit hard to follow. For example; there are two display functions (display_info and result). I would merge these or at the least group them together.

In Python there seem to be two schools: 1) put everything in a function (as a means of documentation), 2) create functions to aid "separation of concerns" and add comments for documentation. My guess is that @Austin is more a type-1 person and I am more a type-2 person. I would suggest to add comments (especially to document functions and function arguments).

These are some observations from my side:

a lot of functions take both the player as the dealer as argument, while their actions are similar. It could be benificial to create a more generic function for an action (stand/hit) and call that function separately. For example: create a function which asks the action of the dealer and a function asking the action of the player.
the dealer_play function always returns False. So I guess you can get rid of the return argument, as well as the 'while dealer_plays:' loop.
the results function seems overly complex to me. I would not call the function at all as long as the dealer or the player are still playing. This would remove the need to keep track of who is still playing. Print the intermediate scores separately and evaluate the winner after the game.
there is no need to provide a boolean for first_hand. Each player and dealer always start with two cards. So len(cards)==2 gives the same information. For readability it could be good to perform this action at the start of the function (first_hand = len(dealer) == 2), as this way there is no need to change the code too much and you remove the burden of keeping track of this variable from the function-caller.

There is one other point that caught my eye: you are mixing different string formatting methods (''.format() vs f-strings). I would choose one of the methods and stick with that.

All these comments might seem like I do not like your code. This is not the case, my aim is to point out some possible improvements.


Building an email body So here we are, using a StringBuilder to build an email body. I know there's a lot on StackOverflow and endless debate/discussion around how best to concatenate strings.

Can this be done in fewer lines of code in C#?
Can this be done better using templating?

My goals are readability and maintainability.

StringBuilder msgBody = new StringBuilder();
msgBody.AppendLine("Deleted Order Item Information");
msgBody.AppendLine("------------------------------------");
msgBody.AppendLine();
msgBody.AppendLine("Delete Date:    {0}".FormatWith(orderDetail.DeletedDate.FormatDateTime(dtFormat)));
msgBody.AppendLine("Delete Comment: {0}".FormatWith(orderDetail.DeletedComment));
msgBody.AppendLine();
msgBody.AppendLine("Order #:        {0}".FormatWith(order.ID.ToString()));
msgBody.AppendLine("Order Date:     {0}".FormatWith(order.SubmitDate.FormatDateTime(dtFormat)));
msgBody.AppendLine("Order Name:     {0}".FormatWith(order.Name));
msgBody.AppendLine("Order Comment:  {0}".FormatWith(order.Comment));
msgBody.AppendLine("Department:     {0}".FormatWith("{0} - {1}".FormatWith(orderDetail.Department, orderDetail.DepartmentDesc)));
msgBody.AppendLine();
msgBody.AppendLine("Item Number:    {0}".FormatWith(orderDetail.StockNumber));
msgBody.AppendLine("Quantity:       {0}".FormatWith(orderDetail.Quantity));
msgBody.AppendLine("Rush:           {0}".FormatWith((orderDetail.Rush.ToYesNo())));
msgBody.AppendLine("Item Name:      {0}".FormatWith(item.Name));
msgBody.AppendLine("Item Desc:      {0}".FormatWith(item.Description1));
msgBody.AppendLine("                {0}".FormatWith(item.Description2));
msgBody.AppendLine("Stock:          {0}".FormatWith((orderDetail.Stock.ToYesNo())));
msgBody.AppendLine("Item Comment:   {0}".FormatWith(orderDetail.Comment));

I have used one template engine in c# by name DotLiquid to resolve similar problems.

Readability: You have to extract your message template from the code to separate file. The template itself will lives its own life, usually independently from the code.
Maintainability:

Collect all required parameters and conditions in one place. Having conditions in your message tends to be next step in real message processing. By the way, anonymous classes from DotLiquid work well with your linq.
Then: template processing, message composing, and and putting it to queue to sent it. Sending operation itself could be time consuming and quite tricky

Thus I have handled hundreds of template based messages with rich HTML per day. Obviously, if you're going to write next smtp monster,  you'll write a more perfomance optimal solution. But using such template engine is good place to start.

And here is a code snippet:

// Template itself, assume that is has been loaded from file
var messageTemplate =
@"Deleted Order Item Information
------------------------------------

Delete Date:    {{Date}}
Delete Comment: {{DeletedComment}}
… other text";

// Parses and compiles the template
var template = DotLiquid.Template.Parse(messageTemplate);

// Renders the output
var messageBody = template.Render(DotLiquid.Hash.FromAnonymousObject(
    // Parameters
    new {
        Date = DateTime.Today,
        DeletedComment = "Some Comment"
    }));


Simple linked list implementation I am learning C and tried to make a basic singly linked list. I am looking for some feedback on my implementation. I have implemented deletion and reversal functions. Be hard on me as I am trying to improve.

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

//very basic linked list implimentation in C.
//basic node structure. No typedef.
struct node
{
  int val;
  struct node *next;
};

struct node *deleteNode(struct node *first, int num)
{
  struct node *prev=NULL;
  struct node *root = first;
  while (root != NULL)
  {
    if (root->val == num)
    {
      if (prev != NULL && root->next != NULL) //middle elements
      {
        prev->next = root -> next;
        free(root);
      }
      else if (prev == NULL) //first element
      {
        free(first);
        first = root->next;
        root = root->next;
      }
      else if (root->next == NULL) //last element
      {
        prev->next = NULL;
        free(root);
      }
    }
    prev = root;
    root = root -> next;
  }
  return first;
}

struct node *reverseList(struct node *root)
{
  struct node *temp= NULL;
  struct node *prev= NULL;

  while (root!= NULL)
  {
    temp = root->next;
    root->next = prev;
    prev = root;
    root = temp;
  }
  return prev;
}

void printList(struct node *root)
{
 while (root!= NULL)
 {
   printf("Value: %d\n", root -> val);
   root = root -> next;
 }
}

int main()
{
 struct node *root = NULL, *tail = NULL;

 int i=1;
 for (;i<=20; ++i)
 {
     struct node *current = malloc(sizeof(struct node));
     current -> val = i;
     current -> next = NULL;

     if (root==NULL) root = current;
     else tail -> next = current;

     tail = current;
 }

 //delete first,last and middle element
 root = deleteNode(root, 20);
 root = deleteNode(root, 1);
 root = deleteNode(root, 10);
 root = reverseList(root);
 printList(root);

 return 0;
}

I suggest using this declaration:

typedef struct _node {
  int val;
  struct _node *next;
} node;

After that, you can declare node *t instead of struct node *t. Also, you'll use sizeof(node) instead of sizeof(struct node).

Your delete is buggy. I'll quote just a bit:

struct node *deleteNode(struct node *first, int num)
{
  struct node *prev=NULL;
  struct node *root = first;
  while (root != NULL)
  {
    if (root->val == num)
    {
      ...
      else if (prev == NULL) //first element
      {
        free(first);
        first = root->next;
        root = root->next;
      }
      ...
    }
    ...
  }
  return first;
}

So, you have root = first, then you free the memory of first, and then you access root->next which is the same as first->next, which is a value contained in the already freed memory.

I'd write it in two parts:

Delete all there is to be deleted from the list start.
Delete all there is to be deleted, but is not at the list start.

Like this:

node *deleteNodes(node *first, int num) {
  node *prev, *active;

  while (first && first->val == num) {
    node *tmp = first;
    first = first->next;
    free(tmp);
  }

  if (!first) return NULL;

  prev = first;
  active = first->next;

  while (active)
    if (active->val == num) {
      prev->next = active->next;
      free(active);
      active = prev->next;
    } else {
      prev = active;
      active = active->next;
    }

  return first;
}

I have changed the name of the function because you seem to want to remove all the nodes containing the value num.

A matter of personal choice: I prefer to transverse through a list the same way I do with arrays -- by using a for loop.

void printList(node *root) {
  node *t;
  for (t = root; t; t = t->next)
    printf("Value: %d\n", t->val);
}

If you really want to reuse root, you can, but one or few additional variables (like my t (stands for "temporary") in the above code) can make your code much more readable.

It may be useful to finish void functions with return;, as it helps you detect certain problems in code (i.e., if you change the return value to something else, such return will result in error and warn you to fix it).

This

node *current = malloc(sizeof(node));

can be written like this like this:

node *current = (node*)malloc(sizeof(node));

Read about using such a cast here and decide for yourself. I prefer to use it.

You should always release all your memory (i.e., delete the remaining list) when you program ends, instead of relying on the OS to do it properly.