Generic methods to find the maximal element in part of a list This is the question from Oracle docs:

  Write a generic method to find the maximal element in the range
  [begin, end) of a list.

I had my implementation like this:

    public static <T extends Comparable> T maximalElement (List<T> list, int from, int to) {
        T max = list.get(from);
        for (int i = from + 1; i < to; i++) {
            T elem1 = list.get(i);
            if (elem1.compareTo(max) > 0) {
                max = elem1;
            }
        }
        return max;
    }

When I viewed the answer, it was something like this:

     public static <T extends Object & Comparable<? super T>> T max (List<? extends T> list,
                                                                    int begin, int end) {

        T maxElem = list.get(begin);

        for (++begin; begin < end; ++begin) {
            if (maxElem.compareTo(list.get(begin)) < 0) maxElem = list.get(begin);
        }
        return maxElem;
    }

So, I got the results running like this:

        List<Integer> c = Arrays.asList(1, 2, 3, 4, 5, 6, 7, 8, 9, 10);
        System.out.println(Test.maximalElement(c, 3, 8));
        System.out.println(Test.max(c, 3, 8));

Results are same, so I am quite excited that my version of generics is more simpler than one implemented in the answer. So, I want to check if my implemented version is really simpler and okay or am I not seeing something? Because oracle developers are not dumb enough to complex things out for nothing.

So, can you experts please help me understanding the difference or scenarios where my implemented version fails?

I'd just say to go with your implementation; just because it is easier to read! Also, you only .get() from the list once, whereas the proposed solution does it twice.

Shorter is not a synonym for better!

However, the generics are not good; you should at least use <T extends Comparable<? super T>>. Not sure why the proposed solution uses Object as a lower bound...


Pages system PHP/SQL I'm working on a CMS, currently on being able to create static pages, manage menus etc.
so.. I have This function that reads from the DB and finds all the static pages, and their position in the menu.
I want to know what Do you think.

function update_menu(&$pdo, $filepath){
$qry = "SELECT * FROM pages WHERE top = 1";
$exec = $pdo->query($qry);
$string = "
";
while($row=$exec->fetch(PDO::FETCH_ASSOC)){
    $string.= "
".$row['title']."";
        $qry = "SELECT * FROM pages WHERE parent = ?";
        $child_ex = $pdo->prepare($qry);
        $child_ex->bindValue(1, $row['id'], PDO::PARAM_INT);
        if($child_ex->execute()){
            $i=0;
            while($child=$child_ex->fetch(PDO::FETCH_ASSOC)){
                if($i==0){
                    $string.="
";
                }
                $string.="
".$child['title']."";
                $i++;
            }
            if($i != 0)
                $string.="
";
        }
    $string.="
";
}
$string.="
";
file_put_contents($filepath, $string);

};

Output html:

Page1

append1
append2

page2

append1b

page3

I think it overall is good, but there's some minor changes that could still quicken this script, namely using fetchAll and foreach instead of fetch and while

using fetchAll will get the values quicker then fetch on every single row. also foreach will prevent you from having to check if you have results, it just won't run if there are not.

also, you prepare the statement multiple times in the huge loop, instead of preparing it outside of the loop and execute() it multiple times

function update_menu(&$pdo, $filepath){
    $qry = "SELECT * FROM pages WHERE top = 1";
    $exec = $pdo->query($qry)->fetchAll(PDO::FETCH_ASSOC);
    $qryChild = $pdo->prepare("SELECT * FROM pages WHERE parent = ?");
    $string = '
' . "\n";
    foreach($exec as $row)){
        $string.= "
".$row['title'].'' . "\n";
        $qryChild->execute($row['id']);
        $childs = $qryChild->fetchAll(PDO::FETCH_ASSOC);
        $i=0;
        foreach($childs as $child){
            if($i==0){
                $string.='
' . "\n";
            }
            $string.="
".$child['title'].'' . "\n";
            $i++;
        }
    if($i != 0) $string.='
' . "\n";
    $string.='
' . "\n";
    }
$string.='
' . "\n";
file_put_contents($filepath, $string);

also, adding some "\n" will give some better readability to you html code


Web scraper for Football (Soccer) data with BeautifulSoup and Requests I wrote a web scraper to get football scores from here. I'm getting the data for all seasons for the three major German leagues. It all works at the moment, but I'm sure it's possible to make it a lot more concise.

import requests
import os
import pandas as pd
from bs4 import BeautifulSoup

# base url for scrape
base_url = "http://www.weltfussball.de/"

ligen = [["Liga1", "bundesliga-2017-2018/"],
         ["Liga2", "2-bundesliga-2017-2018/"],
         ["Liga3", "3-liga-2017-2018/"]]

def get_page(ext):
  # get the page and make soup
  global soup, page
  if (ext[0] == "/"):
    ext = ext[1:]
  url = base_url + ext
  r = requests.get(url)
  page = r.content.decode('utf-8','ignore')
  soup = BeautifulSoup(page, "lxml")

def create_dir(s):
  saison = "saison-" + s.replace("/", "-")
  os.mkdir("data/%s" % saison)

def get_seasons(liga):
  global seasons_df
  # Getting all seasons
  form = soup.find("select", attrs={"name": "saison"})
  seasons = []
  season = []
  for s in form.find_all("option"):
    season.append(s.text)
    season.append(s.get("value"))
    seasons.append(season)
    season = []
  seasons_df = pd.DataFrame(seasons, columns=["Saison", "Link"])
  seasons_df["Spiele"] = "empty"
  seasons_df["Liga"] = str(liga)

def get_games(season_n):
  tables = []
  for t in soup.find_all("table", "standard_tabelle"):
    print(t.contents)
    tables.append(t)

  data_table = tables[0]

  record = []
  records = []
  trs = data_table.findAll("tr")
  for n in range(len(trs)):
    if (trs[n].contents[1].name == "th"):
      th = trs[n].find("th")
      record.append(th.text)
    else:
      if not record:
        record.append(records[-1][0])
      tds = trs[n].findAll("td")
      for p in range(len(tds)):
        record.append(tds[p].text)
      # filling empty date column with previous value
      if (record[1] == ""):
        record[1] = records[-1][1]
      records.append(record)
      record = []

  # Pasting it into a DataFrame
  df = pd.DataFrame(records)

  # Dropping empty columns
  spdf = pd.DataFrame()
  z = 0
  for key in df.keys():
    if (df[key].all() in (" - ", "\n", "\n\n")):
      print("Column %s is empty" % key)
    else:
      print("Adding %s to game DF" % key)
      spdf[z] = df[key]
      z += 1

  # Renaming the columns
  # Defining the new column names
  names = {
      0 : "Spieltag",
      1 : "Datum",
      2 : "Zeit",
      3 : "Heim",
      4 : "Auswärts",
      5 : "Score"
  }

  spdf.rename(columns=names, inplace=True)

  seasons_df["Spiele"][season_n] = spdf

# the execution starts here
for l in range(len(ligen)):
  get_page("alle_spiele/%s" % ligen[l][1])
  get_seasons(ligen[l][0])
  # Getting the data
  for i in seasons_df.index:
    if (ligen[l][0] == "Liga1"):
      try:
        create_dir(seasons_df["Saison"][i])
      except FileExistsError:
        print("Directory %s exists already" % seasons_df["Saison"][i])
      else:
        create_dir(seasons_df["Saison"][i])
    get_page(seasons_df["Link"][i])
    get_games(i)
  ligen[l].append(seasons_df)

for l in range(len(ligen)):
  l1 = ligen[l][2]
  for s in range(len(l1["Saison"])):
    sss = l1["Saison"][s].replace("/", "-")
    sss = sss[0:9]
    l1["Spiele"][s].to_csv("data/saison-%s/%s_Spiele.csv" % (sss, ligen[l][0]), sep=";")

For example, I have to get all tables with class standard_tabelle, because there are two in the page that have no otherwise distinctive attribute.

Data Table:

      ...

Table 2:

    Aktuelle Meldungen &raquo;

I would like to select the data table based on its parent
, but there are other divs with the data class, so I can't just find div with class "data".

Major Issues

Here are some major things that require attention:

indentation (PEP8 reference). Use 4 spaces for indentation
avoid using global variables. (Why are global variables evil?). If you need to share variables between functions of your program, you can pass them as arguments, or define these functions as method of a "scraper" class which would share instance variables
variable naming. The code is much more often read than written - by choosing descriptive variable names, you are improving the readability of your program. Variables like s, ext, t, z, l, l1 etc were bad choices

Next things to address

remove redundant parenthesis. For example, if (ext[0] == "/"): can become if ext[0] == "/":
address other PEP8 code style violations, in particular:

grouping imports
the use of whitespaces around operators in expressions
blank lines between top-level functions

use if __name__ == '__main__': for the main execution block of the program
since base_url and ligen are constants, name them properly

Web-scraping improvements

since you are sending requests to the same domain multiple times, you can improve on the performance by using session.get() instead of a requests.get() where session is initialized as session = requests.Session() once and re-used (reference)
you can use SoupStrainer to let BeautifulSoup HTML-parse only the relevant part of the page
think of using CSS selectors instead of find() and find_all(). For instance, soup.find("select", attrs={"name": "saison"}) can become soup.select_one("select[name=saison]"). It is not necessarily a better way to locate elements but it is quite handy to have it in your toolbox
have you tried to use pandas.read_html() to parse the table HTML block in the get_games() function? It might work to get the table in a dataframe(s) directly

Some simplified code constructs

here is a more concise and readable way to define seasons in the get_seasons() function using a list comprehension:

seasons = [
    [season.get_text(), season.get("value")]
    for season in soup.select("select[name=saison] option")
]

Overall, though, I think it would be a good idea for this code to go through several rounds of code reviews.

Also, consider using a linting tool like flake8 or pylint, or/and a modern smart IDE like PyCharm - would help to catch a ot of stylistic and other mistakes.


Maybe type implementation This supposed to implement a Maybe type that can either hold a value T or "nothing". It's designed to work well with vectors (that's why I take extra care to noexcept constructors whenever possible). I'm governing the actual data with ::std::aligned_storage. Note: The just member function is a reference to the Haskell Maybe data-constructor Just.

Did I miss anything?

#pragma once

#include <utility>
#include <algorithm>
#include <type_traits>

namespace ads {
  template <typename T>
  class Maybe {
    public:
      typedef typename ::std::aligned_storage<sizeof(T),alignof(T)>::type storage_type;
      typedef T value_type;
    private:
      storage_type data;
      bool is_valid;
      inline T* ptr() {
        return reinterpret_cast<T*>(&data);
      }
      inline T const* ptr() const {
        return reinterpret_cast<T const*>(&data);
      }
    public:
      Maybe() noexcept : data(), is_valid(false) {}
      template <typename U>
      Maybe(U&& v) noexcept(::std::is_nothrow_move_constructible<T>::value) : data(), is_valid(false) {
        create(::std::forward<U>(v));
      }
      Maybe(Maybe const& from) noexcept(::std::is_nothrow_copy_constructible<T>::value) : data(), is_valid(false) {
        if (from.is_valid)
          create(*from.ptr());
      }
      Maybe(Maybe&& from) noexcept(::std::is_nothrow_move_constructible<T>::value) : data(), is_valid(false) {
        if (from.is_valid)
          create(::std::move(*from.ptr()));
      }
      template <typename U>
      Maybe& operator=(U&& v) noexcept(::std::is_nothrow_move_constructible<T>::value && ::std::is_nothrow_copy_constructible<T>::value && ::std::is_nothrow_assignable<T,U>::value) {
        create(::std::forward<U>(v));
        return *this;
      }
      Maybe& operator=(Maybe const& from) noexcept(::std::is_nothrow_copy_constructible<T>::value && ::std::is_nothrow_assignable<T,T>::value) {
        if (from.is_valid) {
          create(*from.ptr());
        } else {
          destroy();
        }
        return *this;
      }
      Maybe& operator=(Maybe&& from) noexcept(::std::is_nothrow_move_constructible<T>::value && ::std::is_nothrow_assignable<T,T>::value) {
        if (from.is_valid) {
          create(::std::move(*from.ptr()));
        } else {
          destroy();
        }
        return *this;
      }
      ~Maybe() noexcept(::std::is_nothrow_destructible<T>::value) {
        destroy();
      }

      bool valid() const noexcept {
        return is_valid;
      }
      operator T&() noexcept {
        return *ptr();
      }
      operator T const&() const noexcept {
        return *ptr();
      }
      T& just() noexcept {
        return *ptr();
      }
      T const& just() const noexcept {
        return *ptr();
      }

      template <typename U>
      void create(U&& v) noexcept(::std::is_nothrow_move_constructible<T>::value && ::std::is_nothrow_copy_constructible<T>::value && ::std::is_nothrow_assignable<T,U>::value){
        if (is_valid) {
          *ptr() = ::std::forward<U>(v);
        } else {
          new(&data) T(::std::forward<U>(v));
          is_valid = true;
        }
      }
      void destroy() noexcept(::std::is_nothrow_destructible<T>::value) {
        if (is_valid) {
          ptr()->~T();
          is_valid = false;
        }
      }
  };
}

Creating holes in the type system:

operator T& and it's const sibling is probably very undesirable conversion. Why, then, std::string doesn't have operator const char*()? The answer is a type system. It meant to be explicit. std::optional uses overloading of operator*() and operator->().

Don't pay for what you don't use:

I would expect constructor to not check for is_valid of itself, because it is obvious (you're calling create() from constructor, which does check for validity). In most cases this won't make a difference, but you still should strive for perfection.

Missing features:

swap
comparison operators
emplace
value_or, which is I think the most useful. Returns provided default value if there optional is not initialized yet.
one function that throws when the optional is not initialized yet. just() is a great candidate.

Style:

You could enter a newline in some places where the line is long.

I think that calling global std might be somewhat restricting. It is arguable, though.


Getting products for category I have a PR for a bugfix in which the fix is just 4 characters. In this situation we are invited to make some refactoring on the class that we are touching if possible.

Venkat Subramaniam's advice is to make lambda expressions just one line, hence I'm trying to move a block of code in a method.

Now choosing a better name come to the scene, and the challenge is to use For+ParamName or not use it on the method name.

With this advice from Google folks (Code Health: IdentifierNamingPostForWorldWideWebBlog):

  Omit words that are clear from the surrounding context.

I think that a method name getProducts(Category category) is better than getProductsForCategory(Category category), so could we avoid the For+ParamName?

Before

public class ProductProvider {

private final ProductProviderDatabaseHelper databaseHelper;

public ProductProvider(ProductProviderDatabaseHelper databaseHelper) {
    this.databaseHelper = databaseHelper;
}

public Observable<List<Product>> getProductsForCategory(Category category) {
    if (category == null) {
        return Observable.just(Collections.emptyList());
    }

    return Observable.defer(() -> {
        List<Product> result = new ArrayList<>();

        try {
            // DAO code
            result = daoProduct.query(preparedQuery);
        } catch (SQLException e) {
            Timber.e(e, "Error retrieving Products");
        }

        return Observable.just(result);
    }).subscribeOn(Schedulers.io());
}

After

public class ProductProvider {

private final ProductProviderDatabaseHelper databaseHelper;

public ProductProvider(ProductProviderDatabaseHelper databaseHelper) {
    this.databaseHelper = databaseHelper;
}

public Observable<List<Product>> getProductsForCategory(Category category) {
    if (category == null) {
        return Observable.just(Collections.emptyList());
    }

    return Observable.defer(() -> getProducts(category)
    ).subscribeOn(Schedulers.io());
}

The method name getProducts looks like a getter. However, there is some logic inside and getters usually don't have arguments. getProducts(Category category) seems a bit suspicious for me.

The name getProductsForCategory(Category category) looks too verbose: category is present three times. Why not calling it getProductsFor(Category category)? There would be no repetition and it is read straightforward.

To avoid the confusion with a getter, I also think that it could be named filterProductsBy(Category category) or retrieveProductsFor(Category category).


Insertion Sort Algorithm in Python Implementing basic sorting algorithms to learn them, and coding, better. Criticisms/ critiques welcome. Also possible optimizations.

import unittest
import random

def insertion_sort(seq):
    """Accepts a mutable sequence, utilizes insertion sort algorithm to sort in
    place. Returns an ordered list"""
    #marks point between ordered sublist & unordered list
    partition = 1

    while partition < len(seq):
        temp = partition

        #while temp not in correct pos in sublist, decrement pos
        while temp != 0 and seq[temp] < seq[temp-1]:
            seq[temp], seq[temp-1] = seq[temp-1], seq[temp]
            temp -= 1

        partition += 1

    return seq

class test_insertionsort(unittest.TestCase):
    def test_insertionsort(self):
        """Test insertion_sort()"""
        seq = [random.randrange(0, 1000) for _ in range(1000)]
        self.assertEqual(insertion_sort(seq), sorted(seq))

if __name__ == '__main__':
    unittest.main()

One thing that can speed up insertion sort is to pull the partition value to the side and slide elements up into the vacancy until the correct location is found, then drop the partition value into that location.  This eliminates one of the three assignments in the while loop:

for p in range(1, len(seq)):
    if seq[p] >= seq[p-1]:
        continue
    p_value = seq[p]
    while p != 0 and p_value < seq[p-1]:
        seq[p] = seq[p-1]
        p -= 1
    seq[p] = p_value
return seq

The unit test shows a substantial speed improvement from doing this.


Truncate decimal places In one finance application I'm working on, the requirements call that I truncate a decimal value at certain number of places and not round.

Example: 11.685 truncated to 2 decimal places should be 11.68

I wrote this extension method, but was wondering if someone knew of an easier way.

public static class DecimalExtentsions
{
    public static decimal TruncateDecimalPlaces(this decimal val, int places)
    {
        if (places <= 0)
            return Math.Truncate(val);

        var dv = Math.Pow(10, places);
        decimal part = (val % 1) * Convert.ToDecimal(dv);
        val -= (part % 1) / Convert.ToDecimal(dv);
        return val;
    }
}

One can use Math.Round() to truncate a number by removing 0.5 / 10 ^ places and then rounding

e.g
     11.685 to 2 places

Math.Round(11.685 - 0.005, 2)

The following seems to do the job

public static class NumberExtensions {
    public static decimal TruncateDecimalPlaces(this decimal val, int places) {
        if (places < 0) {
            throw new ArgumentException("places");
        }
        return Math.Round(val - Convert.ToDecimal((0.5 / Math.Pow(10, places))), places);
    }
}

[TestMethod]
public void TruncateToPlaces() {

    Assert.AreEqual(10.665m, 10.6651M.TruncateDecimalPlaces( 3));
    Assert.AreEqual(10.665m, 10.6653M.TruncateDecimalPlaces( 3));
    Assert.AreEqual(10.665m, 10.6655M.TruncateDecimalPlaces( 3));
    Assert.AreEqual(10.665m, 10.6657M.TruncateDecimalPlaces( 3));
    Assert.AreEqual(10.665m, 10.665789M.TruncateDecimalPlaces( 3));
    Assert.AreEqual(10.665m, 10.66599999M.TruncateDecimalPlaces( 3));

    Assert.AreEqual(10m, 10.66599999M.TruncateDecimalPlaces( 0));
    Assert.AreEqual(10.6m, 10.66599999M.TruncateDecimalPlaces( 1));
    Assert.AreEqual(10.66m, 10.66599999M.TruncateDecimalPlaces( 2));
    Assert.AreEqual(10.665m, 10.66599999M.TruncateDecimalPlaces( 3));
    Assert.AreEqual(10.6659m, 10.66599999M.TruncateDecimalPlaces( 4));

}


is there a better way than replace to write this function? I am doing my homework and i have done this? Is there any other better way to do this?

def write_log(name, total):
    # change user input with shape names
    name = name.replace('1', 'triangle')
    name = name.replace('2', 'square')
    name = name.replace('3', 'rectangle')
    name = name.replace('4', 'pentagon')
    name = name.replace('5', 'hexagon')
    name = name.replace('6', 'octagon')
    name = name.replace('7', 'circle')

    total = int(total)

    logfile = open("text.txt", "a")

    if (total == 1):
       towrite = ('%i %s \n'
                  % (total, name))

    elif (total >= 2 and total <= 5):
       name += 's' # add s to name to make it plural
       towrite = ('%i %s \n'
                  % (total, name))
    else:
       towrite = ''
    try:
       logfile.write(towrite)
    finally:
       logfile.close()
    return

-- One --

You can use a dictionary to hold your translation data:

mydict = {'1':'triangle', '2':'square', '3':'rectangle',
          '4':'pentagon','5':'hexagon', '6':'octagon','7':'circle'}

Now your list of replacement lines becomes simply:

for key, value in mydict.iteritems():
    name = name.replace(key, value)

another option would be:

for key in mydict:
    name = name.replace(key, mydict[key])

To make this part complete, consider also a tuple for holding your data as indicated in @larmans answer. See also comments in the answers. I think there are interesting hints about which could be the best alternative for different cases.

-- Two --

also remember you can do

2 <= total <= 5

instead of

total >= 2 and total <= 5

-- Three --

Note you dont need parenthesis in the if expressions. Just do this:

if total == 1:


Batch script to start the service in a gap of 10 min I have created a batch script to start the services in a gap of 10 min. My batch file will start the service on the local system and then will wait for 10 mins and starts the service on another system and then another. I have used the timeout command with nobreak statement which does not allows the user to stop the waiting time. If any changes need to be done, please suggest something.

@echo off
set timeout=600
set host1=Server_IP
set host1_service_name=swiftalm
set host2=Server_IP
set host2_service_name=swiftalm
set host3=Server_IP
set host3_service_name=swiftalm
set host4=Server_IP
set host4_service_name=swiftalm

echo ******Starting the Digite service on host1*****.
sc \\%host1% start %host1_service_name%
timeout /t %timeout% /nobreak
echo ******Starting the Digite service on host2*****.
sc \\%host2% start %host2_service_name%
timeout /t %timeout% /nobreak
echo ******Starting the Digite service on host3*****.
sc \\%host3% start %host3_service_name%
timeout /t %timeout% /nobreak
echo ******Starting the Digite service on host4*****.
sc \\%host4% start %host4_service_name%
timeout /t %timeout% /nobreak

This is a nice and simple script.  There's not a lot to mess up here.  Really, any embellishments to the script are personal preference, and the suggestions I or anyone else might offer are little more than opinion here.

Be that as it may, I do have a few suggestions.  As is my habit in all the other reviews I've offered thus far, I suggest a couple of general practice behavior changes.

Add setlocal to the top of every script you write.  This script uses a lot of variables.  After you exit, those variables are still hanging around, junking up your environment, potentially causing problems for other scripts that expect them not to be defined.  Adding setlocal just below @echo off will narrow the scope of those variables to this script, and the variables will be forgotten on exit (whether the script exits gracefully or not).  See this page for a more detailed explanation of why this is considered good practice.
Get into the habit whenever you set a variable to a string, to set "var=value" with the var + value pair enclosed in quotation marks.  One of these days you may need to capture special characters like & or |, or some XML or HTML to a variable.  If you set "var=<xml tags>" you don't have to worry about those special characters getting evaluated unintentionally.  Get this habit under your fingers now and you will spend much less time in the future debugging.

My script-specific suggestions may not be relevant, depending on your needs.  Really, your script is mostly fine already, and I hope you don't consider these suggestions as my being hyper-critical, or suggesting you're doing anything wrong.  Ultimately, you know what will work best for you.
Unless you have a specific reason for including it, consider removing the final line, timeout /t %timeout% /nobreak.  Does your script really need to pause for 10 minutes after executing its final action?
Since the service name is the same for all four servers, how about doing set "service=swiftalm", then sc \\%host1% start %service% with %service% in all four sc commands?  I see little reason to maintain four variables all with the same value.
If the context under which this script runs has admin privilege on the remote servers, sc is lovely for starting services remotely.  For future projects, if you ever find yourself needing to supply a username and password with the commands, wmic can perform the same task with different credentials.

set /P "user=Domain\Username? "
set /P "pass=Password? "
wmic /node:%host1% /user:%user% /password:%pass% service where 'name="%service%"' call startservice

If you wish, you can mask the input of %pass% with PowerShell (credit: Matt Williamson):

<NUL set /P "=Password? "
set "psCommand=powershell -command "$p=read-host -AsSecureString;^
$m=[System.Runtime.InteropServices.Marshal];$m::PtrToStringAuto($m::SecureStringToBSTR($p))""
for /f "usebackq delims=" %%p in (`%psCommand%`) do set "pass=%%p"

You could condense your script quite a bit by adding a for /f loop to loop through your %hostN% variables.

Example:

@echo off
setlocal
set timeout=600
set "service=swiftalm"
set "host1=Server_IP1"
set "host2=Server_IP2"
set "host3=Server_IP3"
set "host4=Server_IP4"

for /f "tokens=2 delims==" %%I in ('set host') do (

    echo ******Starting the Digite service on %%~I*****.
    sc "\\%%~I" start %service%

    if not "%%~I"=="%host4%" timeout /t %timeout% /nobreak
)

...although, admittedly, that may not as instantly readable as your code above.  It's really a matter of personal preference.
Instead of making the workstation from which you run this script perform the waiting, you could initiate a one-time scheduled task on each server, making the server do the waiting.  Because this involves datetime math, it gets a little complicated.  I typically borrow from JScript when I need to do date math.

@if (@CodeSection == @Batch) @Then

:: begin batch portion

@echo off
setlocal
set "service=swiftalm"
set "host1=Server_IP1"
set "host2=Server_IP2"
set "host3=Server_IP3"
set "host4=Server_IP4"
set /a timeout=600, loop = 1

:loop
echo;
:: start first task immediately
if %loop% equ 1 (
    echo ******Starting the Digite service on host%loop%******
    sc "\\%host1%" start %service%
    set /a loop += 1
    goto loop
)
:: else schedule for later remotely

:: use JScript because cmd is horrible at date math.
:: JScript passes stDate=date and stTime=time back to batch.
for /f "delims=" %%I in ('cscript /nologo /e:Jscript "%~f0" %timeout% %loop%') do (
    set "%%I"
)

echo ******Scheduling the Digite service to start on host%loop%******
setlocal enabledelayedexpansion
schtasks /create /tn "Start %service%" /tr "cmd /c sc start %service%" /sc once /sd %stDate% /st %stTime% /s !host%loop%! /z
endlocal

:: If more hosts in the list, increment loop counter and go back
set /a loop += 1
if defined host%loop% goto loop

:: Runtime complete.
echo All tasks complete.  Press any key to exit.
>NUL pause
goto :EOF

@end
// end batch / begin JScript hybrid code

var seconds = WSH.Arguments(0) * WSH.Arguments(1) - WSH.Arguments(0),

future = new Date(new Date().getTime() + seconds * 1000),

z = function(num) { return num < 10 ? '0' + num : num },

MM = z(future.getMonth() + 1),
DD = z(future.getDate()),
YYYY = z(future.getFullYear()),

oSH = WSH.CreateObject('wscript.shell'),
localeDateFormat = oSH.RegRead('HKCU\\Control Panel\\International\\iDate'),
localeDateSeparator = oSH.RegRead('HKCU\\Control Panel\\International\\sDate'),
localeTimeSeparator = oSH.RegRead('HKCU\\Control Panel\\International\\sTime'),

/* note: the iDate registry value can contain the following values:
VALUE   MEANING
0       mm/dd/yy
1       dd/mm/yy
2       yy/mm/dd
*/

strDate = [

    [MM,DD,YYYY],
    [DD,MM,YYYY],
    [YYYY,MM,DD]

][localeDateFormat].join(localeDateSeparator),

strTime = z(future.getHours()) + localeTimeSeparator + z(future.getMinutes());

// feed calculated "stDate=date \n stTime=time" string back to batch
WSH.Echo('stDate=' + strDate + '\nstTime=' + strTime);

Even though that's much more complicated and much uglier than your original script, it should result in a nicer user experience.  Your script will run from start to finish in a matter of seconds, rather than 30+ minutes.

Update: The above script now gets the locale date format from the registry and formats the date appropriately.

Here's another method, employing PowerShell to perform the date / time math.  It's a little slower to execute, but significantly simpler.

@echo off
setlocal
set "service=swiftalm"
set "host1=Server_IP1"
set "host2=Server_IP2"
set "host3=Server_IP3"
set "host4=Server_IP4"
set /a timeout=600, loop = 1

:loop
echo;
:: start first task immediately
if %loop% equ 1 (
    echo ******Starting the Digite service on host%loop%******
    sc "\\%host1%" start %service%
    set /a loop += 1
    goto loop
)
:: else schedule for later remotely

:: use PowerShell because cmd is horrible at date math.
set /a i = timeout * loop - timeout
for /f "tokens=1*" %%I in ('powershell -command "(Get-Date).AddSeconds(%i%) -f 'HH:mm'"') do (
    set "d=%%I"
    set "t=%%J"
)

echo ******Scheduling the Digite service to start on host%loop%******
setlocal enabledelayedexpansion
schtasks /create /tn "Start %service%" /tr "cmd /c sc start %service%" /sc once /sd %d% /st %t% /s !host%loop%! /z
endlocal

:: If more hosts in the list, increment loop counter and go back
set /a loop += 1
if defined host%loop% goto loop

:: Runtime complete.
echo All tasks complete.  Press any key to exit.
>NUL pause
goto :EOF

Ultimately, the only items of real importance that I feel strongly ought to be addressed are items 1 and 3.  Everything else is just a matter of personal coding style and preference.  Nevertheless, I hope this information is useful, or at least interesting.


Testing a Random number generator Firstly, would appreciate some code-review feedback, from a TDD and design perspective.

Secondly, what are your thoughts on implementing test case: testNumbersAreUnique()? The fact that API returns a Set of immutable objects means elements will be unique, but just wanted to know your thoughts. The reason I used a do-while as against a  while or for loop (and iterate ‘x’ times to populate ‘x’ Random numbers) is to ensure the set has correct number of elements (which will of course be unique) even if the random number gives something that is already existing in the Set.

The problem: Lottery Generator

  The goal of the program is to generate ‘x’ distinct lottery numbers within a given range 1 to ‘N’. Both x and N are int for simplicity.

Following is my code.
I wrote a controller, which will call service layer to fetch ‘x’ distinct lottery numbers:

public class Controller {
  private LotteryGeneratorService lotteryGeneratorService;
  @RequestMapping(method=RequestMethod.GET)
  public Set<Integer> getLotteryNumbers(int x){
    lotteryGeneratorService = new  LotteryGeneratorService();
    return lotteryGeneratorService.generateNRandom (x);
  }
}

This is the Service class:

public class LotteryGeneratorService {
private  Set<Integer> set = new HashSet<>();
private static int generateRandomInt(){
    Random random = new Random();
    return random.nextInt(N);
}
public Set<Integer> generateNRandom(int x){
    int tmp;
    do{
        tmp =generateRandomInt();
        if(tmp!=0)
        set.add(tmp);
    }
    while(set.size()<x);
    return set;
}
public int getSizeOfRandomlyGeneratedSet(){
    return set.size();
 }

}

The tests:

public class LotteryTest {
LotteryGeneratorService lotteryGeneratorService;
int N;
@Before
public void setup() {
    lotteryGeneratorService = new LotteryGeneratorService();
    N=100;
}
@After
public void tearDown() {
    lotteryGeneratorService = null;
}
@Test
public void testNumberOfNumbersGeneratedIsCorrect() {
    assertEquals(6, lotteryGeneratorService.generateNRandom(6).size());
}
@Test
public void testRangeIsCorrect() {
    Set<Integer> set = lotteryGeneratorService.generateNRandom(6);
    for (int i : set) {
        assertTrue(i >= 1 && i <= N);
    }
}
@Test
public void testNumbersAreUnique() {
    //..?
 }
}

About LotteryGeneratorService

The goal of the method generateRandomInt is to generate a random integer:

private static int generateRandomInt(){
    Random random = new Random();
    return random.nextInt(N);
}

This method creates each time it is called a new Random object, which is inefficient. It is better to create the Random object just once, and reuse it each time a new random integer is needed.

This does raise a new concern: this class would benefit from being thread-safe (as it is called by Spring @Controller methods). While Random is thread-safe, it may perform poorly when used concurrently, and it would be preferable to use ThreadLocalRandom in this case:

  When applicable, use of ThreadLocalRandom rather than shared Random objects in concurrent programs will typically encounter much less overhead and contention.

As such, consider refactoring this method to:

private static int generateRandomInt() {
    return ThreadLocalRandom.current().nextInt(N);
}

which makes it perform well concurrently and doesn't create unnecessary objects over and over.

I suggest adding documentation to this method so that it's clearer the range of the random integers returned. Currently, it returns random integers between 0 and N-1 included.

The second method is generateNRandom which generates x distinct random integers. First of all, its name is a bit misleading with regard to the parameters: it generates N random integers, but takes x as input. The current implementation can be improved a lot:

It generates a random integer between 0 (included) and N (excluded), but then it disregards 0 if it was generated. This could be solved by using the right bounds when fetching the random integer in the first place. For example, if you want to generate a random integer starting from 1 instead of 0, you can modify the generateRandomInt method to have ThreadLocalRandom.current().nextInt(1, N) instead.
It loops until x distinct numbers have been generated. This works well when there's a large pool of possible numbers, because more often than not, you'll pick a number that hasn't been generated yet. But when this pool starts to decrease, the time it takes to find one that hasn't been generated will increase dramatically. Consider you have a range of possible numbers from 0 to 10, and you want 10 distinct numbers; by the time you have got 9 of them, you'll have 1/10 chance of getting the 10th one, when this could be deduced automatically, since there's only one possibility remaining anyway.

There's a better approach that doesn't involve "fetch or try again". Since you have a range of N integers, and you want x distinct in it, you can shuffle that range once, and then take the first x elements. They are guaranteed to be distinct, since the source elements were distinct, and they were only shuffled; and you are guaranteed to have x of them. So you could have:

public Set<Integer> generateNRandom(int x) {
    List<Integer> list = new ArrayList<>(x);
    for (int i = 0; i < N; i++) {
        list.add(i);
    }
    Collections.shuffle(list);
    return new HashSet<>(list.subList(0, x));
}

This runs in linear-time, always. (Note that list could only be created once per N, so the creation of that list could be done outside of the method as well.). Also note that with such an implementation, you don't even need the generateNRandom method anymore.

About Controller

This is a Spring Controller, as noted by the RequestMapping annotation. In this case, you shouldn't create instances of LotteryGeneratorService yourself with

lotteryGeneratorService = new LotteryGeneratorService();

Let Spring inject that for you. Annotate LotteryGeneratorService with @Component (or @Service) and autowire it inside the controller with

@Autowired
private LotteryGeneratorService lotteryGeneratorService;

This makes the controller testable, by possibly injecting a mock service.

About LotteryTest

You do not need to set your variables to null:

@After
public void tearDown() {
    lotteryGeneratorService = null;
}

This doesn't help garbage collection, and clutters the code a bit. You should just get rid of that.

Also, since the method returns a Set, the test testNumbersAreUnique is not needed: it cannot contain duplicates, by definition of a set, so the test would be useless.


Array to a hash, ruby code reduction I have an array of thing objects, each of which has an array of categories. I'd like to build a category hash with a category ID for a key and an array of things for a value.

category_hash = Hash.new

things.each { |thing|
  thing.categories.each { |category|
    category_hash[category] = Array.new unless category_hash[category]
    category_hash[category] << thing
  }
}

Purely for education, how can this be shortened?

There are several ways of making this more idiomatic:

A trivial note, but don't use Array.new.

Use [], it's shorter, clearer, and more idiomatic
Don't use category_hash[category] = [] unless category_hash[category]

Instead, use category_hash[category] ||= []. In general you can use a ||= b instead of a = b unless a. In the case of hashes, you can skip this completely and just give the hash an appropriate default value:

category_hash = Hash.new { |hash,key| hash[key] = [] }

Don't initialize a collection to an empty state, and then iterate over another collection, appending to the new collection. Use map/each_with_object/inject/etc. to turn one collection into another collection

A more idiomatic solution might look like this:

category_hash = things.each_with_object(Hash.new {|h,k| h[k] = [] }) do |thing,hash|
  thing.categories.each { |cat| hash[cat] << thing }
end

A completely different approach would be to map each thing's categories into a new hash, and then merge the resulting hashes. The problem is that you need a "deep" merge, like the one that comes with ActiveSupport:

categories_hash = things.map do |thing|
  Hash[thing.categories.map { |cat| [cat,thing] }]
end.reduce(&:deep_merge)


Large numbers power of 10 Is there a way make the code better so that it does not have repeated code?

int totalDistance;

if (totalDistance < pow(10, 3)) {
    cout << "\nTotal (approx) travel distance = " << totalDistance << " million km\n" << endl;
}
else if (totalDistance < pow(10, 6)) {
    totalDistance = totalDistance / pow(10, 3);
    cout << "\nTotal (approx) travel distance = " << totalDistance << " billion km\n" << endl;
}
else if (totalDistance < pow(10, 9)) {
    totalDistance = totalDistance / pow(10, 6);
    cout << "\nTotal (approx) travel distance = " << totalDistance << " trillion km\n" << endl;
}

totalDistance is measured in km and minimum distance is million. My main concern is when totalDistance has reached 1,000 or 1,000,000 then instead "million" change to "billion" or "trillion".

Step 1 is always to try it without the duplicated code and see what it might look like; then you can try to fill in the blanks to make the program compile again. So:

cout << "\nTotal (approx) travel distance = "
     << english_approximation(totalDistance) << " km\n" << endl;

And then you just have to write

std::string english_approximation(long long totalDistance)
{
    if (totalDistance < 1e3) {
        return std::to_string(totalDistance) + " million";
    } else if (totalDistance < 1e6) {
        return std::to_string(totalDistance / 1e3) + " billion";
    } else if (totalDistance < 1e9) {
        return std::to_string(totalDistance / 1e6) + " trillion";
    } else {
        assert(false);  // presumably?
    }
}

Are you at all concerned that 1e3, a.k.a. 1000, is not "a million", and so on?

It is unclear to me whether totalDistance is supposed to be measured in kilometers, or meters, or what. (Megameters would make the math come out right, I suppose, but that just seems crazy.) I strongly recommend that any time you're dealing with measurements or other "unit-ful" data, you encode the expected units into the variable name, unless the expected units are already encoded in the variable's type. For example:

std::chrono::milliseconds timeout;  // excellent
int timeout_ms;  // excellent
int timeout;  // horrible

boost::units::kilometers totalDistance;  // excellent (hypothetically; this isn't actually how Boost.Units works)
int totalDistanceKm;  // excellent
int totalDistance;  // horrible


Validate and Add Service Provider/Location in C# I have a button that will validate the textbox for the service provider and the dropdown list for the locations. I have 3 tables: Locations, Service_Providers, and Service_Providers_Locations with a many-to-many relationship. But the tables that are affected are mainly Service_Providers and Service_Providers_Locations. However, upon running, the button inserts the record even if there are duplicates. What we want is to not allow duplicate record in the transaction table Service_Providers_Locations. There can be the same name of service provider, but not same location. Our code allows duplicate record insertion. We are using a three-tier approach.

protected void btnAddServiceProvider_Click(object sender, EventArgs e)
    {
        bll.Name = txtServiceProvider.Text;
        DataTable temp = bll.ServiceProviderCheckExisting();
        if (temp.Rows.Count == 0)
        {
            bll.IsDeleted = "n";
            bll.ServiceProviderAdd();
            temp = bll.ServiceProviderCheckExisting();
            bll.ServiceProviderID = temp.Rows[0][0].ToString();
            bll.LocationID = ddlLocations.SelectedValue;
            bll.IsApproved = "y";
            bll.ServiceProviderLocationAdd();
            gvServiceProviders.DataSource = bll.GetServiceProviderLocations();
            gvServiceProviders.DataBind();
        }
        else
        {
            bll.ServiceProviderID = temp.Rows[0][0].ToString();
            temp = bll.GetServiceProviderLocationUsingServiceProviderID();
            if (temp.Rows.Count == 0)
            {
                bll.LocationID = ddlLocations.SelectedValue;
                bll.IsApproved = "y";
                bll.ServiceProviderLocationAdd();
                gvServiceProviders.DataSource = bll.GetServiceProviderLocations();
                gvServiceProviders.DataBind();
            }
            else
            {
                for (int x = 0; x < temp.Rows.Count; x++)
                {
                    if (ddlLocations.SelectedValue == temp.Rows[x][1].ToString())
                    {
                        lblServiceProviderError.Text = "This service provider already exists for that location.";
                    }
                    else
                    {
                        bll.LocationID = ddlLocations.SelectedValue;
                        bll.IsApproved = "y";
                        bll.ServiceProviderLocationAdd();
                        gvServiceProviders.DataSource = bll.GetServiceProviderLocations();
                        gvServiceProviders.DataBind();
                    }
                }
            }
        }
    }

There are five lines that are repeated three times:

bll.LocationID = ddlLocations.SelectedValue;
bll.IsApproved = "y";
bll.ServiceProviderLocationAdd();
gvServiceProviders.DataSource = bll.GetServiceProviderLocations();
gvServiceProviders.DataBind();

Those should be moved to a separate method to start with. I also don't like that it does two things: update bll and then use bll to update gvServiceProviders; this looks like a mix of UI and business logic.

temp is a really bad name.

At this point we're basically here, if you apply some techniques to reduce the indentation:

protected void btnAddServiceProvider_Click(object sender, EventArgs e)
{
    bll.Name = txtServiceProvider.Text;
    DataTable temp = bll.ServiceProviderCheckExisting();

    if (temp.Rows.Count == 0)
    {
        bll.IsDeleted = "n";
        bll.ServiceProviderAdd();
        temp = bll.ServiceProviderCheckExisting();
        bll.ServiceProviderID = temp.Rows[0][0].ToString();
        Update();
        return;
    }

    bll.ServiceProviderID = temp.Rows[0][0].ToString();
    temp = bll.GetServiceProviderLocationUsingServiceProviderID();

    if (temp.Rows.Count == 0)
    {
        Update();
        return;
    }

    for (int x = 0; x < temp.Rows.Count; x++)
    {
        if (ddlLocations.SelectedValue == temp.Rows[x][1].ToString())
        {
            lblServiceProviderError.Text = "This service provider already exists for that location.";
        }
        else
        {
            Update();
        }
    }
}

(Update() is the method that contains the five lines I mentioned at the beginning. Not a  great method name either.)

Which I find to be an odd flow, to say the least. Which is partly due to not knowing what bll actually is.

To me it feels like some of the code should be re-thought. For instance this:

bll.LocationID = ddlLocations.SelectedValue;
bll.IsApproved = "y";
bll.ServiceProviderLocationAdd();

Why isn't this one single method on bll that takes the value of ddlLocations.SelectedValue as a parameter?

I'm also not fond of ServiceProviderCheckExisting() returning a DataTable, I'd expect it to be more like GetExisting().


HTML5 canvas video with React I'm trying to work HTML5 canvas video into a React application while making the code as clean as possible. What kind of improvements should be made to improve the performance utilizing the React to its fullest potential?

import React, { Component } from 'react';
import './App.css';
import video from './video.mp4';
import watermark from './watermark.png';

class App extends Component {
constructor() {
    super();
    this.state = {};
}

componentDidMount() {

const canvas = document.createElement('canvas');
canvas.width = 1280;
canvas.height = 720;
const context = canvas.getContext('2d');
context.drawImage(document.querySelector('video'), 0, 0, 720, 1280);
                    context.drawImage(
                        document.querySelector('.watermark'),
                        parseInt(document.querySelector('select').value),
                        parseInt(document.querySelector('select').value)
                    );
                    if (document.querySelector('input[name=live]').checked) {
                        this.setState({ image: canvas.toDataURL() });
                    }

}

render() {
    const range = [];
    for (let i = 0; i < 1280; i++) {
        range.push(i);
    }

    return (

            <video  src={video} controls />

                Watermark X
                <select className="positionX">
                    {range.map(i => (
                        <option key={i}>{i}</option>
                    ))} </select>

                Watermark Y
                <select className="positionY">
                    {range.map(i => (
                        <option key={i}>{i}</option>
                    ))} </select>

                Live
                <input type="checkbox" name="live" />

                <button className = "watermarkButton watermarkSubmit"
                    onClick={() => {
                        const canvas = document.createElement('canvas');
                        canvas.width = 1280;
                        canvas.height = 720;
                        const context = canvas.getContext('2d');
                        context.drawImage(document.querySelector('video'), 0, 0, 1280, 720);
                        context.drawImage(
                            document.querySelector('.watermark'),
                            parseInt(document.querySelector('.positionX').value),
                            parseInt(document.querySelector('.positionY').value)
                        );
                        this.setState({ image: canvas.toDataURL() });
                    }}> Watermark! </button>

    );
                }

            }

export default App;

For loop inside Render()

The loop gets called each time the component re-renders, it's a 1280 pushes to an array and that can be a performance issue, you can set it in componentDidMount or even outside the class since it's not dependent on its data,

Consider Array.from to create and fill the array without creating an intermediate one.

const range = Array.from({length: 1280}, (_, i) => i);

And since you're filling it with ints and the select value is an int you don't really need parseInt.

Using document.querySelector to read values

  Basically, you can't easily rely on the input field because the state needs to come from
  the React app's state, not from the browser's idea of what the value should be.

See more explanation in this blog post.

You should instead use the state to store the values of the inputs. ( see code snippet below )

Accessing the DOM using document.querySelector

  In React, we do not directly manipulate the actual DOM. Instead, we must manipulate the virtual representation and let React take care of changing the browser's DOM.

Check this article for in depth explanation and the different ways to access the DOM.

Using refs, instead of doing document.querySelector('video'), you can add a ref like : <video src={video} controls ref={video => this.video = video}/> then use this.video to do what you would do with a regular html element.

Using html5 canvas with react

import React, { Component } from 'react';
import './App.css';
import video from './video.mp4';
import watermark from './watermark.png';

const range = Array.from({ length: 1280 }, (_, i) => i);

class App extends Component {
  constructor() {
    super();
    this.state = {
      select: 0,
      live: false,
      positionX: 0,
      positionY: 0,
      checked: false
    };
  }

  componentDidMount() {
    const context = this.canvas1.getContext('2d');
    context.drawImage(this.video, 0, 0, 720, 1280);
    context.drawImage(this.watermark, this.state.select, this.state.select);

    if (this.state.live) {
      this.setState({ image: canvas.toDataURL() });
    }
  }

  render() {
    return (

        <video src={video} controls ref={video => this.video = video} />

            Watermark X
              <select
                className="positionX"
                onChange={value => this.setState({ select: value, positionX: value })}
                value={this.state.positionX}>
                {range.map(i => (
                  <option key={i} value={i}>{i}</option>
                ))}
              </select>

            Watermark Y
              <select
                className="positionY"
                onChange={value => this.setState({ select: value, positionY: value })}
                value={this.state.positionY}>
                >
                {range.map(i => (
                  <option key={i} value={i}>{i}</option>
                ))}
              </select>

            Live
            <input
              type="checkbox"
              name="live"
              checked={this.state.live}
              onChange={e => this.setState({ live: e.target.value })}
            />

          <button className="watermarkButton watermarkSubmit"
            onClick={() => {
              const context = this.canvas2.getContext('2d');
              context.drawImage(this.video, 0, 0, 1280, 720);
              context.drawImage(this.watermark, this.state.positionX, this.state.positionY);
              this.setState({ image: canvas.toDataURL() });
            }}> Watermark! </button>

          <canvas ref={canvas => this.canvas1 = canvas} width={1280} height={720} />
          <canvas ref={canvas => this.canvas2 = canvas} width={1280} height={720} />

           this.watermark = watermark}
          />

    );
  }

}

export default App;


Avoiding dynamic RegEx creation in JavaScript This function's job is to replace several text smilies, e.g. :D, :), :love with the appropriate smiley image.
In my opinion my code has several issues, yet the main problem is that a lot of quite similar regexes are created dynamically: one for each different smilie replaced.
As you might expect, replacing smilies in a text is a job done kindly often, so this is indeed code that is executed a lot of time, therefore I think optimizing that makes sense.

Well, why didn't I just call text.replace()?
I don't want the smilies to be replaced if they are enclosed by backticks: `

var replaceTextSmilies = function() {
    var smiliesShortcut = {};
    smiliesShortcut[':)'] = 'smile.png';
    /* adding much more */

    var smiliesWordy = {};
    smiliesWordy[':angry'] = 'angry.png';
    /* adding much more */

    function escapeRegExp(str) {
        return str.replace(/([.?*+^$[\]\\(){}|-])/g, "\\$1");
    }

    return function(text) {
        Object.keys(smiliesWordy).forEach(function(entry) {
            var image = '';

            if(text.indexOf(entry) !== -1) {
                var regex = new RegExp('('+ escapeRegExp(entry) +')(?=(?:(?:[^`]*`){2})*[^`]*$)', 'g');
                text = text.replace(regex, image);
            }
        });
        Object.keys(smiliesShortcut).forEach(function(entry) {
            var image = '';

            if(text.indexOf(entry) !== -1) {
                var regex = new RegExp('('+ escapeRegExp(entry) +')(?=(?:(?:[^`]*`){2})*[^`]*$)', 'g');
                text = text.replace(regex, image);
            }
        });
        return text;
    };
}();

You can store the regex as part of the smily metadata.

var replaceTextSmilies = function() {
    var smiliesShortcut = {};
    smiliesShortcut[':)'] = {
        img : 'smile.png'
    };
    /* adding much more */

    var smiliesWordy = {};
    smiliesWordy[':angry'] = {
        img : 'angry.png'
    };
    /* adding much more */

    addRegex(smiliesShortcut);
    addRegex(smiliesWordy);

    function escapeRegExp(str) {
        return str.replace(/([.?*+^$[\]\\(){}|-])/g, "\\$1");
    }

    function addRegex(smilies) {
        Object.keys(smilies).forEach(function(smily) {
            smilies[smily].regex = new RegExp('(' + escapeRegExp(smily) + ')(?=(?:(?:[^`]*`){2})*[^`]*$)', 'g');
        });
    }

    function replace(text, smilies) {
        Object.keys(smilies).forEach(function(entry) {
            var image = '<img src="img/smilies/' + smilies[entry].img + '"/>';

            if (text.indexOf(entry) !== -1) {
                text = text.replace(smilies[entry].regex, image);
            }
        });
        return text;
    }

    return function(text) {
        text = replace(text, smiliesWordy);
        text = replace(text, smiliesShortcut);
        return text;
    };
}();

Demo: Fiddle

Note: You can make it even better if you can manually create the regex and assign them instead of using addRegex() like

    smiliesShortcut[':)'] = {
        img : 'smile.png',
        regex: /<the regex>/
    };


Query for combining results of same query running across multiple databases I wrote a query to run the same query across multiple databases and combine the results.  While it seems plenty quick I was wondering if there is a better way to do this.

create table #serverlist(
      ID smallint IDENTITY(1,1),
      dbName varchar(50)
      )

create table #browsercounts(
      ID smallint IDENTITY(1,1),
      --Email varchar(50),
      Browser varchar(50),
      Counts int)

insert into #serverlist
select name from sys.databases
where name like '%Test2Portal%'
and name not like '%_Test%'

Declare @counter int, @rows int
set @counter = 1
set @rows = (select COUNT(dbName) from #serverlist)

while (@counter <= (@rows))
Begin
      Declare @SQL varchar(1000)
      Declare @database varchar(50) =
        (select dbName from #serverlist where ID = @counter)

      Select @SQL = 'select Browser, COUNT(Browser) as Counts from ' +
        @database+ '.dbo.Session where Browser is not null group by Browser'

      insert into #browsercounts
      Exec (@SQL)

      set @counter += 1

End

Select * From #browsercounts

drop table #serverlist
drop table #browsercounts

You probably don't need any temp tables unless you need access to these temp tables across different stored procedures, here is how I would write it.

DECLARE @sql VARCHAR(MAX)

SELECT  @Sql = COALESCE(@sql + ' UNION ALL ', '') + 'SELECT [' + name + '].dbo.Session.Browser, COUNT(['+name+'].dbo.Session.Browser) AS Counts FROM [' + name + '].dbo.Session WHERE [' + name + '].dbo.Session.Browser IS NOT NULL GROUP BY ['+name+']dbo.Session.Browser'
from sys.databases
where name like '%Test2Portal%' and name not like '%_Test%'

EXEC @sql --this will perform the select for you

hope this is helpful


Validation null values (preconditions) I would like to know an alternative, more elegant way to write the following methods.

I am especially not enthusiastic of the nested if statement or if for validate null values.

Now there are 3 if statement for null validation.

Any method:

   private void LoadAlertas()
        {
            // *** OMITTED ***

            // Nullable object must have a value
            var PortalUser = HttpContext.Current.User.PortalUser();
            if (string.IsNullOrWhiteSpace(PortalUser)) throw new ArgumentNullException("HttpContext.Current.User.PortalUser is null");
            var personalData = PersonalData.getPersonalData(PortalUser);
            if (personalData == null) throw new ArgumentNullException("PersonalData.getPersonalData is null para " + PortalUser);
            if (!personalData.iID_PersonalData.HasValue) throw new ArgumentNullException("personalData.iID_PersonalData is null");

            var userLogado = Bll.PortalUser.getUsarioPortalByIdPersonalData(personalData.iID_PersonalData.Value);

Another method:

 protected void Btn_Accion_Click(object sender, EventArgs e)
        {
            // *** OMITTED ***

            // Nullable object must have a value
            var PortalUser = Page.User.PortalUser();
            if (string.IsNullOrWhiteSpace(PortalUser)) throw new ArgumentNullException("Page.User.PortalUser is null");
            var personalData = MyPortal.AdminManager.Bll.PersonalData.getPersonalData(PortalUser);
            if (personalData == null) throw new ArgumentNullException("PersonalData.getPersonalData is null para " + PortalUser);
            if (!personalData.iID_PersonalData.HasValue) throw new ArgumentNullException("personalData.iID_PersonalData is null");

            var userLogado = MyPortal.AdminManager.Bll.PortalUser.getUsarioPortalByIdPersonalData(personalData.iID_PersonalData.Value);
            var userConsultado = setuserConsultado();

Why are you using ArgumentNullException? PortalUser isn't an argument, personalData isn't an argument, etc.

PortalUser should be camelcase.
getPersonalData should be PascalCase. Ditto getUsarioPortalByIdPersonalData. Ditto setuserConsultado (and "User" should be capitalized).
iID_PersonalData should be PascalCase; moreover it seems to be using Hungarian notation which is against the C# standards, and it contains an underscore which is also against the C# standards.

I can't say much about the code, since there's so little of it. It doesn't look that good, to be honest, and I'd certainly move it to a specific method since both blocks seems to be part of a much longer method.


User ID & password constraints checker Please take a look at my code below and tell me if I'm doing the right thing here. My wishes:

Username

The username must be at least 6 characters long
The username can't contain any special symbols

Password:

The password must be at least 8 characters
The password must contain at least 1 upper- and lowercase letter
The password must contain at least 1 number
The password can't contain any special symbols

As you can see I am also using a hash function for the password. It is supposed to be hashed like this for the purpose I'm using it for. Does this affect my code at all?

//password hash function
function l2j_hash($password) {
    return base64_encode(pack("H*", sha1(utf8_encode($password))));
}

$pass = mysqli_real_escape_string($db_link, $_POST['password']);
$repass = mysqli_real_escape_string($db_link, $_POST['repeat_password']);
$user = mysqli_real_escape_string($db_link, $_POST['name']);

$uppercase = preg_match('@[A-Z]@', $pass);
$lowercase = preg_match('@[a-z]@', $pass);
$number    = preg_match('@[0-9]@', $pass);

if(!$uppercase || !$lowercase || !$number || !ctype_alnum($password) || strlen($password) < 8) {

    echo "The password must contain at least 1 lowercase letter, 1 uppercase letter and 1 number.";
    echo "The password can't contain special symbols.";

} else if ($pass != $repass) {
    echo "Passwords do not match";
} else if ($user == '' && strlen($user) < 6 && !ctype_alnum($user)) {
    echo "The User ID must be at least 6 characters long and can't contain special symbols.";
} else {
    $pass_hash =  l2j_hash($pass);
    $db_add = mysqli_query( "INSERT INTO `accounts` VALUES ('$user', '$pass_hash', '0', '0', '')" ) or die( 'Error: '.mysqli_error() );
    echo 'Account created';
}

In no particular order:

That hashing method is terrible and insecure. Hashes must be costly (slow) most of all, and they must contain a unique salt. Use password_hash, don't invent your own.

If that is the required hashing method: move away from it ASAP. But at the very least, utf8_encode is entirely superfluous, since you're only allowing ASCII characters to begin with and it won't do anything in that case.

Don't validate strings you have already altered (here: after mysqli_real_escape_string).
You're already using mysqli, use prepared statements rather than tedious and error prone escaping.
Disallowing "special characters" in passwords makes them weaker, not stronger. Unless you have strong business reasons for this restriction (which legitimately may exist), don't limit the allowed character set.
Use more functions to make your code more readable.
Name your SQL columns, don't rely on the implicit order.
Use DEFAULT values in your database table definitions instead of passing default values through the query, if possible.
Check whether your SQL query succeeded (and/or use mysqli's exception error mode); presumably you have a UNIQUE constraint on the username, so the query may legitimately fail, and your code doesn't even know it.

function validateUsername($name) {
    return ctype_alnum($name) && strlen($name) >= 6;
}

function validatePassword($str) {
    return ctype_alnum($str)
        && strlen($str) >= 8
        && preg_match('/[A-Z]/', $str)
        && preg_match('/[a-z]/', $str)
        && preg_match('/[0-9]/', $str);
}

function createUser(mysqli $db, $name, $password) {
    $stmt = $db->prepare('INSERT INTO `accounts` (`name`, `password`) VALUES (?, ?)');
    $stmt->bind_param('ss', $name, password_hash($password, PASSWORD_DEFAULT));
    return $stmt->execute();
}

if (!validateUsername($_POST['name'])) {
    echo 'Invalid name';
} else if (!validatePassword($_POST['password'])) {
    echo 'Invalid password';
} else if ($_POST['password'] !== $_POST['repeat_password']) {
    echo "Passwords don't match";
} else if (!createUser($db_link, $_POST['name'], $_POST['password'])) {
    echo 'Something went wrong';  // add better error handling here
} else {
    echo 'Account created';
}

Of course, you'll probably want to collect all the errors and output them next to the actual <input> elements when you inform the user about errors, instead of just failing on the first error that is produced. That's a bit too broad to tackle here though. And this could all be further improved with OOP or other larger architectural choices of course…


Python hash format converter This program should convert a file containing Mana, Hashcat or John the Ripper NTLMv1 hashes to another file containing Hashcat or John the Ripper hashes. Do you have any suggestions on how to make the code cleaner? The hash format selection code feels kinda dirty.

#!/usr/bin/env python3

# Takes a file of NTLMv1 hashes in mana format and spits out a
#  file of hashes in JtR or hashcat format.

import sys
import re

class Hash():
    pass

class HashcatHash(Hash):
    @staticmethod
    def parse(line):
        m = re.match("(.*?)::::([0-9a-f]{48}):([0-9a-f]{16})", line)
        if m:
            return {"username": m.group(1), "response": m.group(2), "challenge": m.group(3)}
        else:
            raise ValueError("Couldn't find hash in line")

    @staticmethod
    def format(d):
        return "{username}::::{response}:{challenge}".format(
            username=d["username"],
            response=d["response"],
            challenge=d["challenge"])

class JohnHash(Hash):
    @staticmethod
    def parse(line):
        m = re.match("(.*?):\$NETNTLM\$([0-9a-f]{16})\$([0-9a-f]{48})", line)
        if m:
            return {"username": m.group(1), "response": m.group(3), "challenge": m.group(2)}
        else:
            raise ValueError("Couldn't find hash in line")

    @staticmethod
    def format(d):
        return "{username}:$NETNTLM${challenge}${response}".format(
            username=d["username"],
            response=d["response"],
            challenge=d["challenge"])

class ManaHash(Hash):
    @staticmethod
    def parse(line):
        m = re.match("CHAP\|(.*?)\|([0-9a-f:]{23})\|([0-9a-f:]{71})", line)
        if m:
            return {"username": m.group(1), "response": remove_colons(m.group(3)), "challenge": remove_colons(m.group(2))}
        else:
            raise ValueError("Couldn't find hash in line")

    @staticmethod
    def format(d):
        raise NotImplementedError

def print_usage():
    print("Usage:")
    print("exportlog.py <informat> <infile> <outformat> <outfile>")

def remove_colons(hexstr):
    return ''.join(hexstr.split(':'))

if __name__ == '__main__':
    if (len(sys.argv) != 5) or (sys.argv[1] not in ['john', 'hashcat', 'mana']) or (sys.argv[3] not in ['john', 'hashcat']):
        print_usage()
        quit()
    import_format = sys.argv[1]
    export_format = sys.argv[3]
    with open(sys.argv[2], 'r') as infile, open(sys.argv[4], 'w') as outfile:
        for line in infile:
            if import_format == 'john':
                d = JohnHash.parse(line)
            elif import_format == 'hashcat':
                d = HashcatHash.parse(line)
            elif import_format == 'mana':
                d = ManaHash.parse(line)
            if export_format == 'john':
                outline = JohnHash.format(d)
            elif export_format == 'hashcat':
                outline = HashcatHash.format(d)
            outfile.write(outline + "\n")

First of all, you can improve on reading the command-line arguments and, instead of manually parsing the sys.argv, use argparse module. Something like:

import argparse

def parse_args():
    """Parses command-line arguments."""

    parser = argparse.ArgumentParser()

    parser.add_argument('infile', type=argparse.FileType('r'))
    parser.add_argument('informat', action='store', choices=HASH_FORMATS)
    parser.add_argument('outfile', type=argparse.FileType('w'))
    parser.add_argument('outformat', action='store', choices=HASH_FORMATS)

    return parser.parse_args()

if __name__ == '__main__':
    args = parse_args()
    # ...

The format selection logic can be simplified if you would use a dictionary:

HASH_FORMATS = {
    'john': JohnHash,
    'hashcat': HashcatHash,
    'mana': ManaHash
}

# get the input and output hash classes beforehand
input_hash_class = HASH_FORMATS[args.informat]
output_hash_class = HASH_FORMATS[args.outformat]

for line in args.infile:
    parsed_line = input_hash_class.parse(line)
    converted_line = output_hash_class.format(parsed_line)
    args.outfile.write(converted_line)

And, as a side note about defining the Hash-based classes - I think you can make use of Abstract Base Classes with abstract methods which may lead to a cleaner object-oriented design.


Greasemonkey script to auto fill the values of a form on a specified page I created a Greasemonkey script to allow me to quickly fill a specific form on a specific URL, for testing/dev purposes. If the current URL matches any of the urls found in the formsDefault array a button is added to the page that allows the user to auto fill with the default values.

This is my first time using arrow functions in JS.

/*jshint esversion: 6 */

(function($) {
    'use strict';

    var formDefaults = [
        {
            urlPattern : '/profile/user',
            valueSet : [
                { selector : '#username', value : 'bill' },
                { selector : '#lastname', value : 'miller' },
                { selector : '#auth_pin', value : '1234' },
            ]
        },
        {
            urlPattern : '/profile/company',
            valueSet : [
                { selector : '#companyname', value : 'bill' },
                { selector : '#state', value : 'washington' },
            ]
        },
    ];

    var formFiller = {
        valueSet : null,

        run(formDefaults) {
            var valueSetObj = this.tryEachPattern(this);
            if (valueSetObj !== null) {
                this.valueSet = valueSetObj.valueSet;
                this.showFormFillButton(this);
            }
        },

        tryEachPattern : (_this) => {
            var valueSetObj = null;
            for (var i = formDefaults.length - 1; i >= 0; i--) {
                valueSetObj = formDefaults[i];
                var matched = _this.matchUrl(_this, valueSetObj.urlPattern);
                if (matched === true)
                {
                    break;
                }
            }
            return valueSetObj;
        },

        matchUrl : (_this, urlPattern) => {
            var matched = !!location.href.match(urlPattern);
            return matched;
        },

        fillForm : (_this) => {
            var valueSet = _this.valueSet;
            for (var i = valueSet.length - 1; i >= 0; i--) {
                var value = valueSet[i];
                $(value.selector).val(value.value);
            }
        },

        showFormFillButton : (_this) => {
            var button = $('<button type="button" style="position: absolute; right: 10px; bottom: 10px">Auto Fill</button>');
            button.on('click', function(event) {
                event.preventDefault();
                _this.fillForm(_this);
            });
            $('body').append(button);
        },
    };

    // Ready, set, go
    formFiller.run(formDefaults);

})(jQuery);

While your code is generally easy to read and understand, I believe you have slightly over complicated things, there's no need to define an object with several methods to autocomplete a form.

Before I get to that though, let's take a look at your code.

Good work with encapsulating all of your code in an IIFE and forcing strict mode.
formDefaults is a decent name, I was able to guess almost exactly what it would contain when I first read it. However, since it also contains the page url, I would rename this to pages. urlPattern is also decent, though I expected it to be a regex due to the pattern suffix. valueSet on the other hand gives me absolutely no information about what it contains. I'd recommend renaming this to something like defaults.
The run method accepts a formDefaults parameter, but doesn't use it. It calls tryEachPattern and tryEachPattern uses the variable declared outside formFiller. If you take parameters, you should use them.
Using arrow functions doesn't actually help you here. Every single arrow function in this code takes a _this parameter. Arrow functions are supposed to make working with this easier, not more cumbersome! Just use regular functions if you need to pass a _this argument.
tryEachPattern is more complex than it needs to be. There is an Array.find method which can find a match in an array for you and returns undefined if the value was not found.
Don't explicitly compare boolean values with ===, in tryEachPattern you can just do if (matched) which is easier to read.
Don't assign a result to a variable just to return that variable. var matched = ...; return matched can just be written as return ....
Don't use String.match to test if a string contains another string. Use String.includes.
Prefer const and let to var. const is especially helpful as it tells readers that the assigned value will not be changed (though it could be mutated).
In fillForm, there's no need to write a bounded for loop. Prefer for...of pattern or the Array.forEach call to make it easier to immediately know you are looping over everything.
I don't see any reason that the click handler should call event.preventDefault(), the button won't cause any navigation so it shouldn't be necessary unless the page does something odd.
If the page is already including jQuery, it makes sense to use it here as it simplifies the creation of the auto fill button. Otherwise, it only takes like 4 more lines of code and I would recommend dropping the jQuery dependency to speed up your script.
jQuery provides a .click method as a shorter alternative to .on('click', ...
Destructuring is an amazing tool that can make code easier to read by giving immediate access to desired properties on an object.

At the start I mentioned that this was more complicated than it needed to be. Here is how I would write an equivalent script.

/*jshint esversion: 6 */
(function ($) {
    'use strict';

    const pages = [
        {
            url: '/profile/user',
            defaults: [
                { selector: '#username', value: 'bill' },
                { selector: '#lastname', value: 'miller' },
                { selector: '#auth_pin', value: '1234' },
            ]
        },
        {
            url: '/profile/company',
            defaults: [
                { selector: '#companyname', value: 'bill' },
                { selector: '#state', value: 'washington' },
            ]
        },
    ];

    const page = pages.find(({ url }) => location.href.includes(url));

    if (page) {
        const button = $('<button type="button" style="position: absolute; right: 10px; bottom: 10px">Auto Fill</button>');
        button.click(() => {
            for (const { selector, value } of page.defaults) {
                $(selector).val(value);
            }
        });
        $('body').append(button);
    }

})(jQuery);


Translating from English to Entean I wrote a python script that converts English to Entean language from an anime called "The Devil is a part timer". I am sloppy at coding and know that the script can be made better.

In the anime, the language in Ente Isla is called Entean, which is basically most of the consonants in the English alphabet switched around. Vowels and the consonants 'L', 'N', 'Q' keep their original position. ABCDEFGHIJKLMNOPQRSTUVWXYZ is changed to AZYXEWVTISRLPNOMQKJHUGFDCB.
    For example, 'human' is 'tupan', and 'world' becomes 'foklx'.

Here is the code:

alphabet = "ABCDEFGHIJKLMNOPQRSTUVWXYZ".lower()
switched = "AZYXEWVTISRLPNOMQKJHUGFDCB".lower()

usr_txt = input("Enter Englsih text:")

print(usr_txt)

def get_letter(eng_letter):
    if alphabet.find(letter):
        if eng_letter.isupper():
            alpha_place = alphabet.find(letter.lower())
            return(switched[alpha_place].upper())
        else:
            alpha_place = alphabet.find(letter.lower())
            return(switched[alpha_place])
    else:
        return(eng_letter)

ent_word = ""
ent_output = ""

for word in usr_txt.split(" "):
    ent_word = ""
    for letter in word:
        ent_letter = get_letter(letter)
        ent_word += ent_letter
    ent_output += " " + ent_word

print(ent_output.strip())

If you interested the updated code can be found at https://github.com/Logmytech/English-to-Entean

Use Dictionaries

We are basically just mapping from one letter to the next. The most naturally way to do that is to actually use a map:

to_entean = {...}

The easiest way to build up the dictionary is to use zip():

to_entean = {eng: ent
    for eng, ent in zip(string.ascii_uppercase, 'AZYXEWVTISRLPNOMQKJHUGFDCB')
    }

Now to_entean['A'] == 'A' and to_entean['B'] == 'Z', etc. This simplifies your get_letter function to:

def get_letter(letter):
    ent = to_entean.get(letter.upper(), letter)
    if not letter.isupper():
        ent = ent.lower()
    return ent

String Concatentation

String concatenation is slow. Also it reads terribly. What we want to do is call get_letter for every letter in word and then join them all together. That's what join() is for:

ent_word = ''.join(get_letter(letter) for letter in word)

Why split?

Since get_letter() handles spaces already, we don't have to split into words. Just transform the whole thing:

print(''.join(get_letter(letter) for letter in usr_txt))

Which, if you prefer, can also be spelled:

print(''.join(map(get_letter, usr_txt)))

Typo

You have a typo here:

usr_txt = input("Enter Englsih text:")
                       ^^^^^^^


Simple virtual machine in Rust About

I'm trying to implement a simple virtual machine. Now it supports only three operations:

ADD(reg0, reg1, reg2) -> reg0 = reg1 + reg2
LOAD(reg, value) -> reg = value
EXIT -> finish process

It has 8 registers and no memory (for now).

Source code

main.rs:

use std::process;

mod opcodes;
mod utils;

const REGISTERS_COUNT: usize = 8;

#[derive(Copy, Clone)]
struct Program<'a> {
    code: &'a Vec<u8>,
    pos: usize
}

impl<'a> Program<'a> {
    pub fn new(code: &'a Vec<u8>) -> Program<'a> {
        Program { code: code, pos: 0 }
    }

    pub fn fetch(&mut self) -> u8 {
        self.pos += 1;
        if self.pos > self.code.len() {
            process::exit(0);
        }
        return self.code[self.pos - 1];
    }
}

fn eval(mut program: Program, mut registers: [u64; REGISTERS_COUNT]) {
    loop {
        let operator = program.fetch();
        let operands = (
            program.fetch(),

            vec![
                program.fetch(), program.fetch(),
                program.fetch(), program.fetch(),
                program.fetch(), program.fetch(),
                program.fetch(), program.fetch()
            ]
        );

        match operator {
            opcodes::EXIT => {
                process::exit(0);
            }
            opcodes::LOAD => {
                registers[operands.0 as usize] = utils::concat_bytes(&operands.1);
            }
            opcodes::ADD => {
                registers[operands.0 as usize] = registers[utils::concat_bytes(&operands.1[..4]) as usize]
                    + registers[utils::concat_bytes(&operands.1[4..]) as usize];
            }
            _ => {
                println!("error: invalid instruction code");
                process::exit(1);
            }
        }

        println!("{:?}", registers);
    }
}

fn main() {
    let registers: [u64; REGISTERS_COUNT] = [0; REGISTERS_COUNT];
    let code = vec![
        opcodes::LOAD,
            0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x64,
        opcodes::LOAD,
            0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xC8,
        opcodes::ADD,
            0x02, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01,
        opcodes::EXIT,
            0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
    ];
    let program = Program::new(&code);

    eval(program, registers);
}

utils.rs:

pub fn concat_bytes(bytes: &[u8]) -> u64 {
    let mut result = bytes[0] as u64;
    for i in 1..bytes.len() {
        result = (result << 8) | (bytes[i] as u64)
    }
    return result;
}

opcodes.rs:

pub const EXIT: u8 = 0x00;
pub const LOAD: u8 = 0x01;
pub const ADD:  u8 = 0x02;

Output

[100, 0, 0, 0, 0, 0, 0, 0]
[100, 200, 0, 0, 0, 0, 0, 0]
[100, 200, 300, 0, 0, 0, 0, 0]

Disadvantages:

No memory
Only integers
All operators require the same number of arguments (even if they don't need any, like EXIT)

Anything else?

Document your instruction format! Looks to be opcode,
destination, 8 bytes of input. You need to describe the endianness
of the data though.  Comments are an OK first step, but data
structures with self-describing names are probably even better.
You could avoid entire classes of errors by splitting the
program and data memory. This is "Harvard" vs "von Neumann"
architecture.
Remove unneeded type specification, such as when defining
registers in main.
Can use Default::default to get a reasonable initial state,
such as a vector of zeroes. Prevents having to re-type the length
again.
Don't use explicit return statements at the end of functions.
What is the expected behavior if you concat a slice with more
than 8 bytes? I made this limit to the first 8 bytes.
Become intimately familiar with the methods on Iterator and try to leverage the functional tools it provides, like fold.
Avoid slice indexing whenever possible. It's slightly slower as
it performs bounds checks.
concat is a bit odd of a name. I feel that pack is more
typical for this operation.
Your opcode is crying out to be an enum. This also happens to be
a great place to hang behavior, such as converting it to/from an
integer.
It's a little strange that eval accepts the program and
registers by value. Maybe you want to create a Machine that owns
them, or accept them by mutable reference?
It's strange that Program contains a pointer to where the data
is. Typically, the program should be immutable and you'd have a
Program Counter (PC) that tracks where you are in the program.
Printing the registers each iteration is a form of logging. If
you need such a thing, inject it using a generic. In this case, I
just accepted a generic function type, but you could also create
your own trait if you wanted more granular places to log.
You should basically never use a &Vec<T>; use &[T] instead.
You should basically never use process:exit inside of any
function but main; use proper error handling like Option or
Result. I usually start with expect on these types which will
panic the program, then thread the errors through once the
general structure is in place.
Use slice::get instead of checking the length manually.
Creating a fetch_many method allows better naming,
flexibility, and a more efficient implementation.
Create more types! For example, create a RegisterFile and
Arguments to offload some of the decision making and
implementation details. These types provide naming (which is
self-documentation), extra seams for types and assertions, as well
as places to put "helper" methods (e.g. pack_bytes). They might
show places where you might be misusing sizes (e.g. indexing the
register file beyond 8-bit makes no sense).

use std::ops::{Index, IndexMut};
use std::cmp;
use std::error::Error;

enum Opcode {
    Exit = 0x00,
    Load = 0x01,
    Add  = 0x02,
}

impl Opcode {
    fn from_u8(byte: u8) -> Result<Opcode, Box<Error + Send + Sync>> {
        use Opcode::*;

        let opc = match byte {
            x if x == Exit as u8 => Exit,
            x if x == Load as u8 => Load,
            x if x == Add as u8 => Add,
            _ => return Err(From::from("invalid instruction code")),
        };

        Ok(opc)
    }
}

#[derive(Debug)]
struct RegisterFile {
    registers: [u64; 8],
}

// TODO: Consider catching and reporting access to registers outside
// of the register file. The indexing implementation panics the
// program.

impl Index<u8> for RegisterFile {
    type Output = u64;

    fn index(&self, idx: u8) -> &u64 {
        &self.registers[idx as usize]
    }
}

impl IndexMut<u8> for RegisterFile {
    fn index_mut(&mut self, idx: u8) -> &mut u64 {
        &mut self.registers[idx as usize]
    }
}

impl Default for RegisterFile {
    fn default() -> RegisterFile {
        RegisterFile { registers: Default::default() }
    }
}

#[derive(Copy, Clone)]
struct Program<'a> {
    code: &'a [u8],
    pos: usize
}

impl<'a> Program<'a> {
    pub fn new(code: &'a [u8]) -> Program<'a> {
        Program { code: code, pos: 0 }
    }

    fn fetch(&mut self) -> Result<u8, Box<Error + Send + Sync>> {
        let v = try!(self.code.get(self.pos).ok_or("No more code to read"));
        self.pos += 1;
        Ok(*v)
    }

    fn fetch_many(&mut self, n: usize) -> Result<Vec<u8>, Box<Error + Send + Sync>> {
        if self.code.len() < self.pos + n {
            return Err(From::from("No more code to read"));
        }
        let v = &self.code[self.pos..(self.pos + n)];
        self.pos += n;
        Ok(v.to_owned())
    }
}

struct Arguments {
    values: Vec<u8>,
}

impl Arguments {
    // TODO: Did I pick `high` and `low` right?
    pub fn high_u32_8(&self) -> u8 {
        self.values[3]
    }

    pub fn low_u32_8(&self) -> u8 {
        self.values[7]
    }

    pub fn u64(&self) -> u64 {
        Arguments::pack_bytes(&self.values)
    }

    fn pack_bytes(bytes: &[u8]) -> u64 {
        let len = cmp::min(bytes.len(), 8);
        bytes[..len].iter().fold(0, |acc, &byte| acc << 8 | (byte as u64))
    }
}

fn eval<F>(mut program: Program, mut registers: RegisterFile, each_iteration: F) -> Result<(), Box<Error + Send + Sync>>
    where F: Fn(&RegisterFile),
{
    loop {
        let operator = try!(program.fetch());
        let operator = try!(Opcode::from_u8(operator));
        let destination = try!(program.fetch());
        let arguments = try!(program.fetch_many(8));
        let arguments = Arguments { values: arguments };

        match operator {
            Opcode::Exit => return Ok(()),
            Opcode::Load => {
                registers[destination] = arguments.u64();
            }
            Opcode::Add => {
                registers[destination] = registers[arguments.high_u32_8()]
                    + registers[arguments.low_u32_8()];
            }
        }

        each_iteration(&registers);
    }
}

fn main() {
    let registers = Default::default();
    let code = vec![
        Opcode::Load as u8,
            0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x64,
        Opcode::Load as u8,
            0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xC8,
        Opcode::Add as u8,
            0x02, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01,
        Opcode::Exit as u8,
            0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
    ];
    let program = Program::new(&code);

    if let Err(e) = eval(program, registers, |registers| println!("{:?}", registers)) {
        println!("Evaluation failed: {}", e);
        // TODO: Could add process::exit with an error code here
    }
}


Check if all lotto numbers are covered in input Each ticket for the Pick-10 lotto has 10 unique numbers ranging from 1 to 99. Suppose you buy a lot of tickets and like to have them cover all numbers from 1 to 99. Write a program that reads the ticket numbers from a file and checks whether all numbers are covered. Assume the last number in the file is 0. Suppose the file contains the numbers:

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

Your program should display:

"The tickets cover all numbers"

Suppose the file contains the numbers

11 48 51 42 8 74 1 41 36 53
52 82 16 72 19 70 44 56 29 33
0

Your program should display:

  "The tickets do not cover all numbers"

    import java.util.Scanner;

    public class LottoCheck {
    public static void main(String[] args) {
        Scanner input = new Scanner(System.in);

        //Create an array of booleans
        boolean[] neededNumbers = new boolean[99];

        //Prompt input
        System.out.println("Enter your ticket numbers: ");
        int ticketNumber = input.nextInt();

        while (ticketNumber != 0) {
            for (int i = 1; i <= neededNumbers.length; i++) {
                neededNumbers[i] = true;
            }
            ticketNumber = input.nextInt();
        }

        for (int i = 0; i < neededNumbers.length; i++) {
            if (neededNumbers[i] == false) {
                System.out.println("You do not have all the numbers.");
                System.exit(0);
            }
        }
        System.out.println("You have all the numbers.");
    }
    }

First, this will not work with the input 99.  Arrays are 0-indexed, but when you create one, you need to specify the value you wish .length to have, which is 100.

This part is also erroneous:

while (ticketNumber != 0) {
    for (int i = 1; i <= neededNumbers.length; i++) {
        neededNumbers[i] = true;
    }
    ticketNumber = input.nextInt();
}

Here, you input the values until the input equals 0, but you loop from 1 through needeNumbers.length and set the values to true.  First off, this will crash when you run it because the value neededNumbers[neededNumbers.length] does not exist.  Additionally, your logic is wrong as you do not want all the values of neededNumbers to be set to  true, just the input ones:

while (ticketNumber != 0) {
    neededNumbers[ticketNumber] = true;
    ticketNumber = input.nextInt();
}

This loop is almost right, but you check the value an index 0, which you did not set to true because 0 was the value to exit the loop:

for (int i = 0; i < neededNumbers.length; i++) {
    if (neededNumbers[i] == false) {
        System.out.println("You do not have all the numbers.");
        System.exit(0);
    }
}

You can solve this problem in one of two ways - starting with i = 1, or changing your while loop to use neededNumbers[ticketNumber - 1] = true; and intializing it to new boolean[99], like you currently do.  This will have value 1 be stored in the 0 index of the array, so all values will be set to true once all the numbers in the range of 1 - 99 have been entered.


Processing an input image using probability I have the function below which is used to process an input image using probability predicted by a CNN network model.

def interp_map(prob, zoom, width, height):
    zoom_prob = np.zeros((height, width, prob.shape[2]), dtype=np.float32)
    for c in range(prob.shape[2]):
        for h in range(height):
            for w in range(width):
                r0 = h // zoom
                r1 = r0 + 1
                c0 = w // zoom
                c1 = c0 + 1
                rt = float(h) / zoom - r0
                ct = float(w) / zoom - c0
                v0 = rt * prob[r1, c0, c] + (1 - rt) * prob[r0, c0, c]
                v1 = rt * prob[r1, c1, c] + (1 - rt) * prob[r0, c1, c]
                zoom_prob[h, w, c] = (1 - ct) * v0 + ct * v1
    return zoom_prob

At the moment, the method takes around 26-30 secs execution time. I want to reduce it, if possible. The 3 for loops consumes a lot of time, however, not sure how can I reduce it.

Any suggestions on how to optimize it?

Attributes of the input parameters:

prob.shape
Out[3]: (66, 66, 13)
width
Out[4]: 480
height
Out[5]: 360

I'm not familiar with the algorithm, so I can't say whether there is a radically different approach which will improve performance. That said:

With such short variable names it's hard to know what things are. What is c0 actually? What does the "t" in rt and ct mean? Also, the structure and semantics of prob is completely opaque. I can guess that it's related to probability in some way, but why do you only care about the third index of its shape and what is prob[r0, c0, c]?
Try compressing the calculation into a single expression to avoid reassigning so many times. This should be optimised away, but see point 4.
Instead of converting the height and width indexes into floating point numbers, can you use an integer scaling factor to avoid floating point math?
This sort of performance problem really should be profiled, but without the full context and realistic images it would amount to nothing more than guesses.


Printing 80 (or more) times the same character I want to print a lot of the same characters and I don't want to use a for loop and I'm also looking for a way to do it as less lines as possible. Even better I would like to be able to store that string in a static variable to use again and again.

This is how I currently do it (ugly but at least no for loop):

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

What's a better way (aesthetically wise)?

String constants in Java are very efficient. In practice, your code is very high-performing. The issue here is not about how fast the code runs, because, essentially, it cannot be run any faster than this.

If you want to have more efficient code in terms of space, then the practical thing to do is to declare the value as a string constant, something like:

private static final String DASHES =
              "----------------------------------------"
            + "----------------------------------------"
            + "----------------------------------------";

And then, in your code, you just:

System.out.println(DASHES);

This will be just as fast, but the code is easier to read.

If the one place where you declare the dashes is still unsightly, then you can compute the value using the other mechanisms shown in other answers... e.g. :

public static final String DASHES = new String(new char[80]).replace("\0", "-");

but that is overkill, and unnecessary.

Edit:

In your comments you raised the issue of having multiple different-length values. I would recommend that you create a constant for the longest one (whether you use the String-constant or some other generator method), and then do a DASHES.substring(0, length) on that to get the shorter constants...

alternatively, I would consider a helper function as useful for this problem....

private static final String repeatChar(char c, int length) {
    char[] data = new char[length];
    Arrays.fill(data, c);
    return new String(data);
}

and then your constants can be initialized with:

private static final String DASH80 = repeatChar('-', 80);
private static final String SPACE40 = repeatChar(' ', 40);

etc.