RattleHiss (fizzbuzz in python) This is an iterative review.
The previous iteration can be found here.
The next iteration can be found here

I've now been programming in python for all of 4 hours.

At 70 lines it's rather verbose for a FizzBuzz. If you can suggest a more pythonic way to structure it, that would be awesome.

FizzBuzz.py

def print_fizz_buzz_output(start_end_pair, pair_list):

    start_num, end_num = start_end_pair
    for num in range(start_num, end_num + 1):
        has_divisor = False
        for divisor, text in pair_list:
            if num % divisor == 0:
                has_divisor = True
                print(text, end='')
        if has_divisor:
            print() # New Line
        else:
            print(str(num))

def ask_divisor_text_pairs():
    exit_flag = False
    while not exit_flag:
        divisor_is_valid = False
        while not divisor_is_valid:
            divisor = input('Divisor? ')
            try:
                divisor = int(divisor)
                divisor_is_valid = True
            except ValueError:
                print('Invalid input for divisor. Divisor must be a whole number. Please try again.')

        text = input('Text? ')
        yield (divisor, text)

        while True:
            continue_response = input('Input Another Divisor (y/n)? ')
            if continue_response in ('N', 'n'):
                exit_flag = True
                break
            elif continue_response in ('Y', 'y'):
                exit_flag = False
                break
            else:
                print('Invalid response. Please input "y" or "n"')

def ask_iteration_range() -> tuple:

    start_num_is_valid = False
    while not start_num_is_valid:
        start_num = input('Start Number? ')
        try:
            start_num = int(start_num)
            start_num_is_valid = True
        except ValueError:
            print('Invalid input for start number. Must be a whole number. Please try again.')

    end_num_is_valid = False
    while not end_num_is_valid:
        end_num = input('End Number? ')
        try:
            end_num = int(end_num)
            if end_num >= start_num:
                end_num_is_valid = True
            else:
                raise ValueError
        except ValueError:
            print('Invalid input for end number. Must be a whole number greater than or equal to the start number.'
                  ' Please try again.')

    return start_num, end_num

print_fizz_buzz_output(ask_iteration_range(),list(ask_divisor_text_pairs()))

Example Input/Output

Start Number? 1
End Number? 20
Divisor? 3
Text? Rattle
Input Another Divisor (y/n)? y
Divisor? 5
Text? Hiss
Input Another Divisor (y/n)? n
1
2
Rattle
4
Hiss
Rattle
7
8
Rattle
Hiss
11
Rattle
13
14
RattleHiss
16
17
Rattle
19
Hiss

Flags, flags everywhere

There are constructs in Python that can reduce the amount of needed flags like the for ... else blocks or the try .. else construct. break and return should also be your friends:

def print_fizz_buzz_output(start_end_pair, pair_list):

    start_num, end_num = start_end_pair
    for num in range(start_num, end_num + 1):
        has_divisor = False
        for divisor, text in pair_list:
            if num % divisor == 0:
                has_divisor = True
                print(text, end='')
        if has_divisor:
            print() # New Line
        else:
            print(str(num))

def ask_divisor_text_pairs():
    while True:
        while True:
            divisor = input('Divisor? ')
            try:
                divisor = int(divisor)
            except ValueError:
                print('Invalid input for divisor. Divisor must be a whole number. Please try again.')
            else:
                break

        text = input('Text? ')
        yield (divisor, text)

        while True:
            continue_response = input('Input Another Divisor (y/n)? ')
            if continue_response in ('N', 'n'):
                return
            elif continue_response in ('Y', 'y'):
                break
            else:
                print('Invalid response. Please input "y" or "n"')

def ask_iteration_range() -> tuple:
    while True:
        start_num = input('Start Number? ')
        try:
            start_num = int(start_num)
        except ValueError:
            print('Invalid input for start number. Must be a whole number. Please try again.')
        else:
            break

    while True:
        end_num = input('End Number? ')
        try:
            end_num = int(end_num)
        except ValueError:
            print('Invalid input for end number. Must be a whole number.'
                  ' Please try again.')
        else:
            if end_num < start_num:
                print('Invalid input for end number. Must be greater than or equal to the start number. Please try again.')
            else:
                break

    return start_num, end_num

print_fizz_buzz_output(ask_iteration_range(),list(ask_divisor_text_pairs()))

Factorize out common behaviour

Next we see that you make extensive use of constructs like:

while True:
    value = input('Something')
    try:
        result = convert(value)
    except ValueError:
        print('Error message')
    else:
        break

You can factorize that out:

def ask_something(prompt, type_=int, error_message='Invalid input'):
    while True:
        value = input(prompt)
        try:
            return type_(value)
        except ValueError:
            print(error_message)

def print_fizz_buzz_output(start_end_pair, pair_list):
    start_num, end_num = start_end_pair
    for num in range(start_num, end_num + 1):
        has_divisor = False
        for divisor, text in pair_list:
            if num % divisor == 0:
                has_divisor = True
                print(text, end='')
        if has_divisor:
            print() # New Line
        else:
            print(str(num))

def ask_divisor_text_pairs():
    while True:
        divisor = ask_something('Divisor? ', error_message='Invalid input for divisor. Divisor must be a whole number. Please try again.')
        text = input('Text? ')
        yield (divisor, text)

        while True:
            continue_response = input('Input Another Divisor (y/n)? ')
            if continue_response in ('N', 'n'):
                return
            elif continue_response in ('Y', 'y'):
                break
            else:
                print('Invalid response. Please input "y" or "n"')

def ask_iteration_range() -> tuple:
    start_num = ask_something('Start Number? ', error_message='Invalid input for start number. Must be a whole number. Please try again.')

    while True:
        end_num = ask_something('End Number? ', error_message='Invalid input for end number. Must be a whole number. Please try again.')
        if end_num < start_num:
            print('Invalid input for end number. Must be greater than or equal to the start number. Please try again.')
        else:
            break

    return start_num, end_num

print_fizz_buzz_output(ask_iteration_range(),list(ask_divisor_text_pairs()))

Build strings before printing them

The last flag we didn't take care of is in the print_fizz_buzz_output function. For this one, we need to see the inner for loop as something equivalent to:

text_for_divisor = []
for divisor, text in pair_list:
    if num % divisor == 0:
        text_for_divisor.append(text)
if text_for_divisor:
    print(''.join(text_for_divisor))
else:
    print(num)

So we got rid of the flag but the construct is inneficient. Better use a list-comprehension instead:

text_for_divisor = [text for divisor, text in pair_list if num % divisor == 0]
print(''.join(text_for_divisor) if text_for_divisor else num)

Use if __name__ == '__main__'

@Dex'ter already told you, but trully, you want to take that habit. When you jump into an interactive session and try to debug your functions, you don't want to be prompted about what are your bounds. You want to be able to directly do:

>>> import FizzBuzz as fb
>>> fb.print_fizz_buzz_output((1, 50), [(3, 'Fizz'), (5, 'Buzz')])

Better signature

I would use print_fizz_buzz_output(begin, end, pair_list) instead of requiring a tuple as the begin/end pair, this feel more natural. You would then need to change your main call to print_fizz_buzz_output(*ask_iteration_range(), list(ask_divisor_text_pairs()))

Proposed improvements

def ask_something(prompt, type_=int, error_message='Invalid input'):
    while True:
        value = input(prompt)
        try:
            return type_(value)
        except ValueError:
            print(error_message)

def print_fizz_buzz_output(begin, end, pair_list):
    for num in range(begin, end + 1):
        text_for_divisors = [text for divisor, text in pair_list if num % divisor == 0]
        print(''.join(text_for_divisors) if text_for_divisors else num)

def ask_divisor_text_pairs():
    while True:
        divisor = ask_something('Divisor? ', error_message='Invalid input for divisor. Divisor must be a whole number. Please try again.')
        text = input('Text? ')
        yield (divisor, text)

        while True:
            continue_response = input('Input Another Divisor (y/n)? ')
            if continue_response in ('N', 'n'):
                return
            elif continue_response in ('Y', 'y'):
                break
            else:
                print('Invalid response. Please input "y" or "n"')

def ask_iteration_range() -> tuple:
    start_num = ask_something('Start Number? ', error_message='Invalid input for start number. Must be a whole number. Please try again.')

    while True:
        end_num = ask_something('End Number? ', error_message='Invalid input for end number. Must be a whole number. Please try again.')
        if end_num < start_num:
            print('Invalid input for end number. Must be greater than or equal to the start number. Please try again.')
        else:
            break

    return start_num, end_num

if __name__ == '__main__':
    print_fizz_buzz_output(*ask_iteration_range(), list(ask_divisor_text_pairs()))


Array Sorting in C++ I have this simple implementation for array sorting:

#include <iostream>
#include <stdlib.h>

using namespace std;

// some magic for the sorter
int compare(const void *i, const void *j) {
    int a = *(int *) i, b = *(int *) j;
    return a < b ? -1 : a > b ? 1 : 0;
}

int main() {
    int arr[100], temp = 0, count = 0;

    // reads values into the array (assumed integers as input)
    while (cin >> arr[count++]);
    count--;

    // what sorts the array
    qsort(arr, count, sizeof(int), compare);

    // prints everything but duplicate elements
    for (int i = 0; i < count; i++) {
        bool match = false;
        for (int j = 0; j < i && !match; j++)
            if (arr[i] == arr[j])
                match = true;
        if (!match)
            cout << arr[i];
    }
}

How can it be improved? I'd like to also remove duplicate elements rather than just silently ignoring them too.

You should use your standard libraries when you can. Consider that a std::set already does all the hard work, unique, and sorted values:

#include <iostream>
#include <set>

int main() {
    std::set<int> data;

    // reads values into the set (assumed integers as input)
    int val;
    while (std::cin >> val) {
        data.insert(val);
    }

    for(std::set<int>::iterator iter = data.begin(); iter != data.end(); ++iter) {
        std::cout << (*iter) << "\n";
    }

}

Note though, that the above implementation has horrible handling of issues that may come from invalid input. Still, the entire implementation is significantly simpler than yours.


Paginating blog posts from SQL server using nested LINQ statements I made an ASP.NET MVC application using Entity Framework for code-first migrations to the SQL Server database. I also use Unity (DI/IoC) for managing the services and repositories.

I made an overview page but I'll limit the items on that page to 10, 50 or 100 (variable named pagecount) items as selected by the user. The rest of the items are on other pages (variable named page). The result is something similar to what Code review has:

Each Blog has a Category. I'll also that the user can see one, more, or all categories (variable named categories). This contains a list of integers with the categories that the user will see.

using System.Data.Entity;

public class BlogRepo : GenericRepo<Blog>
{
    public List<Blog> GetByPage(int page, List<int> categories, int pagecount)
    {
        return (from b1 in ((from b2 in ((from b3 in dbSet.Include(b3 => b3.Category).Include(b3 => b3.User)
                                          orderby b3.CreationDateStamp ascending
                                          where categories.Contains(b3.CategoryId)
                                          select b3).Take<Blog>(page * pagecount)).Include(b2 => b2.Category).Include(b3 => b3.User) // step one
                             orderby b2.CreationDateStamp descending
                             select b2).Take<Blog>(pagecount)).Include(b1 => b1.Category).Include(b3 => b3.User) // step two
                orderby b1.CreationDateStamp ascending
                select b1).ToList<Blog>(); // step tree
    }
}

Here is my way of working:

I take the first items calculated by the formula page * pagecount sorted in ascending order by CreationDateStamp where categories.Contains(b3.CategoryId) is true.
Form that items I sort it in descending order on the same property, and take the value of pagecount.
Finally, sort it again (ascending), and call ToList<Blog>() to materialize the results.

Note one: I'll always include the properties Category (type of Category) and User (type of ApplicationUser).

Note two: the variable dbSet comes from a generic repository and is declared as follows:

public class GenericRepo<TEntity> : IGenericRepo<TEntity> where TEntity : class
{
    internal DbSet<TEntity> dbSet;
    // other basic linq statements for get basic stuff like GetAll, Delete, Insert, SaveChanges and Update
}

The code works, but I wondering if I can improve its performance and readability. In other words, can I combine some steps and can I reduce the code to make it shorter or simpler?

Your linq in linq in linq statement is written as a one-liner, with a single semicolon:

return (from b1 in ((from b2 in ((from b3 in dbSet.Include(b3 => b3.Category).Include(b3 => b3.User)
                                  orderby b3.CreationDateStamp ascending
                                  where categories.Contains(b3.CategoryId)
                                  select b3).Take<Blog>(page * pagecount)).Include(b2 => b2.Category).Include(b3 => b3.User) // step one
                     orderby b2.CreationDateStamp descending
                     select b2).Take<Blog>(pagecount)).Include(b1 => b1.Category).Include(b3 => b3.User) // step two
        orderby b1.CreationDateStamp ascending
        select b1).ToList<Blog>(); // step tree

In other words, it's doing too many things - you even have comments to identify the steps!

LINQ queries don't execute until they're materialized - you can take advantage of that and split the complex query into smaller steps that don't iterate the results.

Step one, grab all items up to the end of the desired page - this could be in its own method:

private IEnumerable<Blog> FetchItemsUpToSpecifiedPage(int page, int pageSize, IEnumerable<int> categories)
{
    return (from blog in dbSet.Include(item => blog.Category)
                              .Include(item => blog.User)
            orderby blog.CreationDateStamp ascending
            where categories.Contains(blog.CategoryId)
            select blog)
           .Take(page * pageSize);
}

Notice the following:

pagecount is renamed to a more accurate pageSize; "page count" sounds like like the number of pages, but what you want it to convey is the number of items on a page.
Meaningful identifiers. blog makes for easier reading than b3.
The compiler infers the generic type argument of Take<T> from the source parameter of the extension method, so specifying it is redundant and clutters up the code.

Step two, filter for specified page - again, in its own method:

private IEnumerable<Blog> FilterForSpecifiedPage(int page, int pageSize, IEnumerable<int> categories)
{
    var items = FetchItemsUpToSpecifiedPage(page, pageSize, categories);
    return (from blog in items.Include(item => blog.Category)
                              .Include(item => blog.User)
            orderby blog.CreationDateStamp descending
            select blog)
           .Take(pageSize);
}

Step three, sort and select - at that point that can go inside GetByPage:

public IEnumerable<Blog> GetByPage(int page, int pageSize, IEnumerable<int> categories)
{
    var items = FilterForSpecifiedPage(page, pageSize, categories);
    return (from blog in items.Include(item => blog.Category)
                              .Include(item => blog.User)
            orderby blog.CreationDateStamp ascending
            select blog)
           .ToList();
}

Now, this is strictly equivalent and improves nothing but readability, by un-nesting the queries.

I believe the .Include calls are redundant anywhere other than in the final step, in GetByPage - by including them so deep in the query you're carrying more data than you actually need along the way; I'd try removing them in the two private methods, and see if I get the same results.

Notice I'm returning an IEnumerable<Blog> and not a List<Blog> - the client code shouldn't be adding or removing items from this result set, and so doesn't need a List<Blog>. Rather, it merely needs to enumerate the results, so IEnumerable<Blog> is enough for its needs. And it's an interface, so you're not tying your hands with a specific implementation either.

Your pagination seems rather inefficient, you're sorting back and forth and back again.

Instead of sorting backwards and Takeing, you could look into what Skip can do for you - I believe it could boil down to something like this (untested):

return (from blog in dbSet.Include(item => item.Category)
                          .Include(item => item.User)
        where categories.Contains(blog.CategoryId)
        order by blog.CreationDate)
       .Skip((page - 1) * pageSize)
       .Take(pageSize)
       .ToList();

...which basically means, scratch pretty much everything I said above about extracting to private methods :)


Implementing find middle element of singly linked list without using 2 pointers There is a solution floating around about using 2 pointers. But I decide to keep the implementation much simpler by using basic constructs that works use to build basic methods of a singly list list data structure.

The code works. My question is, am I off the hook from making it better as long as the code works? Is the code sufficient?

 public Object findMiddleElement()
        {
            Node node = head;

            if(size % 2 != 0)
            {
                for(int i = 0; i < size/2 ;i++ )
                {
                    node = node.getNext();

                }
            }
            else
            {
                    System.out.println("there is no middle element");
                    node.setElement(null);;

            }
                    return node.getElement();
        }

It could be a little easier to read like this:

public Object findMiddleElement() {
    Node node = head;

    if(hasMiddleElement()) {
        for(int i = 0; i < size / 2; i++) {
            node = node.getNext();
        }
    } else {
        System.out.println("there is no middle element");
        node.setElement(null);
    }
    return node.getElement();
}

private boolean hasMiddleElement() {
   return size % 2 == 1;
}

This works if size is set in the class.


Boyer Moore Horspool Search Algorithm in Go I am practicing Go and try to implement Boyer Moore Horspool Algorithm in Go, It returns all ocurrance of a "phrase" in a given text. This code is working, but I would be pleased to have any feedback and suggestions from Go Gurus, about all aspect of my code.

package boyer_moore_horspool_search

type badMatchTable struct{
    table [256]int
    pattern string
}

func newBadMatchTable(pattern string) *badMatchTable{
    b := badMatchTable{
        pattern: pattern,
    }

    b.table = [256]int{}
    b.table = b.generateTable()

    return &b
}

func (b *badMatchTable) generateTable() [256]int{

    for i := 0; i < 256 ; i++  {
        b.table[i] = len(b.pattern)
    }

    lastPatternByte := len(b.pattern) - 1

    for i := 0; i < lastPatternByte ; i++  {
        b.table[int(b.pattern[i])] = lastPatternByte - i
    }

    return b.table
}

func Search(text string, pattern string) []int{

    var indexes []int
    byteText := []byte(text)
    bytePattern := []byte(pattern)

    textLength := len(byteText)
    patternLength := len(bytePattern)

    if textLength == 0 || patternLength == 0 || patternLength > textLength{
        return indexes
    }

    lastPatternByte := patternLength - 1

    mt := newBadMatchTable(pattern)
    index := 0
    for index <= (textLength - patternLength)  {
        for i := lastPatternByte ; byteText[index + i] == bytePattern[i]; i-- {
            if i == 0{
                indexes = append(indexes, index)
                break
            }
        }

        index += mt.table[byteText[index + lastPatternByte]]
    }

    return indexes
}

And this is the test file:

import (
    "fmt"
    "testing"
)

func TestSearch(t *testing.T) {

    indexes := Search("Hello, we want to find word Guru, so this #phrase has the word Guru.", "word Guru")

    fmt.Printf("Occurance: %v\n", len(indexes))
    for _, i := range indexes  {
        fmt.Printf("Index: %v\n", i)
    }

}

I added

func BenchmarkSearch(b *testing.B) {
    c := "Hello, we want to find word Guru, so this #phrase has the word Guru."
    s := "word Guru"
    for i := 0; i < b.N; i++ {
        Search(c, s)
    }
}

Then i ran

$ go test -v -bench=. -benchmem -memprofile=mem.out

It yielded

goos: linux
goarch: amd64
pkg: test/boyermoor
BenchmarkSearch-4        2000000           780 ns/op         104 B/op          3 allocs/op
PASS
ok      test/boyermoor  2.360s

Now i open pprof to get detailed stats

$ go tool pprof mem.out

(pprof) list Sear
Total: 268.52MB
ROUTINE ======================== test/boyermoor.BenchmarkSearch in /home/mh-cbon/gow/src/test/boyermoor/main_test.go
         0   268.52MB (flat, cum)   100% of Total
         .          .     18:
         .          .     19:func BenchmarkSearch(b *testing.B) {
         .          .     20:   c := "Hello, we want to find word Guru, so this #phrase has the word Guru."
         .          .     21:   s := "word Guru"
         .          .     22:   for i := 0; i < b.N; i++ {
         .   268.52MB     23:       Search(c, s)
         .          .     24:   }
         .          .     25:}
ROUTINE ======================== test/boyermoor.Search in /home/mh-cbon/gow/src/test/boyermoor/main.go
  268.52MB   268.52MB (flat, cum)   100% of Total
         .          .     36:}
         .          .     37:
         .          .     38:func Search(text string, pattern string) []int {
         .          .     39:
         .          .     40:   var indexes []int
  208.52MB   208.52MB     41:   byteText := []byte(text)
         .          .     42:   bytePattern := []byte(pattern)
         .          .     43:
         .          .     44:   textLength := len(byteText)
         .          .     45:   patternLength := len(bytePattern)
         .          .     46:
         .          .     47:   if textLength == 0 || patternLength == 0 || patternLength > textLength {
         .          .     48:       return indexes
         .          .     49:   }
         .          .     50:
         .          .     51:   lastPatternByte := patternLength - 1
         .          .     52:
         .          .     53:   mt := newBadMatchTable(pattern)
         .          .     54:   index := 0
         .          .     55:   for index <= (textLength - patternLength) {
         .          .     56:       for i := lastPatternByte; byteText[index+i] == bytePattern[i]; i-- {
         .          .     57:           if i == 0 {
      60MB       60MB     58:               indexes = append(indexes, index)
         .          .     59:               break
         .          .     60:           }
         .          .     61:       }
         .          .     62:
         .          .     63:       index += mt.table[byteText[index+lastPatternByte]]

You have allocations that are totally superfluous, given current test case, and they could be avoided by changing the input parameters types.

Lets give it a try

The code is updated to

package main

func main() {

}

type badMatchTable struct {
    table   [256]int
    pattern []byte
}

func newBadMatchTable(pattern []byte) *badMatchTable {
    b := badMatchTable{
        pattern: pattern,
    }

    b.table = [256]int{}
    b.table = b.generateTable()

    return &b
}

func (b *badMatchTable) generateTable() [256]int {

    for i := 0; i < 256; i++ {
        b.table[i] = len(b.pattern)
    }

    lastPatternByte := len(b.pattern) - 1

    for i := 0; i < lastPatternByte; i++ {
        b.table[int(b.pattern[i])] = lastPatternByte - i
    }

    return b.table
}

func Search(text, pattern []byte) []int {

    var indexes []int

    textLength := len(text)
    patternLength := len(pattern)

    if textLength == 0 || patternLength == 0 || patternLength > textLength {
        return indexes
    }

    lastPatternByte := patternLength - 1

    mt := newBadMatchTable(pattern)
    index := 0
    for index <= (textLength - patternLength) {
        for i := lastPatternByte; text[index+i] == pattern[i]; i-- {
            if i == 0 {
                indexes = append(indexes, index)
                break
            }
        }

        index += mt.table[text[index+lastPatternByte]]
    }

    return indexes
}

I behncmarked again

$ go test -v -bench=. -benchmem -memprofile=mem.out
goos: linux
goarch: amd64
pkg: test/boyermoor
BenchmarkSearch-4        2000000           669 ns/op          24 B/op          2 allocs/op
PASS
ok      test/boyermoor  2.023s

Those are small improvements.

I change the code again to remove the int slice allocation

package boyermoor

type badMatchTable struct {
    table   [256]int
    pattern []byte
}

func newBadMatchTable(pattern []byte) *badMatchTable {
    b := badMatchTable{
        pattern: pattern,
    }

    b.table = [256]int{}
    b.table = b.generateTable()

    return &b
}

func (b *badMatchTable) generateTable() [256]int {

    for i := 0; i < 256; i++ {
        b.table[i] = len(b.pattern)
    }

    lastPatternByte := len(b.pattern) - 1

    for i := 0; i < lastPatternByte; i++ {
        b.table[int(b.pattern[i])] = lastPatternByte - i
    }

    return b.table
}

func Search(text, pattern []byte, indexes []int) []int {

    // var indexes []int
    indexes = indexes[:0]

    textLength := len(text)
    patternLength := len(pattern)

    if textLength == 0 || patternLength == 0 || patternLength > textLength {
        return indexes
    }

    lastPatternByte := patternLength - 1

    mt := newBadMatchTable(pattern)
    index := 0
    for index <= (textLength - patternLength) {
        for i := lastPatternByte; text[index+i] == pattern[i]; i-- {
            if i == 0 {
                indexes = append(indexes, index)
                break
            }
        }

        index += mt.table[text[index+lastPatternByte]]
    }

    return indexes
}

The benchmark is modified to

func BenchmarkSearch(b *testing.B) {
    indexes := make([]int, 0, 100)
    c := []byte("Hello, we want to find word Guru, so this #phrase has the word Guru.")
    s := []byte("word Guru")
    for i := 0; i < b.N; i++ {
        indexes = Search(c, s, indexes)
    }
}

Now i got

goos: linux
goarch: amd64
pkg: test/boyermoor
BenchmarkSearch-4        3000000           597 ns/op           0 B/op          0 allocs/op
PASS
ok      test/boyermoor  2.402s

That is more substantial.

In the details, you rewrite the generateTable this way to save a few more ops

func (b *badMatchTable) generateTable() [256]int {

    k := len(b.pattern)
    for i := 0; i < 256; i++ {
        b.table[i] = k
    }

    lastPatternByte := k - 1

    for i := 0; i < lastPatternByte; i++ {
        b.table[b.pattern[i]] = lastPatternByte - i
    }

    return b.table
}

At that point, consider that your algorithm is 1/ cpu bound 2/ a bunch of tight loops. So to get much out of the runtime, squeeze as many instructions as possible.

Given the last state of the code, let just get ride of the badTable struct. So the whole thing is contained into one function call.

func Search(text, pattern []byte, indexes []int) []int {

    // var indexes []int
    indexes = indexes[:0]

    textLength := len(text)
    patternLength := len(pattern)

    if textLength == 0 || patternLength == 0 || patternLength > textLength {
        return indexes
    }

    lastPatternByte := patternLength - 1

    table := [256]int{}
    {
        k := len(pattern)
        for i := 0; i < 256; i++ {
            table[i] = k
        }

        lastPatternByte := k - 1
        for i := 0; i < lastPatternByte; i++ {
            table[pattern[i]] = lastPatternByte - i
        }
    }

    index := 0
    for index <= (textLength - patternLength) {
        for i := lastPatternByte; text[index+i] == pattern[i]; i-- {
            if i == 0 {
                indexes = append(indexes, index)
                break
            }
        }

        index += table[text[index+lastPatternByte]]
    }

    return indexes
}

And now it gives us

goos: linux
goarch: amd64
pkg: test/boyermoor
BenchmarkSearch-4        5000000           339 ns/op           0 B/op          0 allocs/op
BenchmarkSearch-4        5000000           340 ns/op           0 B/op          0 allocs/op
BenchmarkSearch-4        5000000           341 ns/op           0 B/op          0 allocs/op
BenchmarkSearch-4        5000000           338 ns/op           0 B/op          0 allocs/op
PASS

Now we have a good 2 times improvement.

But this is not all about performance, it is also about usability for other devs. So the API can be presented this way, which produces a decent trade between the two.

type Horspool struct {
    table   [256]int
    indexes []int
}

func (t *Horspool) Search(text, pattern []byte) []int {

    table := t.table
    indexes := t.indexes
    if cap(indexes) < 1 {
        indexes = make([]int, 0, 100)
    }
    indexes = indexes[:0]

    textLength := len(text)
    patternLength := len(pattern)

    if textLength == 0 || patternLength == 0 || patternLength > textLength {
        t.indexes = indexes
        return indexes
    }

    lastPatternByte := patternLength - 1

    {
        for i := 0; i < 256; i++ {
            table[i] = patternLength
        }

        lastPatternByte := patternLength - 1
        for i := 0; i < lastPatternByte; i++ {
            table[pattern[i]] = lastPatternByte - i
        }
    }

    index := 0
    for index <= (textLength - patternLength) {
        for i := lastPatternByte; text[index+i] == pattern[i]; i-- {
            if i == 0 {
                indexes = append(indexes, index)
                break
            }
        }

        index += table[text[index+lastPatternByte]]
    }
    t.indexes = indexes
    return indexes
}

It gives me a slightly slower program

BenchmarkSearch-4        5000000           370 ns/op           0 B/op          0 allocs/op

I have not tried to understand as to why it is slower because here is a more interesting case involving struct alignment and padding. See this code,

type Horspool struct {
    indexes []int
    table   [256]int
}

And check the results

BenchmarkSearch-4        3000000           480 ns/op           0 B/op          0 allocs/op

read more: https://dave.cheney.net/2015/10/09/padding-is-hard

FTR, consider this is an old algorithm and that it is not utf-8 valid. Go being utf-8 first, this should be improved.

If my understanding of the algorithm is correct, the fix is rather simple

type Horspool struct {
    table   map[rune]int
    indexes []int
}

func (t *Horspool) Search(text, pattern []rune) []int {

    table := t.table
    if table == nil {
        table = map[rune]int{}
    } else {
        for r := range table {
            delete(table, r)
        }
    }
    indexes := t.indexes
    if cap(indexes) < 1 {
        indexes = make([]int, 0, 100)
    }
    indexes = indexes[:0]

    textLength := len(text)
    patternLength := len(pattern)

    if textLength == 0 || patternLength == 0 || patternLength > textLength {
        return indexes
    }

    lastPatternByte := patternLength - 1

    {
        for _, r := range pattern {
            table[r] = patternLength
        }

        for i := 0; i < lastPatternByte; i++ {
            table[pattern[i]] = patternLength - 1 - i
        }
    }

    index := 0
    for index <= (textLength - patternLength) {
        for i := lastPatternByte; text[index+i] == pattern[i]; i-- {
            if i == 0 {
                indexes = append(indexes, index)
                break
            }
        }
        x, ok := table[text[index+lastPatternByte]]
        if ok {
            index += x
        } else {
            index += lastPatternByte
        }
    }
    t.table = table
    t.indexes = indexes
    return indexes
}

But, take care of maps.
https://stackoverflow.com/questions/58475257/map-delete-doesnt-actually-delete-entries

Use a simple counter to free-it-by-allocation regularly.

Finally, as somewhat expected the code is slower by a factor 2

goos: linux
goarch: amd64
pkg: test/boyermoor
BenchmarkSearch-4        2000000           756 ns/op           0 B/op          0 allocs/op
PASS
ok      test/boyermoor  2.282s

Two notes:

There might be more interesting/subtle optimizations that i m not aware of myself, i did the obvious.


Finding prime numbers in user specified array I have a program that searches for prime numbers in an array specified by the user.  The program starts by asking how big the user wants the array to be, then asks how many threads to split the computations.

I tested 1000 numbers, 10,0000, 100,000 and 1 million, and they all yielded results in a fairly reasonably time.  1 million numbers took about 2 minutes.  When I tried 10 million numbers, well.. the program is still running well over 2 hours with no results.

I'm wondering if the speed is normal, or if there's something fundamentally wrong with my find prime algorithm that is making it take so long to yield the desired results.

Prime.Java

import java.util.ArrayList;
import java.util.List;
import java.util.Scanner;
import java.util.concurrent.ArrayBlockingQueue;

public class Prime extends Thread implements Runnable {
    //public List<Integer> primeList = new ArrayList<>();

    static ArrayBlockingQueue<Integer> primeList;
    int start, end;

    //int max = num;
    public int count = 0;

    public Prime(int start, int end) {
        this.start = start;
        this.end = end;
    }

    Scanner scan = new Scanner(System.in);

   public void run() {
       for (int n = start; n <= end; n++) {
           boolean prime = true;
                  for (int j = 2; j < n; j++) {
                          if (n%j == 0){
                              prime = false;
                              break;
                                }
                  }

                  if(prime && n !=1 && n!=0){   // Added conditions so it does not count 1 or 0 as prime.
                        count++;
                        primeList.add(n);

                }

          }
   }
}

Worker.Java

import java.util.ArrayList;
import java.util.List;
import java.util.Scanner;
import java.util.concurrent.ArrayBlockingQueue;

public class Worker {
    public static void main(String[] args) {

        Scanner scan = new Scanner(System.in);

        System.out.println("How big would you like the array? ");
        int num = scan.nextInt();

        int [] primes = new int [num];

        System.out.println("How Many threads? ");
        int nThreads = scan.nextInt();  // Create variable 'n'  to handle whatever integer the user specifies.  nextInt() is used for the scanner to expect and Int.

        final Prime[] pThreads = new Prime[nThreads];

        long startTime = System.currentTimeMillis();
        Prime.primeList = new ArrayBlockingQueue<>(primes.length); // Guaranteed to be enough
        int step = primes.length / nThreads + 1;
        for(int i = 0; i<nThreads; i++){
            pThreads[i] = new Prime(i * step, Math.min(primes.length, (i + 1) * step - 1));
            pThreads[i].start();
        }

        try {
            for (int i = 0; i < nThreads; i++)
                pThreads[i].join();
        } catch (InterruptedException e) {
            }

            long stopTime = System.currentTimeMillis();

            long elapsedTime = stopTime - startTime;
            System.out.println("Execution time = : "+  elapsedTime);

            System.out.println("----------------------------------------------------");

            int cores = Runtime.getRuntime().availableProcessors();
            System.out.println("How many Cores this Java Program used: " + cores);

            System.out.println("Array length " + primes.length);

          for ( int i = 0; i < nThreads; i++)
             System.out.println("Thread " + i + "  Prime count: " + pThreads[i].count); // Display Thread count
             System.out.println("Total prime count: " + Prime.primeList.size()); // Output total amount of primes from the Array List
            // System.out.println(Prime.primeList);

    }
}

Indentation

Your indentation in run() and other methods can be improved:

boolean prime = true;
       for (int j = 2; j < n; j++) {
               if (n%j == 0){
                   prime = false;
                   break;
                     }
       }

At first, I thought that was another level of nesting.  You should write it like this.

boolean prime = true;
for (int j = 2; j < n; j++) {
    if (n%j == 0){
        prime = false;
        break;
    }
}

Naming

Don't name loop variables any random number:

for (int n = start; n <= end; n++) {
    for (int j = 2; j < n; j++) {

You should give them a name describing what they are:

for (int potential_prime = start; potential <= end; potential++) {
    for (int divisor = 2; divisor < n; divisor++) {

Algorithm

When you check whether any number n is prime, you check the divisors j from 2 to n.  If a divisor j is greater than the floor (sqrt (n)), you need not check it.  This is because once you get above this number and less than n / 2, you are only checking for the reverse pairs you checked before (4 * 5 == 5 * 4), and any number greater than n / 2 cannot go into n more than once.  Eliminating these extra checks could greatly speed your algorithm up on very large numbers.


Recursive partition sort is inefficient I have written a recursive method for a partition sort that sorts the array. However, when I use an array of more than 10-20 elements the program takes a really long time to complete. (On my computer a bubble sort of a 100,000 int array will take about 15-20 seconds, but with an array of only 30 ints my partition sort is taking around 45 seconds to be sorted.)

public static int[] partitionSortRecursive(int[] array, int beginning, int end)
{
    if (end < beginning)
        return array;

    int pivot = (array[beginning] + array[end]) / 2;
    int firstUnknown = beginning;
    int lastS1 = beginning - 1;
    int firstS3 = end + 1;

    while (firstUnknown < firstS3)
    {
        if (array[firstUnknown] == pivot)
        {
            firstUnknown++;
        }
        else if (array[firstUnknown] > pivot)
        {
            firstS3--;
            int temp = array[firstUnknown];
            array[firstUnknown] = array[firstS3];
            array[firstS3] = temp;
        }
        else
        {
            lastS1++;
            int temp = array[firstUnknown];
            array[firstUnknown] = array[lastS1];
            array[lastS1] = temp;
            firstUnknown++;
        }

    }

    partitionSortRecursive(array, 0, lastS1);
    partitionSortRecursive(array, firstS3, end);

    return array;
}

Your code at the current state looks very slow. Partition sort (or quicksort as it is generally known) is supposed to be faster than Bubble Sort. With my quicksort and bubble sort code, here is what I get for a 1000-element array:

Time taken (ns): 612535
Sorted List: ...
Time taken (ns): 5819902
Sorted List: ...

This is because quicksort is \$O(n \log n)\$ while bubble sort is \$O(n^2)\$.

How I would do quicksort:

Select a pivot
Create two pointers, one at the front and one at the back
Move first pointer up until there is a value greater or equal to the pivot
Move second pointer down until there is a value less than or equal to the pivot
Swap and advance both pointers
If the pointers have not passed each other, go to step 1. Otherwise, continue
Repeat recursively for each subarray

Code:

private static final Random RANDOM = new Random();

public static void quickSort(int[] array) {
    quickSort(array, 0, array.length);
}

private static void quickSort(int[] array, int begin, int end) {
    if (end - begin < 2) {
        return;
    }
    if (end - begin == 2) {
        // Optimization: array length 2
        if (array[begin] > array[end - 1]) {
            swap(array, begin, end - 1);
        }
        return;
    }
    int splitIndex = partition(array, begin, end);
    quickSort(array, begin, splitIndex);
    quickSort(array, splitIndex, end);
}

private static int partition(int[] array, int begin, int end) {
    int pivot = array[RANDOM.nextInt(end - begin) + begin];
    begin--;
    while (begin < end) {
        do {
            begin++;
        } while (array[begin] < pivot);
        do {
            end--;
        } while (array[end] > pivot);
        if (begin < end) {
            // Make sure they haven't crossed yet
            swap(array, begin, end);
        }
    }
    return begin; // TODO check
}

private static void swap(int[] array, int begin, int end) {
    int temp = array[begin];
    array[begin] = array[end];
    array[end] = temp;
}

The results:

Time taken (ns): 507845
Sorted Array: ...


Implementation of a simple hash function This is my implementation of a hash function described below. The (compiled) program can be run as follows:

$ java Hash1 < input.txt

The program reads a text file (of any size) with 8-digit student numbers on each line, splits each number into three numbers such that they have 3, 3 and 2 digits and then sums these numbers (e.g., for a student number 12345678, the hash is 123+456+78). Then, the hash is simply printed.

If the hash has already been used, the student number is reversed and the hash function is calculated from the reversed number (12345678 is reversed to 87654321 and the hash is 876+543+21). If this value has also been taken, it just prints "-1" and continues to the next value.

I use two external libraries for input and output:

edu.princeton.cs.algs4.stdin
edu.princeton.cs.algs4.stdout

The implementation:

public class Hash1 {

    // the main method reads student number and
    // gets the hash value. it outputs the hash
    // if it hasn't been used, reverse hash
    // or -1 if both hash values are taken
    public static void main(String[] args) {

        // the size is 2098 because the highest
        // hash (index) can be 999+999+99=2097
        String[] indexarray = new String[2098];

        while (!StdIn.isEmpty())
        {
            String key = StdIn.readLine();
            char[] charray =  key.toCharArray();
            int hash = hash(charray);

            if (indexarray[hash] != null) {
                int newhash = reverse_hash(charray);
                if (indexarray[newhash] != null) {
                    int errorFlag = -1;
                    StdOut.println(errorFlag);
                    }
                else {
                    indexarray[newhash] = key;
                    StdOut.println(newhash);
                    }
            }
            else {
                indexarray[hash] = key;
                StdOut.println(hash);
            }
        }
    }

    // this method performs the hash as specified
    // and returns the value of the hash
    private static int hash(char[] charray) {
        String[] numbers = {"", "", ""};

        for (int i = 0; i < charray.length; i++)
            if (i < 3)
                numbers[0] = numbers[0] + charray[i];
            else if (i >= 3 && i < 6)
                numbers[1] = numbers[1] + charray[i];
            else if (i >= 6 && i < 8)
                numbers[2] = numbers[2] + charray[i];

        int firstNumb = Integer.parseInt(numbers[0]);
        int secondNumb = Integer.parseInt(numbers[1]);
        int thirdNumb = Integer.parseInt(numbers[2]);
        int result = firstNumb + secondNumb + thirdNumb;

        return result;
    }

    // this method reverses the student number,
    // calls hash() method to hash the reversed
    // student number and returns the hash
    private static int reverse_hash(char[] charray) {
        char [] reverse = new char[charray.length];
        for (int i = 0; i < reverse.length; i++) {
            int index = reverse.length - i -1;
            reverse[i] = charray[index];
        }
        int hash = hash(reverse);
        return hash;
    }
    }

I am new to Java and would like to know how could I improve or optimize this program, if there are any issues with the code or I am using any bad (discouraged) practises etc.

The very idea of "if that hash is already taken, use another one" leads to bugs that are really hard to find later.

It's because your hash function (oh, it is called a function, hopefully in the mathematical sense) no longer has the signature hash(studentNumber) but instead becomes hash(studentNumber, allPreviousStudentNumbers). This means you get different hashes based on how the students are sorted.

A hash function may by definition produce collisions, and that is fine. Choosing an appropriate hash function that minimizes these collisions is often crucial. In cryptography the commonly used hashes have 256 bits, which makes a collision very unlikely. I hope your teacher plans to discuss these topics in class, as these are important.

Regarding your implementation of the basic hash function. It looks a little more complicated than necessary. An experienced Java programmer will know that there is a predefined String constructor that forms a String from part of a character array. It is new String(charray, start, count). Using this, the code becomes simple:

private static int hash(char[] studentNo) {
    int first = Integer.parseInt(new String(studentNo, 0, 3));
    int second = Integer.parseInt(new String(studentNo, 3, 3));
    int third = Integer.parseInt(new String(studentNo, 6, 2));
    return first + second + third;
}

This code is almost as simple as you would describe the algorithm to a human, and that's how it should be.

One tricky thing that bites me every time I use the String class: in general there are two ways of referring to a substring, a subarray or a subsequence: either (start, end) or (start, count). Unfortunately Java uses both of them. In the String constructor I used above, it's (start, count), while in the substring method it's (start, end). Even after 20 years of programming in Java, I still have to look at the documentation to know when each of these applies.


Deserializing JSON in an expression-bodied member All of these if conditions in the below method have similar pattern, Any ideas to come up with a common method to reduce the duplication in this method?

some case we want all the version, and in some case we want only specific version.

I tried with switch case.

public override Func<JObject, dynamic, string> VersionMethod => (jobject, parameters) =>{
        bool hasValidObject = false;

        if (jobject["swVersion"] != null)
        {
            _livetv.SoftwareVersion = new VersionInfo(jobject["swVersion"].Value<string>());
            hasValidObject = true;
        }

        if (jobject["hwVersion"] != null)
        {
            _livetv.HardwareVersion = new VersionInfo(jobject["hwVersion"].Value<string>());
            hasValidObject = true;
        }

        if (jobject["ltvVersion"] != null)
        {
            _livetv.LTV2Version = new VersionInfo(jobject["ltvVersion"].Value<string>());
            hasValidObject = true;
        }

        if (jobject["ltv3Version"] != null)
        {
            _livetv.LTV3Version = new VersionInfo(jobject["ltv3Version"].Value<string>());
            hasValidObject = true;
        }

        if (jobject["cricVersion"] != null)
        {
            _livetv.KAVersion = new VersionInfo(jobject["cricVersion"].Value<string>());
            hasValidObject = true;
        }

        if (jobject["bbVersion"] != null)
        {
            _livetv.BasebandVersion = new VersionInfo(jobject["bbVersion"].Value<string>());
            hasValidObject = true;
        }

        if (hasValidObject)
        {
            return GenerateSuccessful();
        }

        return GenerateUnsuccessful(
            "Unable to parse version from request, try again.");
    };

We can of course try to optimize it but this would be only an ugly workaround trying to minimize the flaws of the current design.

There are more questions then answers:

Why is the the parameter dynamic?
Are you trying to parse json? JObject suggests you are and I'm pretty sure this is the wrong way to do it.
Why do you use such a crazy syntax for a method? The expression bodied members are meant for one-liners and not for such huge blocks.
Why don't you deserialize the object with JsonConvert.Deserialize?
Why don't you decorate the properties like SoftwareVersion with the appropriate attribute like [JsonProperty(PropertyName = "swVersion")]

You should rethink the entire parsing process and not just this helper method.


C++ reimplementation of Python's threading.Event I've reimplemented in C++ the basic interface implemented by Python's threading.Event class using a mutex and condition variable. I've inlined functions rather than pasted both the header and source file. My reimplementation also skips the timeout argument to wait since I don't need it. The code appears to work through basic testing (test shown below reimplementation), but I'm particularly concerned about thread safety -- is using a mutex to protect isSet_ enough, or is there a way for a thread to get stuck waiting for an event that's already been set? Are there any other improvements I need to make?
Reimplementation:
#include <mutex>
#include <condition_variable>

class Event {
private:
    std::mutex isSetLock;
    bool isSet_ = false;
    std::mutex setTriggerLock;
    std::condition_variable setTrigger;
public:
    bool isSet () {
        isSetLock.lock ();
        bool isSetRet (isSet_);
        isSetLock.unlock ();
        return isSetRet;
    };
    void set () {
        isSetLock.lock ();
        isSet_ = true;
        isSetLock.unlock ();
        setTrigger.notify_all ();
    };
    void clear () {
        isSetLock.lock ();
        isSet_ = false;
        isSetLock.unlock ();
    };
    void wait () {
        if (!isSet ()) {
            std::unique_lock <std::mutex> setTriggerUniqueLock (setTriggerLock);
            setTrigger.wait (setTriggerUniqueLock);
        }
    };
};

Basic test:
#include "Event.hpp"
#include <thread>

class Test {
    Event event;
    void thread_ () {
        event.wait ();
    };
public:
    Test () {
        std::thread thread (&Test::thread_, this);
        event.set ();
        thread.join ();
    };
};
int main () {
    Test test;
    // Should exit immediately
};

You can simplify things using RAII.
bool isSet () {
    isSetLock.lock ();
    bool isSetRet (isSet_);
    isSetLock.unlock ();
    return isSetRet;
};

Can be simplified to:
bool isSet () {
    std::lock_guard<std::mutex>  lockGuard(isSetLock);    // Calls lock.
    return isSet_;
};                                                        // Calls unlock
                                                          // in destructor.

Why do you have two different mutexes? There should only be a single one:
void wait () {
    if (!isSet ()) {
        std::unique_lock <std::mutex> setTriggerUniqueLock (setTriggerLock);
        setTrigger.wait (setTriggerUniqueLock);
    }
}

This should be:
    std::unique_lock <std::mutex> setTriggerUniqueLock(isSetLock);
    while (isSet()) {
       setTrigger.wait(setTriggerUniqueLock);
    }

Note: While the thread is paused (waiting) in the condition variable the mutex is unlocked (so other threads can still lock the mutex). The thread is only released from the condition variable (after a notify) when it has re-acquired the lock.
Note: You have to use while here (not if). After the thread is woken you need to retest the condition because another thread may have re-locked the isSet_ variable before this thread re-acquired the lock.
Note: The condition variable has this built in:
    std::unique_lock <std::mutex> setTriggerUniqueLock(isSetLock);
    setTrigger.wait(setTriggerUniqueLock, [&](){return !isSet_WithoutNeedingToLockAsWeHaveTheLock()});

Note: After doing this your notify_all() should be on setTrigger.


Download NASA images to use as Windows wallpaper I made myself a simple script (Batch/JScript) that download a pic from a blog (APOD) & set it as wallpaper.

@if (@a==@b) @end /*

Set BaseUrl=https://apod.nasa.gov/apod/
set yr=%date:~8,2%
set mn=%date:~3,2%
set dy=%date:~0,2%
set dte=%yr%%mn%%dy%
set SaveDest=C:\Users\ME\Desktop\Wallpaper\%dte%.jpg
Set Url=%baseUrl%ap%dte%.html

@echo off
setlocal
for /f "delims=" %%I in ('cscript /nologo /e:jscript "%~f0" "%Url%"') do (
    rem
    echo %%I
    set LastImg=%%I
)

set ImgUrl=%BaseUrl%%LastImg:~6%
bitsadmin.exe /transfer GETWALLPAPER %ImgUrl% %SaveDest%
timeout /t 01
reg add "HKEY_CURRENT_USER\Control Panel\Desktop" /v Wallpaper /t REG_SZ /d %savedest% /f
timeout /t 01
for /l %%x in (1,1,80) do  (
RUNDLL32.EXE USER32.DLL,UpdatePerUserSystemParameters
reg add "HKEY_CURRENT_USER\Control Panel\Desktop" /v Wallpaper /t REG_SZ /d %savedest% /f
timeout /t 01
)

goto :EOF

JScript */
function fetch(url) {
    var xhr=WSH.CreateObject("Microsoft.XMLHTTP");
    var dom=WSH.CreateObject('htmlfile');
    xhr.open("GET",WSH.Arguments(0),true);
    xhr.setRequestHeader('User-Agent','XMLHTTP/1.0');
    xhr.send('');
    while (xhr.readyState!=4) {WSH.Sleep(25)};
    dom.write('<meta http-equiv="x-ua-compatible" content="IE=9" />');
    dom.write(xhr.responseText);
    return dom;
}
var dom=fetch(WSH.Arguments(0));
var link=dom.getElementsByTagName('IMG');

WSH.Echo(link[0].src);

It's working. However, I am unhappy about the for /l %%x in (1,1,80) d loop: it seems not to work without the loop. Any Idea ?

First : I recommend you to change this line in your code in order to anybody can use it easily without changing or editing it.

set SaveDest=C:\Users\ME\Desktop\Wallpaper\%dte%.jpg

by those lines :

set WallpaperFolder=%userprofile%\Desktop\Wallpaper
If not exist "%WallpaperFolder%" MD "%WallpaperFolder%"
set SaveDest=%WallpaperFolder%\%dte%.jpg

Second : If you want to hide the output of the console you can redirect it to NUL

for example : reg add "HKEY_CURRENT_USER\Control Panel\Desktop" /v Wallpaper /t REG_SZ /d %savedest% /f>nul

And this line too : timeout /t 1 /nobreak>nul

Third : Refer to How to force Windows desktop background to update or refresh

You can write it simply by this line and avoid the loop : RUNDLL32.EXE USER32.DLL,UpdatePerUserSystemParameters 1, True

So, the whole code can be written like this :

@if (@a==@b) @end /*
@echo off
Title Download NASA images to use as Windows wallpaper
color 0A & Mode 70,3
Set BaseUrl=https://apod.nasa.gov/apod/
set yr=%date:~8,2%
set mn=%date:~3,2%
set dy=%date:~0,2%
set dte=%yr%%mn%%dy%
set WallpaperFolder=%userprofile%\Desktop\Wallpaper
If not exist "%WallpaperFolder%" MD "%WallpaperFolder%"
set SaveDest=%WallpaperFolder%\%dte%.jpg
Set Url=%baseUrl%ap%dte%.html

setlocal
for /f "delims=" %%I in ('cscript /nologo /e:jscript "%~f0" "%Url%"') do (
    echo %%I
    set LastImg=%%I
)

set ImgUrl=%BaseUrl%%LastImg:~6%

bitsadmin.exe /transfer GETWALLPAPER "%ImgUrl%" "%SaveDest%"
echo(
echo        Please wait a while ... Update  UserSystemParameters
reg add "HKEY_CURRENT_USER\Control Panel\Desktop" /v Wallpaper /t REG_SZ /d "%savedest%" /f>nul
Timeout /t 1 /nobreak>nul
RUNDLL32.EXE USER32.DLL,UpdatePerUserSystemParameters 1, True
goto :EOF

JScript */
function fetch(url) {
    var xhr=WSH.CreateObject("Microsoft.XMLHTTP");
    var dom=WSH.CreateObject('htmlfile');
    xhr.open("GET",WSH.Arguments(0),true);
    xhr.setRequestHeader('User-Agent','XMLHTTP/1.0');
    xhr.send('');
    while (xhr.readyState!=4) {WSH.Sleep(25)};
    dom.write('<meta http-equiv="x-ua-compatible" content="IE=9" />');
    dom.write(xhr.responseText);
    return dom;
}
var dom=fetch(WSH.Arguments(0));
var link=dom.getElementsByTagName('IMG');
WSH.Echo(link[0].src);


Bubble sort algorithm in JavaScript I'm new to JavaScript (and programming) and taking a course. I was tasked with creating a bubble sort on any array using JavaScript. Of course, since this is an algorithm challenge, no sort method allowed. After I wrote this, I looked around at other implementations of bubble sort. I only found one that used a similar algorithm. Most set a variable to true or false to exit the loop.

I'm curious if the way I've written this is valid, efficient, and even a bubble sort at all -- since I don't loop through the entire array at any point (unless edge case is the array is already sorted). I know there are tons of bubble sort questions out there, but I couldn't find one exactly like this.

//Bubble Sort algorithm practice for JavaScript class

arrayX = [1,9,-1,5,10,23,-2,7,4,5,1];

for (i = 0; i <= arrayX.length-1; i++)
  {
    if (arrayX[i] > arrayX[i+1])
      {
        temp = arrayX[i+1];
        arrayX[i+1] = arrayX[i];
        arrayX[i] = temp;
        i = i-2;
      }
  }
console.log(arrayX);

Let me start with the interesting question:

  Is it even a bubble sort?

This sort is a bit like bubble sort and a bit like insertion sort. You have re-invented gnome sort, so congratulations, that's a nice coup for a beginner.

The other answers -- which state that you should not access arrays out-of-bounds, and should use var to declare local variables rather than relying on automatic declaration to save you, are correct.  To that basic advice I would add the following:

Make a sorting function that takes an array as its argument
Now that you have such a function, write test cases.  Don't forget to test already-sorted arrays, empty arrays, arrays with one, two, or three elements, arrays sorted into reverse order, and so on.

That's the basic stuff. A more advanced technique that you should start using now is

clearly state your loop invariants in comments or, better, assertions

A loop invariant is something that you believe to be true no matter what at various points in the loop. For example, a loop invariant that you have violated is "the index is always in bounds".

By clearly stating your invariants you can then analyze the code to see if it correctly maintains the invariants. If it does, then you have more confidence that the code is correct.

The loop invariant of bubble sort is "after n iterations, the last n elements are in their proper place".

The loop invariant of gnome sort is "everything to the left of i is sorted".

  I'm curious if the way I've written this is valid, efficient, and even a bubble sort at all

Though the code uses poor practices that have already been discussed, it appears to be a valid sort.

It is not, as sorts go, efficient. Asking whether a bubble sort (or bubble sort variation) written in JavaScript is efficient is a bit like asking which of two pet turtles is the faster; if you care about speed then you've chosen the wrong pet.


A very small crawler created using class I've created a crawler using class. This crawler is able to scrape a certain webpage. Total data out there is 249 and the data are displayed there through different pages. I tried to make it accurately. Here is what I did.

import requests
from lxml import html

class wiseowl:

    def __init__(self, start_url):
        self.start_url = start_url
        self.links = [self.start_url]    # a list of links to crawl
        self.storage = []

    def crawl(self):           # calling get_link for every link in self.links
        for link in self.links :
            self.get_link(link)

    def get_link(self,link):
        print('Crawling: ' + link)
        url = "http://www.wiseowl.co.uk"
        response = requests.get(link)
        tree = html.fromstring(response.text)
        for items in tree.xpath("//p[@class='woVideoListDefaultSeriesTitle']"):
            name = items.xpath(".//a/text()")[0]
            urls = url + items.xpath(".//a/@href")[0]
            docs = name , urls
            self.storage.append(docs)
            next_page = tree.xpath("//div[contains(concat(' ', @class, ' '), ' woPaging ')]//*[@class='woPagingItem' or @class='woPagingNext']/@href")    # get links form 'woPagingItem' or 'woPagingNext' #
            for npage in next_page:
                if npage and url + npage not in self.links :    # avoid getting the same link twice
                    self.links += [url + npage]

    def __str__(self):
        return "{}".format(self.storage)

crawler=wiseowl("http://www.wiseowl.co.uk/videos/")
crawler.crawl()
for item in crawler.storage:
    print(item)

General

Imports should be grouped, and groups should be separated by a single blank line.1
Class names should use CamelCase.2
There shouldn't be a blank line following the class signature.
Top-level function and class definitions should be separated by two blank lines.3 You already correctly separate method definitions by a single blank line. :)
Assignment operators should be separated by whitespace.4
In wiseowl, no method ever needs access to self.start_url (the only exception being __init__(), of course). You might as well get rid of it.
If you want to cast an object to str, just pass the object to the str constructor:

str_obj = str(obj)

Rewrite

I've removed redundant code and improved code style.

from lxml import html

import requests

class WiseOwl:
    def __init__(self, start_url):
        self.links = [start_url]
        self.storage = []

    def crawl(self):
        # Calling get_link for every link in self.links
        for link in self.links :
            self.get_link(link)

    def get_link(self, link):
        print('Crawling: ' + link)
        url = "http://www.wiseowl.co.uk"
        response = requests.get(link)
        tree = html.fromstring(response.text)

        for items in tree.xpath("//p[@class='woVideoListDefaultSeriesTitle']"):
            name = items.xpath(".//a/text()")[0]
            urls = url + items.xpath(".//a/@href")[0]
            docs = name , urls
            self.storage.append(docs)
            next_page = tree.xpath("//div[contains(concat(' ', @class, ' '), '
woPaging ')]//*[@class='woPagingItem' or @class='woPagingNext']/@href")

            for npage in next_page:
                if npage and url + npage not in self.links:
                # Avoid getting the same link twice
                    self.links += [url + npage]

    def __str__(self):
        return str(self.storage)

crawler = WiseOwl("http://www.wiseowl.co.uk/videos/")
crawler.crawl()
for item in crawler.storage:
    print(item)

References

1 PEP-8: Imports

2 PEP-8: Naming Conventions: Descriptive: Naming Styles

3 PEP-8: Blank Lines

4 PEP-8: Other Recommendations


Assigning initial value via getter or viewDidLoad I've been coding in a vacuum for several years and am trying to get up to speed on best practices, in anticipation of my eventual return to employment. I'm hoping this is the best forum in which to ask these types of questions. If not, please let me know.

My first question regards assigning the initial values of a UIViewController's properties. In general, is it better to do this in viewDidLoad or in the property's getter?

For example, I have a UIViewController with graphics that are based on the user's chosen graphics Theme. The first thing that needs to happen is to load an NSDictionary for the chosen theme, based on the current setting in NSUserDefaults.

So to start, I have:

#import "MainViewController.h"

@interface MainViewController ()
@property NSString *currentTheme;
@end

@implementation MainViewController
@synthesize currentTheme = _currentTheme;

My preferred method is to use lazy instantiation:

- (void)setCurrentTheme:(NSString *)currentTheme {
    _currentTheme = currentTheme;
}

- (NSString *)currentTheme {
    if (!_currentTheme) {
        _currentTheme = [[NSUserDefaults standardUserDefaults] objectForKey:@"currenttheme"];
    }
    return _currentTheme;
}

But since this is the first thing that needs to happen, anyway, I don't think there's any real difference in performance or utility from assigning this in viewDidLoad:

- (void)viewDidLoad
{
    [super viewDidLoad];
    self.currentTheme = [[NSUserDefaults standardUserDefaults] objectForKey:@"currenttheme"];
}

Is there a preferred way of doing this?

I would recommend to use one common method for initialization of all the variables, constants, NSUserDefaults, etc..

viewDidLoad should not be messy, it should mostly have calls to the methods.
I believe you are all aware with code reusability and code optimization techniques.


Check if two strings are permutations of each other Description:

Given two strings check if those two are permutation of each other. Its expected that input string can contain ASCII characters.

Code:

class Main {
  public static boolean isPermutation(String s1, String s2) {
    if (s1 == null || s2 == null) {
      throw new IllegalArgumentException("Input string cannot be null");
    }
    if (s1.length() != s2.length()) {
      return false;
    }
    int[] count = new int[256];
    for (int i = 0; i < s1.length(); i++) {
      int index = s1.charAt(i) - '0';
      count[index] += 1;
    }
    for (int i = 0; i < s2.length(); i++) {
      int index = s2.charAt(i) - '0';
      count[index] -= 1;
    }
    for (int i = 0; i < count.length; i++) {
      if (count[i] != 0) return false;
    }
    return true;
  }

  public static void main(String[] args) {
    //System.out.println(isPermutation(null, null) == true);
    System.out.println(isPermutation("", "") == true);
    System.out.println(isPermutation("a", "") == false);
    System.out.println(isPermutation("", "b") == false);
    System.out.println(isPermutation("a", "a") == true);
    System.out.println(isPermutation("a", "b") == false);
    System.out.println(isPermutation("foo", "bar") == false);
    System.out.println(isPermutation("eat", "ate") == true);
    System.out.println(isPermutation("1010", "1100") == true);
  }
}

Questions:

From interview point of view are the test cases sufficient?
am I using Java features correctly?
I think I can merge the two for loops but I did above coding in time
bounded fashion, can it raise red flag from interviewer perspective?

PS: According to me I can use map if the input string can contain unicode characters.

Test cases

From an interview perspective, I would suggest that you're missing the test case for the null-handling. Creating full unit test cases in an interview is overkill, but is it too much to just add:

try {
    System.out.println("Should not happen: " + isPermutation(null, ""));
} catch (IllegalArgumentException e) {
    System.out.println("Got expected exception for null");
}

Java features

Your use of Java features is OK, but you are not really doing a whole lot.

Bugs

Your spec says the strings will contain ASCII characters, and, to me, things like spaces  and punctuation -_!@#$%.... are all ASCII. Your code subtracts the 0 character value from the string's character to get the array index. This will fail for the spaces and punctuation. Why do you need to do the 0 subtraction at all? The code would work for all ASCII without that anyway.

Algorithm

My biggest suggestion is about the algorithm, anyway. Your code counts each character and compares the counts. A much simpler, and intuitive algorithm, is to sort the characters and compare the results.....

Consider:

private static String normalize(String value) {
    char[] chars = value.toCharArray();
    Arrays.sort(chars);
    return new String(chars);
}

public static boolean isPermutation(String s1, String s2) {
    if (s1 == null || s2 == null) {
      throw new IllegalArgumentException("Input string cannot be null");
    }
    return normalize(s1).equals(normalize(s2));
}


Shuffling array elements in C This is an answer to this problem.

Basically, given an array, swap the given number of elements in the array.

The solution that came to my mind: since we always swap at least two elements, pick two random indices and swap array[first_random] with array[second_random].  Then, if there's more to swap, find index that was not swapped yet and swap it with one of previously swapped.

This might be confusing a little, but it works. I am curious whether there are some other, better approaches.

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

void print_arr(int a[], int size) {
    int i;
    for (i = 0; i < size; i++) {
        printf("%d ",a[i]);
    }
        printf("\n");
}

void swap(int a[], int i, int j) {
    int temp = a[i];
    a[i] = a[j];
    a[j] = temp;
}

int next_idx(int swapped[], int s_count, int size) {
    int n, i;
    char in_arr;
    while (1) {
        in_arr = 0;
        n = rand() % size;
        for (i = 0; i < s_count; i++) {
            if (swapped[i] == n) {
                in_arr = 1;
                break;
            }
        }
        if (!in_arr) {
            break;
        }
    }
    return n;
}

void swap2_or_more(int a[], int size,int count) {
    srand(time(NULL));
    int i, j, s_count = 0;
    int swapped[size];
    i = rand() % size;
    swapped[s_count] = i;
    s_count++;
    do {
        j = rand() % size;
    } while (i == j);  // make sure indexes are different
    swapped[s_count] = j;
    s_count++;

    swap(a, i, j);
    count -= 2;
    while (count) {
        i = next_idx(swapped, s_count, size);
        j = rand() % s_count;
        swap(a, i, swapped[j]);
        swapped[s_count] = i;
        s_count++;
        count--;
    }
    printf("swapped indexes:\n");
    print_arr(swapped, s_count);
}

int main(void)
{
    int a[] = {1,2,3,4,5,6,7,8,9,10};
    swap2_or_more(a, 10, 5);
    printf("array after swap:\n");
    print_arr(a, 10);
    return 0;
}

There is a huge amount of guessing in your program, code like:

int next_idx(int swapped[], int s_count, int size) {
    int n, i;
    char in_arr;
    while (1) {
        in_arr = 0;
        n = rand() % size;
        for (i = 0; i < s_count; i++) {
            if (swapped[i] == n) {
                in_arr = 1;
                break;
            }
        }
        if (!in_arr) {
            break;
        }
    }
    return n;
}

Let's say the input data has 1000000 members, and we need to swap 10 of them... swapped[] array contains.... {1, 100, 500, 800, 1000, 10000} do we really just keep guessing until we hit a random value that is in the data? This is... a problem.

One solution which will produce a suitably random distribution of the swapped items relies on a 'simple' algorithm which does just one rand() per item, and there's no guess-work. This is done using a fisher-yates shuffle. But, for this problem, we don't shuffle the items, we shuffle the indexes... so:

void swap2_or_more(int a[], int size,int count) {
    int i, tmp;
    int swapped[size];

    // initialize the swapped array with all the data indices.
    for (i = 0; i < size; i++) {
        swapped[i] = i;
    }

    // Fisher-Yates shuffle the indices.
    // http://en.wikipedia.org/wiki/Fisher%E2%80%93Yates_shuffle
    for (int i = size-1; i > 0; i--) {
        tmp = rand() % (i + 1);
        swap(swapped, i, tmp);
    }

    // 'rotate' the values in the first count swapped/random indexes
    tmp = a[swapped[--count]];
    while(count > 0) {
        a[swapped[count]] = a[swapped[count - 1]];
        count--;
    }
    a[swapped[0]] = tmp;
}


Converting an integer to Greek and Roman numerals Yesterday I posted a question about converting an integer to it's Roman Numeral equivalent. Apparently you guys liked it and gave me some good advice, so I decided to take all your advice, and go a step further, it will now not only convert to Roman  but will now also convert to Greek numerals.

I would like some critique on this project that I'm continuing.

# <~~ coding=utf-8 ~~>
import argparse

opts = argparse.ArgumentParser()
opts.add_argument("-g", "--greek", type=int,
                  help="Convert to Medieval Numerals")
opts.add_argument("-r", "--roman", type=int,
                  help="Convert to Roman Numerals")
args = opts.parse_args()

ROMAN_NUMERAL_TABLE = (
    ("~M", 1000000), ("~D", 500000), ("~C", 100000),
    ("~L", 50000), ("~X", 10000), ("~V", 5000),  # "~" indicates a Macron
    ("M", 1000), ("CM", 900), ("D", 500),
    ("CD", 400), ("C", 100),  ("XC", 90),
    ("L", 50),   ("XL", 40),  ("X", 10),
    ("IX", 9),   ("V", 5),    ("IV", 4),
    ("I", 1)
)

GREEK_NUMERAL_TABLE = (
    ("α", 1), ("β", 2), ("γ", 3),
    ("δ", 4), ("ε", 5), ("Ϝ", 6),
    ("ζ", 7), ("η", 8), ("θ", 9),
    ("ι", 10), ("κ", 20), ("λ", 30),
    ("μ", 40), ("ν", 50), ("ξ", 60),
    ("ο", 70), ("π", 80), ("ϙ", 90),
    ("ρ", 100), ("σ", 200), ("τ", 300),
    ("υ", 400), ("φ", 500), ("χ", 600),
    ("ψ", 700), ("ω", 800), ("ϡ", 900),
    ("α", 1000), ("β", 2000), ("γ", 3000),
    ("δ", 4000), ("ε", 5000), ("ϛ", 6000),
    ("ζ", 7000), ("η", 8000), ("θ", 9000)  # The Greeks weren't very creative
)

def convert_init(number, convert_to=None):
    """ Convert a number to a numeral, Greek or Roman
    >>> print(convert_init(45, convert_to=GREEK_NUMERAL_TABLE))
    ϜϜϜϜϜϜϜγ
    >>> print(convert_init(45, convert_to=ROMAN_NUMERAL_TABLE))
    XLV """
    display_numerals = []

    for numeral, value in sorted(convert_to)[::-1]:  # sort the list from largest to least
        count = number // value
        number -= count * value
        display_numerals.append(numeral * count)

    return ''.join(display_numerals)

if __name__ == '__main__':
    if args.greek:
        data = convert_init(int(args.greek), convert_to=GREEK_NUMERAL_TABLE)
        with open("greek_numerals.txt", "a+") as file_data:
            file_data.write(data)  # Write it to a file

    elif args.roman:
        data = convert_init(int(args.roman), convert_to=ROMAN_NUMERAL_TABLE)
        with open("roman_numerals.txt", "a+") as file_data:
            file_data.write(data)

    else:
        raise NotImplementedError("{} is not implemented yet".format(args))

Key points I would like to focus on, of course critique the whole program.

I used writing to a file because when if I output it to the console I get an IO error, basically an encoding error. Does this have to do with my computer or with the program?
Would there be a way for me to condense the two tuples down into one and still convert correctly?

Your conversion routines make no sense.  I don't see why 45 should be ϜϜϜϜϜϜϜγ, as stated in your doctest.  I also get this clearly wrong result:

>>> convert_init(1939, ROMAN_NUMERAL_TABLE)
'XLXLXLXLXLXLXLXLXLXLXLXLXLXLXLXLXLXLXLXLXLXLXLXLXLXLXLXLXLXLXLXLXLXLXLXLXLXLXLXLXLXLXLXLXLXLXLXLXVIV'

Since you are using Unicode for the Greek numerals, I should be pedantic and point out that the Roman numerals should also use the appropriate Unicode characters.

The docstring would be better written as

def convert_init(number, convert_to=None):
    """
    Convert a number to a Greek or Roman numeral

    >>> convert_init(45, convert_to=ROMAN_NUMERAL_TABLE)
    'XLV'
    """
    …

… since the print statement obscures the fact that you are returning a string.  (Based on just your docstring, it could also be returning some other kind of object, whose __str__ produces the desired output.)

What is convert_init?  Did you mean convert_int?

Your args = opts.parse_args() is not guarded by if __name__ == '__main__' as it should be.  You shouldn't need to cast int(args.greek) or int(args.roman), since argparse should take care of the type=int for you.


Converting snake_case to PascalCase Is there an effective way, maybe with regex, to change the following text pattern?

I have a string like abc_def_ghi_jkl. I want to replace it to AbcDefGhiJkl.  I currently use the following code to change it. Is there a more effective way than this?

implode('',array_map('ucfirst',explode('_',$string)));

More effective solution:

str_replace('_', '', ucwords($key, '_'));

From Gears library


Bad practice to have try / catch in constructor? I have a constructor which sets a member variable. The setting of this variable throws an exception. Should I be throwing the exception in the contructor or enclosing the try/catch like below. Is below snippet an example of code smell ?

private NewManager newManager;

    public Save(final TheManager manager, final String str) {
        this.manager = manager;

        try {
            this.newManager = manager.getManager(str);
        } catch (Exception e) {
            e.printStackTrace();
        }
    }

Catching an exception and just printing stack trace and proceeding like nothing happened is certainly Very Bad. What if no one is watching the standard error stream (where e.printStackTrace() dumps its output)? In your case, you'll probably end up with NullPointerException later on, since newManager is not set. It could be one second from now, or one year from now, depending on when someone is trying to use newManager. You want to fail fast instead.

There's nothing wrong in throwing exceptions from constructors. Just make sure that, wherever you catch it, you handle it appropriately.


PagedList with large data sets I'm using X.PagedList for paging functionality in my application.  The code I have works fine but when I return larger sets of data it seems like it could be faster.  Is there anything I can do to my code to have it perform more efficiently?

Controller Code

var employees = _service.GetEmployeesByOffice(region, area, office).OrderBy(e => e.FullName);
var model = Mapper.Map<IEnumerable<Employee>,IEnumerable<EmployeesViewModel>>(employees).ToPagedList(page, 20);

var viewModel = new EmployeeListingView
{
    EmployeeViewModel = model,
    // Additional...
};

return View(viewModel);

Service Call

public IEnumerable<Employee> GetEmployeesByOffice(string region, string area, string office)
{
    var query = _context.Employees.AsQueryable();

    if (!String.IsNullOrEmpty(region))
        query = query.Where(e => e.Region == region);

    if (!String.IsNullOrEmpty(area))
        query = query.Where(e => e.Area == area);

    if (!String.IsNullOrEmpty(office))
        query = query.Where(e => e.Office == office);

    return query;
}

Razor Page

@Html.PagedListPager((IPagedList)Model.EmployeeViewModel, page => Url.Action("Index", new { area = Model.SelectedArea, office = Model.SelectedOffice, page }))

So it seems that the issue was with the mapping.  Using this answer, I created an extension to handle mapping the object after calling it.  I changed this code

var employees =
      _service
          .GetEmployeesByOffice(region, area, office)
          .OrderBy(e => e.FullName);
var model =
      Mapper.Map<IEnumerable<Employee>, IEnumerable<EmployeesViewModel>>(employees)
      .ToPagedList(page, 20);

to this

var model =
    _service
        .GetEmployeesByOffice(region, sArea, sOffice)
        .OrderBy(e => e.FullName)
        .ToPagedList(page, 20)
        .ToMappedPagedList<Employee, EmployeesViewModel>();


need some guidance for better coding practice Given a int optimumMemory and int[] array with list of available memories find the pair of indexes of memories whose sum of the values will be equal to the optimumMemory?
If there exists more than one such pair whose sum of values are equal to the optimumMemory then filter out the indexes for the below conditions
condition1: find the pair of indices which has maximumMemory
condition2: If there exists 2 pairs with same maximumMemory then select the pair with lowest index
Example: int optimumMemory = 10MB int[] availableMemories= [6,3,8,1,1,4,9,2,9,7]
The pair of indices whose sum is equal to 10MB ={(0,5),(1,9),(2,7),(3,6),(3,8),(4,6),(4,8)};
applying condition1 to the above list ={(3,6),(3,8),(4,6),(4,8)};
applying condition2 to the above list will result in only one order pair = {(3,6)}
import com.sun.tools.javac.util.Pair;

public class Test(){

public static void main(String[] args){
         int optimumMemory = 10;
         int[] list = new int[]{6,3,8,1,1,4,9,2,9,7};
         Pair<Integer,Integer> result = new Pair<>(Integer.MIN_VALUE,Integer.MIN_VALUE);
         //2 pointer approach
         for(int i=0;i<list.length;i++){
            for(int j=i+1;j<list.length;j++){
                //checking if the sum of memory is equal to optimumMemory
                if(list[i]+list[j] == optimumMemory){
                    Pair<Integer,Integer> pair = new Pair<>(i,j);
                    //this happens only once, when the very first pair that satisfies the condition is found
                    if(result.fst==Integer.MIN_VALUE && result.snd == Integer.MIN_VALUE)
                    {
                        result = pair;
                        continue;
                    }
                    int maximumMemoryInResult = Math.max(list[result.fst],list[result.snd]);
                    int maximumMemoryInPair = Math.max(list[pair.fst],list[pair.snd]);
                    //applying **condition1**
                    if(maximumMemoryInResult < maximumMemoryInPair){
                        result = pair;
                        continue;
                    }//end of condition1
                    //applying **condition2**
                    else if(maximumMemoryInResult == maximumMemoryInPair){
                        int minIndexInResult = Math.min(result.fst,result.snd);
                        int minIndexInPair = Math.min(pair.fst,pair.snd);
                        if(minIndexInPair<minIndexInResult){
                            result = pair;
                            continue;
                        }
                    }//end of condition2
                }
            }//end of j loop
        }// end of i loop
   }// end of main method
}

Is there any way to improve the code I posted?

First, lets show how I'd solve this. Of course, I've got a bit more experience with classes and such.
Note that I have left out quite a few checks, e.g. of negative sizes, invalid references and MemoryReferences without references.
package com.stackexchange.codereview;

import java.util.Optional;

public class Memories {

    public static class MemoryReferences implements Comparable<MemoryReferences> {

        private int[] memorySizes;
        private int[] refs;

        public MemoryReferences(int[] list, int ... refs) {
            this.memorySizes = list;
            this.refs = refs;
        }

        public int totalMemory() {
            int total = 0;
            for (int ref : refs) {
                total += memorySizes[ref];
            }
            return total;
        }

        private int refWithMax() {
            int refWithMax = refs[0];
            int max = memorySizes[refWithMax];
            for (int refIndex = 1; refIndex < refs.length; refIndex++) {
                int curRef = refs[refIndex];
                int curValue = memorySizes[curRef];
                if (curValue > max) {
                    refWithMax = curRef;
                    max = curValue;
                }
            }
            return refWithMax;
        }

        @Override
        public int compareTo(MemoryReferences that) {
            int totalCompared = Integer.compare(this.totalMemory(), that.totalMemory());
            if (totalCompared != 0) {
                return totalCompared;
            }

            return Integer.compare(memorySizes[this.refWithMax()], memorySizes[that.refWithMax()]);
        }

        @Override
        public String toString() {
            StringBuilder sb = new StringBuilder();
            for (int ref : refs) {
                if (sb.length() != 0) {
                    sb.append(", ");
                }
                sb.append(Integer.toString(ref));
            }

            return sb.toString();
        }
    }

    public static Optional<MemoryReferences> getLocationsWithOptimumMemory(int[] memorySizes, int optimumMemory) {
        Optional<MemoryReferences> optMem = Optional.empty();

        for (int i = 0; i < memorySizes.length; i++) {
            for (int j = i + 1; j < memorySizes.length; j++) {
                MemoryReferences curMem = new MemoryReferences(memorySizes, i, j);

                if (curMem.totalMemory() == optimumMemory) {
                    if (optMem.isEmpty()) {
                        optMem = Optional.of(curMem);
                    } else if (curMem.compareTo(optMem.get()) > 0) {
                        optMem = Optional.of(curMem);
                    }
                }
            }
        }
        return optMem;
    }

    public static void main(String[] args) {
        {
            int optimumMemory = 10;
            int[] memorySizes = new int[] { 6, 3, 8, 1, 1, 4, 9, 2, 9, 7 };

            Optional<MemoryReferences> optMem = getLocationsWithOptimumMemory(memorySizes, optimumMemory);

            if (optMem.isPresent()) {
                System.out.println(optMem.get());
            }
        }
    }
}

The main idea of this class is to get rid of the overly complex code within the loops and if statements.
It uses a little trick by referencing memorySizes (previously list). As it is a reference, it won't actually consume any additional memory. Instead of making it a pair I allowed any number of references as it was actually easier to program it that way.
It's slightly slow because it always calculates the total and such but hey, maybe memorySizes changes dynamically, right? It implements Comparable to look for the best option out there.

Great, that out of the way, let's do the review:
First of all, there is way too little white-space in the code, compare your code with mine, e.g. put spaces between the memory sizes in list.
import com.sun.tools.javac.util.Pair;

That's a Java implementation class. You should never use any classes within com.sun packages unless they have explicitly been indicated as being a public API.
int[] list = new int[]{6,3,8,1,1,4,9,2,9,7};

Please don't call an array list. And always name containers after the elements they contain.
Pair<Integer,Integer> result = new Pair<>(Integer.MIN_VALUE,Integer.MIN_VALUE);

Never use magics to indicate a bad result. Use null if you have to. But now the result will be set to this result if there isn't an optimalMemory. That's not correct, how are you going to check this?
Pair<Integer,Integer> pair = new Pair<>(i,j);

Again with the naming, what does pair mean, and a pair of what? At least call it pairOfReferences or curPairOfReferences. Don't be afraid of longer names, as long as they are functional.
 //this happens only once, when the very first pair that satisfies the condition is found

Great comment, very helpful. However it would be even better if you can code in such a way that such comments are not needed at all (hence my use of Optional in the code above).
int maximumMemoryInResult = Math.max(list[result.fst],list[result.snd]);
int maximumMemoryInPair = Math.max(list[pair.fst],list[pair.snd]);

Here you show a much better control over variable names, great!
else if(maximumMemoryInResult == maximumMemoryInPair){

Do you really need the confusing else if you just did a continue on the previous line?
int minIndexInResult = Math.min(result.fst,result.snd);
int minIndexInPair = Math.min(pair.fst,pair.snd);

Think about this: when will result.snd ever be smaller than result.fst? Same for pair?
        continue;
    }
}//end of condition2

Yeah, that continue is a bit wasted on me, we're at the end anyway.
}// end of main method

I personally dislike end of line comments with a passion. But besides that, loops and if statements should not need them to indicate where they end. You can do that by moving code out of the loop.


Sorting a list in ascending/descending order with a button toggle I have this piece of code written as a function to handle a button-toggle for sorting a list in ascending and descending order. Below is the code which is working as expected, but I would appreciate if someone knows a better, shorter and cleaner way of writing it.

  // method called every time the button is clicked
  // it will change the sorting order
  //(ASCENDING => DESCENDING => ASCENDING => DESCENDING) in that order

  onSortChange = () => {
      this.setState({
        users: this.state.order
          ? this.state.users.sort((a, b) => {
          if (a.name < b.name) return -1;
          if (a.name > b.name) return 1;
          return 0;
      })
          : this.state.users.reverse((a, b) => {
          if (a.name < b.name) return 1;
          if (a.name > b.name) return -1;
          return 0;
      }),
        order: !this.state.order,
      });
  }
```

Instead of resorting the list in state just store the preferred order in state. Then in render simply sort or reverse the order as needed to present the data. Be sure to use keys that don't change when the order is changed and it'll be quite performant.


Finding the market price for each order at the time it was placed I have two datasets. The first dataset “insideMarke1” contains the Best Bid / Best Ask (=BBA) data for a given day, starting at 8.00 am. Every time the BBA is updated, there is a new row with a new timestamp.

The second dataset “orders1” contains order data. The order entry timestamps (‘Entry date and time’) in this dataset could be older than the oldest timestamp in the “insideMarke1” dataset. Each row contains one order. There are more orders in “orders1” than BBAs in “insideMarket1” (because not every order entry leads to a BBA update).

What I want to achieve is to create a new dataset “orders1Complete”, with is a combination of both datasets: For every order in “orders1”, I want the Best Bid / Best Ask Price at the moment when the order was entered. If the order entry timestamp is older than the oldest timestamp in the “insideMarke1” dataset, I just want the oldest BBA for these orders.

Therefore, I first added a new column to the “orders1” dataset, called ‘BBA_Timestamp’. And then I merged both datasets on this column. As a want the BBA timestamp at the moment before the order was entered, it is necessary to consider that the BBA timestamp has to be older than the order ‘Entry Date and Time’ timestamp.

Below stands my code:

import pandas as pd

insideMarke1 = pd.DataFrame({'Timestamp':['2019-12-01 08:00:00.123456', '2019-12-01 08:00:01.123456', '2019-12-01 08:00:02.123456', '2019-12-01 08:00:03.123456', '2019-12-01 08:00:05.123456'],
                             'bestBidQnt':[100, 100, 50, 50, 100],
                             'bestBidPrice':[50.01, 50.01, 50.02, 50.02, 50.01],
                             'bestAskPrice':[51.00, 50.99, 50.99, 50.50, 50.50],
                             'bestAskQnt':[200, 100, 100, 200, 200]})

orders1 = pd.DataFrame({'Entry Date and Time':['2019-11-30 17:29:50.000000','2019-12-01 07:30:01.112233', '2019-12-01 08:00:00.123456', '2019-12-01 08:00:00.512341', '2019-12-01 08:00:01.123456', '2019-12-01 08:00:02.123456', '2019-12-01 08:00:02.987654', '2019-12-01 08:00:03.123456', '2019-12-01 08:00:04.000000', '2019-12-01 08:00:05.123456'],
                       'Bid':['True', 'True', 'False', 'False', 'False', 'True', 'True', 'False', 'True', 'True'],
                       'Price':[49.00, 49.50, 51.00, 51.50, 50.99, 50.02, 48.00, 50.50, 49.00, 50.01 ],
                       'Qnt':[50, 100, 200, 150, 100, 50, 10, 200, 80, 100 ]})

insideMarke1[['Timestamp']] = insideMarke1[['Timestamp']].apply(pd.to_datetime, unit='ns')
orders1[['Entry Date and Time']] = orders1[['Entry Date and Time']].apply(pd.to_datetime, unit='ns')

orders1['BBA_Timestamp'] = 0
i = 0
minInsideMarke1 = min(insideMarke1['Timestamp'])

for i in range(len(orders1['Entry Date and Time'])):
    if orders1['Entry Date and Time'][i] <= minInsideMarke1:
        orders1['BBA_Timestamp'][i] = minInsideMarke1
    else:
        insideMarke1_temp = insideMarke1.loc[(insideMarke1['Timestamp'] < orders1['Entry Date and Time'][i])]
        orders1['BBA_Timestamp'][i] = min(insideMarke1_temp['Timestamp'], key=lambda x: abs(orders1['Entry Date and Time'][i]-x))

orders1 = orders1.reset_index(drop=True)
insideMarke1 = insideMarke1.reset_index(drop=True)

insideMarke1 = insideMarke1.drop_duplicates('Timestamp', keep='last')
insideMarke1.rename(columns={'Timestamp': 'BBA_Timestamp'}, inplace=True)
insideMarke1[['BBA_Timestamp']] = insideMarke1[['BBA_Timestamp']].apply(pd.to_datetime, unit='ns')
insideMarke1 = insideMarke1.sort_values(by=['BBA_Timestamp']).reset_index(drop=True)
orders1[['BBA_Timestamp']] = orders1[['BBA_Timestamp']].apply(pd.to_datetime, unit='ns')
orders1 = orders1.sort_values(by=['BBA_Timestamp']).reset_index(drop=True)

orders1Complete = pd.merge_asof(orders1, insideMarke1, on='BBA_Timestamp')

The code works fine, but it is soooo sloooow. My data sets contain several thousand lines and I have to do this for several of those data sets...

I would be very very glad for any help, tips, advices,...


Constrained type alias What do you think about the following syntax for some very simple and intensively reusable validations?

Does this:

string name = (SomeText)"Hm…";

mean for you a none-safe cast to none-empty required text? How does it play with C# syntax/semantics? So these two should throw:

string name = (SomeText)null;
string name = (SomeText)"";

More examples:

int Age = (AdultAge)43; // OK
int Age = (AdultAge)10; // throws…

Basically, I am just looking for type alias with a guard constrain, but not so many things are possible in C#...

Let’s define the casts (yes, it is ugly):

class SomeText : Constrain<SomeText, string>
{
    public static implicit operator SomeText(string value) =>
        Create(value, !string.IsNullOrWhiteSpace(value));
}

class AdultAge : Constrain<AdultAge, int>
{
    public static implicit operator AdultAge(int value) =>
        Create(value, value >= 21);
}

Where library code is:

abstract class Constrain<TConstrain, TValue>
    where TConstrain : Constrain<TConstrain, TValue>, new()
{
    public static implicit operator TValue(Constrain<TConstrain, TValue> check) =>
        check.Value;

    protected static TConstrain Create(TValue value, bool satisfied, string message = null)
    {
        if (!satisfied)
            throw new ArgumentException(message ?? $"{typeof(TConstrain).Name} required.");
        else
            return new TConstrain() { Value = value };
    }

    TValue Value { get; set; }
}

P.S. More examples:

string email = (EMail)"test@example.com";
Stream stream = (InputStream)File.OpenText("c:\Autoexec.bat");
double weight = (Positive)160;

P.P.S. Even more examples:

class Adult
{
    public Adult(string name, int age)
    {
        Name = (SomeText)name;
        Age = (AdultAge)age;
    }

    public override string ToString() => $"{Name}, {Age}";
    string Name { get; }
    int Age { get; }
}

Your recent questions give me impression, that you are purposely trying to to find new ways to mock C# syntax. :) You know this saying from MSDN: "code is read way more often, than it is written"? I think you have to keep this in mind, when designing those things. I would definitely have a WTF-moment, if I were to see int Age = (AdultAge)43 in code. I then would have to go and look for AdultAge and Constrain implementations in order to figure out what is going on there. I would rather have this:

Validator.ValidateEmail("test@example.com");

or this:

reference.ThrowIfNull();

or even this:

var age = (Age)21;
if (age.IsAdult) ...

Because those options are easy to read and easy to understand. They feel natural. Your code does not. It's clever, but it is obscure and it is easy to get wrong:

int Age1 = (AdultAge)43; // OK
var Age2 = (AdultAge)43; // OK?
double Age3 = (AdultAge)43.1; // OK?
var Age4 = String.Format("Age: {0}", (AdultAge)43); // OK?