Create a 2D array from SQL table I am a beginner to PHP programming.  I wanted to make a 2D array that would store the values of a table into the array. Below is the Frankenstein code I created (every time I wanted to achieve a task I researched a solution, so this is a bunch of solutions plus my own doing put together).

I would be really grateful if anyone could offer any improvements to my code. One thing I am slightly unsure of is the use of the unset function. I understand that it 'destroys' a variable to free up memory(?), but I am not entirely sure where/ if to use it.

<?php
$sql = "DESCRIBE $table"; // get a list of all fields and datatypes (ignore the latter)
$retval = mysqli_query($conn, $sql);
$iii = 0; // counter variable
while ($tableHead = mysqli_fetch_array($retval, MYSQLI_ASSOC))
{
    $colNames[$iii] = $tableHead["Field"]; // store each column name in array
    $iii++;
}
unset($iii);

$sql = "SELECT * FROM $table"; // get a list of all fields and datatypes (ignore the latter)
$retval = mysqli_query($conn, $sql);
$iii = 0; // counter variable
while ($row = mysqli_fetch_array($retval, MYSQLI_ASSOC))
{
    // create a 2D array, $iii for x and $jjj for y
    for ($jjj = 0; $jjj < $colCount; $jjj++)
    {
        $tableContent[$iii][$colNames[$jjj]] = $row[$colNames[$jjj]];
    }
    $iii++; // move onto next row
}
unset($iii, $jjj);
?>

EDIT: I have attached some pictures of what I have created. The quick report just reads the ID and returns any ID that is not found in the database or in the CSV. The full report is not yet developed but will check in more depth by checking each field one at a time and reporting the differences to the user.

The CSV looks like this:

1,a
2,b
3,c
4,d
5,e
6,f
7,g
8,h
9,i

The front page:

The result:

This comment lies.

$sql = "SELECT * FROM $table"; // get a list of all fields and datatypes (ignore the latter)

You copy pasted the whole line and then neglected to update/remove the comment.
It's traditional to use a simple i for loop counter variables. I see no reason to use a triple iii or jjj. However, in this case it makes much more sense to use more verbose (and hence, descriptive) names like $row and $col.


some style issues to consider:

1) it's conventional to use ALL_CAPS for constants such as 'supers' and w1, etc.

2) for long term maintenance, it's also a good idea to use longer more descriptive names - not only is this hard to parse for the uninitiated, it's also prone to hard-to-catch typos which will be hard to spot.  It's especially bad when you do multiple assignments on a single line.

3) supers could be a dictionary

4) you can simplify the range logic by excepting for incomplete groups:

<snip>
enter code here
group = group.replace("-", "q")
fnv, lnv = None
try:
   fnv = group[:5]
   lnv = group[6:9]
except IndexError:
   raise ValueError "group string %s is malformed"
fn_value = int(fnv, 36)
ln_value = max(int(lnv, 36), 1000) or 1000

5) maxNum is always the same, it should be a constant or a cached module-level field:

informative_name = sum(map(lambda x: x[1], supers))

at the top will execute at module load and wont need to be recalced for every call.


Should a piece of code only ever called once be a separate method? I'm curious when a piece of code should be its own method or just left alone. This came up when I was creating a "work item" from the thread pool. When passing the argument to the "WaitCallback" object, I would argue that if the code that will run will ONLY run when this thread is being run, then it's OK to wrap the code inside of an Action delegate. If other places need to run this same code, then I understand separating it into a new method.

So, for example, what's better practice?

This --

ThreadPool.QueueUserWorkItem(new WaitCallback((state) =>
{
    Contract.SaveContract(Customer);
     pushResultNotification = DoPushContact();
      MyDispatcher.BeginInvoke((Action)(() => actionNotification.CompleteAction()), null);
}));

Or this --

ThreadPool.QueueUserWorkItem(new WaitCallback(DoStuff));

private void DoStuff(object state)
{
    Contract.SaveContract(Customer);
    pushResultNotification = DoPushContactToGp();
    MyDispatcher.BeginInvoke((Action)(() => actionNotification.CompleteAction()), null);
}

I think the answer here is no, it shouldn't be a separate method, as long as the piece of code in question is short. That way, when you read the code, you immediately see what the code does, you don't have to go elsewhere for that. But if the piece of code was too long, it would make the whole method less readable, so in that case, I think you should put it into a separate method.

Few more notes:

You don't need the new WaitCallback() in either case, the compiler can create the delegate automatically.

Also, it might make sense to use Task instead of ThreadPool, assuming you're on .Net 4+.


Sum of differences of adjacent elements in an array For an array of N integers, at most K will be taken out (without changing the original order of elements). A function f calculates the sum of differences between adjacent elements (when thought about, for a given sequence of numbers, the result value of f will be lastNumber - firstNumber, because all the others in between will get canceled out). The job is to find out the maximum value that f can have of the newly constructed array when an arbitrary choice of m<=K numbers get taken out of the original array (resulting in a new array).

Example:
A = 1 7 2 5 3 8 2 3 6 5 5 , K = 4
Solution is 5. So, for some m (in range of 0 to K) some m numbers get taken out, giving a new array. Function f deals with that array.

Since for f, only the first and the last element matter, my angle was to cover the cases when these "bounds" shifted, for the given m, compare the results and take the greatest value. If I examined the code correctly, the time complexity is \$o(n^2)\$, but when I sent it over for evaluation, the response was "Time Limit Exceeded". Is there a better approach to this problem or did I miss something in my code (I made a few test examples and the outputs were correct, so it's a matter of algorithm, I believe)?

Here is the code:

#include <iostream>
#include <string>

using namespace std;

int main() {

    int N, K;
    int *arr;
    int max;

    cin >> N;

    arr = new int[N];

    if (N < 2 || N > 500000) {
        fprintf(stderr, "Los input\n");
        exit(EXIT_FAILURE);
    }

    cin >> K;

    if (K < 0 || K > N - 2) {
        fprintf(stderr, "Los input\n");
        exit(EXIT_FAILURE);
    }

    for (int i = 0; i < N; i++) {
        cin >> arr[i];
        if (arr[i] < -1000000 || arr[i] > 1000000) {
            fprintf(stderr, "Los input\n");
            exit(EXIT_FAILURE);
        }
    }

    max = arr[N-1] - arr[0];

    for (int i = 1; i <= K; i++) {

        for (int j = 0; j <= i; j++) {

            if (max < arr[j+N-1-i] - arr[j])
                max = arr[j + N - 1 - i] - arr[j];

            cout << (arr[j + N - 1 - i] - arr[j]) << endl;

        }

    }
    cout << max << endl;
    delete [] arr;

    return 0;

}

General implementation

using namespace std; is considered bad practice and should be avoided.
The C++ style std::cin and std::cout are used for input and output, but the C style fprintf is used for error reporting. Maybe choose one style for consistency (e.g. use std::cerr for errors)?
The whole array arr could as easily be replace by a std::vector<int>, letting the container take care of bookkeeping and memory cleanup.

Algorithm

Your algorithm uses (as you correctly stated) \$O(K^2)\$ time by calculating all possible combinations. This can be reduced to \$O(K)\$ with a bit of cleverness:

As far as I understand it, the problem basically boils down to: f = last - first, maximize f by removing up to m <= K total elements. This means to maximize f, one needs to find how many elements to remove from each "end" of the array, because as you correctly deduced, only those matter.

Now we can step from one end up to K elements in, and for each step calculate the change in f if we were to remove elements up to this point. If it's the best result so far, we note it for the current position, else we note the previous better result (after all, we always can take less elements away). We do this for both ends.

Then we can add the noted values for taking x elements from the front and K-x elements from the back together (so up to K elements total), and find the maximum for this value. This value is the highest increase possible for f.

Adding this value to the previously calculated original value of f gives us then the maximum value for f.

Step by step

Let's take your example: A = [1 7 2 5 3 8 2 3 6 5 5] , K = 4.

Let B = [? ? ? ? ?] be the map x elements removed from the front -> max increase of f so far.

If we were to remove m = 0 elements from the front, f wouldn't change. B = [0 ? ? ? ?].
If we were to remove m = 1 element from the front, f would change by 1 - 7 = -6. This is worse than removing 0 elements, so we store the result for that instead (after all, we can always remove less elements for a better result). B = [0 0 ? ? ?].
If we were to remove m = 2 elements from the front, f would change by 1 - 2 = -1. Since this is again worse than removing nothing (our best result so far), let's store that again, B = [0 0 0 ? ?].
Same results for m = 3 (1 - 5 = -4) and m = 4 (1 - 3 = -2) respectively (so final B = [0 0 0 0 0]).

Let's do this again with C = [? ? ? ? ?], but now we remove elements from the back.

If we were to remove m = 0 elements from the back, nothing changes. C = [0 ? ? ? ?].
If we were to remove m = 1 elements from the back, nothing would change (5 - 5 = 0). C = [0 0 ? ? ?].
If we were to remove m = 2 elements from the back, f would increase by 6 - 5 = 1. This is better than previous results, so we store that. C = [0 0 1 ? ?].
If we were to remove m = 3 elements from the back, f would change by 3 - 5 = -2. This is worse than our best result (1) so far, so let's store that instead. C = [0 0 1 1 ?].
If we were to remove m = 4 elements from the back, f would change by 2 - 5 = -3, so let's again store the best result so far. C = [0 0 1 1 1].

Now we can go over both B and C and look what the best results for removing up to  x elements from the front and K - x (so m <= x + (K - x) = K)  elements from the back.

x = 0: The best result we could get by removing up to 0 elements from the front and K elements from the back is B[0] + C[K] = 0 + 1 = 1.
x = 1: The best result we could get by removing up to 1 elements from the front and K-1 elements from the back is B[1] + C[K-1] = 0 + 1 = 1.
x = 2: B[2] + C[K-2] = 0 + 1 = 1.
x = 3: B[3] + C[K-3] = 0 + 0 = 0.
x = 4: B[4] + C[K-4] = 0 + 0 = 0.

So the best we could get is an increase of f by 1 (by removing 0 elements from the front and 2 elements from the back, though that wasn't asked).

Algorithm code

In code (using arrays for consistency with existing code):

/* other stuff, like reading arr, up to this statement */
max = arr[N-1] - arr[0];

auto frontRemovals = new int[K+1]; // B
auto backRemovals = new int[K+1]; // C
auto maxFront = = frontRemovals[0] = 0;
auto maxBack = baxkRemovals[0] = 0;

for(auto frontIndex = 1, backIndex = N-2; frontIndex <= K; ++frontIndex, --backIndex) {
    if(arr[0] - arr[frontIndex] > maxFront) {
        maxFront = arr[0] - arr[frontIndex];
    }
    frontRemovals[frontIndex] = maxFront;

    if(arr[backIndex] - arr[N-1] > maxBack) {
        maxBack = arr[backIndex] - arr[N-1];
    }
    backRemovals[frontIndex] = maxBack;
}

auto maxDifference = 0;
for(auto x = 0; x <= K; ++x) {
    if(frontRemovals[x] + backRemovals[K - x] > maxDifference) {
        maxDifference = frontRemovals[x] + backRemovals[K - x];
    }
}

max += maxDifference;
/* printing result, freeing array, etc. */

  Note: With a bit more cleverness, this operation can be done in one pass. However, this exercise is up to the reader ;)


Using common code inside a Higher Order Component I have a common List in mind which looks something like this:

it has one remove button

one input in which user can enter the name of element and on clicking insert the element will be added in the list.

For this i have added HOC like this:

function ListHOC(Component, data, listName, placeholder) {
  return class extends React.Component {
    constructor() {
      super();
      this.state = { data, element: "" };
    }

    add = item => {
      const { data } = this.state;
      data.push(item);
      this.setState({ data });
    };

    remove = keyToRemove => {
      const { data } = this.state;
      const newData = data.filter(({ key }) => keyToRemove !== key);
      this.setState({ data: newData });
    };

    render() {
      const { data, element } = this.state;
      const updatedList = data.map(({ name, key }) => (

{name}

          <button onClick={() => this.remove(key)}>remove</button>

      ));
      return (
        <>

{listName}:

          <Component data={updatedList} {...this.props} />
          <input
            placeholder={placeholder}
            onChange={e => this.setState({ element: e.target.value })}
          />
          <button
            onClick={() => this.add({ name: element, key: data.length + 1 })}
          >
            insert
          </button>
        </>
      );
    }
  };
}

one thing i am not sure about is weather to use the input and button and listname inside HOC or not

link to codepen: https://codepen.io/saxenanihal95/pen/NWKVJOx?editors=1010

There's no reason to use a HOC for this, it can be done more simply and clearly with a component:

class List extends React.Component {
  state = { data: this.props.initialData, element: "" };

  add = item => {
    this.setState(prev => ({ data: prev.data.concat(item) }));
  };

  remove = keyToRemove => {
    this.setState(prev => ({
      data: prev.data.filter(({ key }) => keyToRemove !== key)
    }));
  };

  render() {
    const { data, element } = this.state;
    const { placeholder, listName } = this.props;

    return (
      <>

{listName}:

        {data.map(({ name, key }) => (

{name}

            <button onClick={() => this.remove(key)}>remove</button>

        ))}
        <input
          placeholder={placeholder}
          onChange={e => this.setState({ element: e.target.value })}
        />
        <button
          onClick={() => this.add({ name: element, key: data.length + 1 })}
        >
          insert
        </button>
      </>
    );
  }
}

const Users = () => (
  <List
    initialData={[
      { name: "a", key: 1 },
      { name: "b", key: 2 }
    ]}
    listName="Users"
    placeholder="insert user"
  />
);
const Comments = () => (
  <List initialData={[]} listName="Comments" placeholder="insert comment" />
);
const AnotherList = () => <Users />;

function App() {
  return (

      <Users />
      <Comments />
      <AnotherList />

  );
}

ReactDOM.render(<App />, document.getElementById("app"));

HOCs are generally better for cross-cutting concerns, or behavior (not presentation) which you want to add to any component. for example logging:

const withLogging = Component => props => {
  console.log('Props:', props);
  return <Component {...props} />;
}

const List = ({ name, data }) => ...
const ListWithLogging = withLogging(List); // <-- This will log all props


Calculating sum after ranged-based subtractions The puzzle consists from something like this:

You will receive an array of integer with values, then you will receive X numbers. Each number (call it x) represents an index of a 1-indexed array. Look up the value in the array (call it y), then subtract 1 from all the values in the array in the range [x-y, x+y]. The minimum value is 0.

After the given number of operations, you must return the sum of all values in the array.

My code looks like this.

#include <cstdio>
int array[111111];

int main(){
  int m,n;
  unsigned long long int sum = 0;

  //here n is the size of the array, and m is the number of operations
  scanf("%d %d", &n, &m);

  for (int i = 0; i < n; i++){
    scanf("%d", &array[i]);
    sum+= array[i];
  }

  for (int i = 0; i < m; i++) {
    int idx;
    scanf("%d", &idx);
    int count= array[idx - 1];

    for (int j = (idx -1 -count >=0 ? idx -1 -count : 0); j < (idx + count <=n ? idx + count : n); j++)
      if (array[j]){
        sum--;
        array[j]--;
      }
  }

  printf("%d\n", sum);
}

It solves the problem, but does not pass in the worst case, that would be a ton of cases decreasing from the entire array, basically an O(n*n) (I think).

The value restrictions are:

1 ≤ N,M ≤ 100000;
Value inside array index between 0 and 50000.

Test case:

Input:

10 4
1 2 0 8 4 2 9 8 1 3
9 5 10 6

Output:

23

Visualization of the test case:

For example, the first number that is read is 9. If we treat the number sequence as a 1-indexed array, we read value 1. This means that we need to subtract 1 from every value in the range [9-1, 9+1], which corresponds to the last three elements of the array.

Performance
From what I understand, your solution is as good as it would get. The "n^2" (it's more like O(n*m)) performance seems inevitable. It also seems that an array is the optimal data structure since you must index into it.
If you really want to remove the worst case (decreasing from the entire array), you could keep a count of the number of times you have subtracted from the whole array and let the array value be array[idx] - theCountIMentioned and the final sum be sum - n * theCountIMentioned. Despite this, the performance would still be nearly n^2, especially since decreasing all but one of the values would be an O(n) operation, so the worst case would be O(n * m).
Readability
On another note: to make the inner loop more readable consider the following two functions:
inline int min(int a, int b){
    return (a < b)? a : b;
}

inline int max(int a, int b){
    return (a > b)? a : b;
}

This would make the inner loop's header much more readable. (It would be for(int j = max(0, idx - 1 - count); j < min(idx + count, n); j++)).
Also, you should make the variable names more readable.

n = arrayLength or even arrLen.
m = numberOfOps or even opCt.


Structuring objects for proper MVVM in Knockout I'm working with knockout and am trying to stay true the MVVM structure and trying to make the objects have that dependency on each other.

Model, ViewModel, Service definitions:

var App = window.App || {};

(function(ns, $, ko) {
    ns.Models = {};
    ns.ViewModels = {};
    ns.Services = ns.Services || {};

    //Service def
    ns.Services.SearchService = function() {
        this.SearchByName = function(name, callback) {
            $.get("/api/SearchByName/" + name, function(d){
                callback(d);
            });
        };
    };

    //Model Def
    ns.Models.SearchResultModel = function(json) {
        var self = this;

        ko.mapping.fromJS(json, {}, self);
    };

    //ViewModel def
    ns.ViewModels.SearchResultsViewModel = function() {
        var self = this;

        self.dataService = new ns.Services.SearchService();
        self.SearchResults = ko.observableArray();

        self.GetSearchResultsByName = function(name){
            self.dataService.SearchByName(name, function(d) {
                $.each(d, function(i, e) { self.SearchResults.push(new ns.Models.SearchResultModel(e)); });
            });
        };
    };
}(App, jQuery, ko));

And I can currently use it like so:

var vm = new App.ViewModels.SearchResultsViewModel();

vm.GetSearchResultsByName("Doe");

ko.applyBindings(vm, document.getElementById("search-results-form"));

This is just my starting point and it seems like the ko.applyBindings(...) should be in the ViewModel somewhere.

With all that, am I going the right direction for this or am I completely off with it?

This looks good me, just as it looked good for the SO reviewers.

Some thoughts;

Consider 'use strict';
Consider naming your anonymous functions
It is not clear how SearchResultModel deals with failed calls
var self = this; is only needed for closures, you don't need it in ns.Models.SearchResultModel

All in all, pretty solid.


splitting and merging Ranges array in javascript I have an array of ranges as you can see in the snippet. when user select certain number from dropdown I want to remove that number from the range and return new ranges,

Everything is working as per my expectation but I think, I have over coded and used too many if else statements in my code. I was wondering if there is any better way to do it?

If yes please let me know, if Javascript has any methods to make this simple and reduce the amount of code.

var ranges = [
  {start: 1, end: 100},
  {start: 150, end: 300},
  {start: 400, end: 500},
  {start: 550, end: 650},
];

function onSelect(event) {
  let newRange = [];
  let HMID = parseInt(document.getElementById("mySelect").value);

  for (let range of ranges) {

      if ((HMID > range.start) && (HMID < range.end)) {

          newRange.push({start: range.start, end: (HMID - 1)});
          newRange.push({start: (HMID + 1), end: range.end});
      } else if (HMID === range.start) {

          newRange.push({start: (range.start + 1), end: range.end});
      } else if (HMID === range.end) {

          newRange.push({start: range.start, end: (range.end - 1)});
      } else {

          newRange.push(range);
      }
  }
  console.log(JSON.stringify(newRange));

}

function onSelectAdd() {

  let newRange = [];
  let HMID = parseInt(document.getElementById("mySelectAdd").value);

  for (let i = 0; i < (ranges.length); i++) {

  let start, end;

  if (HMID == (ranges[i].start -1)) {
      start = HMID;
      end = ranges[i].end;

      newRange.push({start: start, end: end});
  } else if (HMID == (ranges[i].end + 1)) {
      start = ranges[i].start;

      if (ranges.length === (i + 1)) {
          end = HMID;
      } else {

          if (HMID == (ranges[i+1].start - 1)) {
              end = ranges[i+1].end;
              i++;

          } else {
              end = ranges[i].end;
          }
      }

      newRange.push({start: start, end: end});

  } else {
      newRange.push({start: ranges[i].start, end: ranges[i].end});
  }

  }

  console.log(JSON.stringify(newRange));
}

Remove item

<select id="mySelect" onchange="onSelect()">
  <option value="1">1</option>
  <option value="150">150</option>
  <option value="450">450</option>
  <option value="650">650</option>
</select>

Add removed item

<select id="mySelectAdd" onchange="onSelectAdd()">
  <option value="1">1</option>
  <option value="150">150</option>
  <option value="450">450</option>
  <option value="650">650</option>
</select>

EDIT:
I am looking only for logical and functional code review. Please don't comment on syntax and variable names. Thanks.

I'll go over some of the basics

let HMID = parseInt(document.getElementById("mySelect").value);

what should HMID be? Aside from being a number, I don't know what it is. And the only reason I know it's a number is because it's the value of parseInt. Looking at how it's used doesn't shed any light. You should name the variable something more descriptive.

Speaking of naming things more descriprive - mySelect is a really bad name, too. Part of why I don't know what HMID is, is because it's the value of something with the non-descriptive ID of mySelect. The myX naming convention is fine when prototyping but you really need to go with descriptive names upon finishing your code.

Echoing the same "descriptive names needed" sentiment for onSelect and onSelectAdd. What do they do? One is executed on select which...doesn't tell me much. The other...adds stuff on select? It's not clear. Somewhat better names would be

onSelect -> removeItemFromRange
onSelectAdd -> addRemovedToRange

I'm not great at naming things but I'd err on the side of verbose but descriptive rather than short but unclear. If you can have something shorter, feel free to use that. As it stands, the names aren't good.

You are better off adding the event listeners via element.addEventListener rather than in the HTML. That way, you keep the HTML clear of logic - it's all contained in your JavaScript. As an added benefit, you can attach more event listeners that do other stuff without changing these functions. Here is an example

let button = document.getElementById("buttonWithMultipleClickListenersExample"); //told you I'm not good at naming things...

button.addEventListener("click", sayFoo);
button.addEventListener("click", sayBar);

function sayFoo() {
  console.log("foo");
}

function sayBar() {
  console.log("bar");
}
<button id="buttonWithMultipleClickListenersExample">Click Me</button>

Two different event listeners are attached for the click event and they can do two completely separate things. If sayBar() is altered or even a sayBaz() is added, we don't need to change anything else, whereas if we define the click handler in the HTML, we have to change that function with every alteration needed.


Python server that only returns error 418 For this project, I'm using a small piece of Python code to return an error 418 to any request.  I'm interested in knowing if there is a much smaller piece of code that does the same thing.

#Modfied from code provided by Copyright Jon Berg , turtlemeat.com import string,cgi,time from os import curdir, sep from BaseHTTPServer import BaseHTTPRequestHandler, HTTPServer
#import pri

class MyHandler(BaseHTTPRequestHandler):

    def do_GET(self):
            self.send_error(418,'I am a Teapot')

def main():
    try:
        server = HTTPServer(('', 80), MyHandler)
        print 'started httpserver...'
        server.serve_forever()
    except KeyboardInterrupt:
        print '^C received, shutting down server'
        server.socket.close()

if __name__ == '__main__':
    main()

PS - I considered putting this on Code Golf SE, but I wasn't sure if single-language-only requests are on-topic.

Not really smaller but it will respond to any request, not just GET:

class MyHandler(BaseHTTPRequestHandler):
    def handle_one_request(self):
        self.send_error(418, 'I am a Teapot')

Usually, handle_one_request delegates to the appropriate do_METHOD method, but since we want to treat all of them equal, we may as well not let that delegation take place.

In the interest of making things shorter, you generally don't need the server.socket.close(), unless you plan on restarting a server on that same port right after shutting down this server.


Crawl multiple pages at once This an update to my last question.

I want to process multiple pages at once pulling URLs from tier_list in the crawl_web method. The problem I am having is crawl_web already has two threads to crawl and index simultaneously, and the .join() seems to prevent opening more than one page.

import urllib2, collections, threading, robotparser, urlparse, Queue
from bs4 import BeautifulSoup
q = Queue.Queue()

def get_page_links(page, crawled, to_crawl, next_tier, robot_set):
    #respect the robots.txt and returns all urls from page
    rp = robotparser.RobotFileParser()
    links = []
    uparse = urlparse.urlparse
    for link in page[1].find_all('a'):
        url = link.get('href')
        base_url = (uparse(page[0]).scheme + '://' + uparse(page[0]).netloc)
        if base_url not in robot_set:
            robot_set.add(base_url)
            rp.set_url(urlparse.urljoin(base_url,'robots.txt'))
            rp.read()
        abs_url = urlparse.urljoin(base_url, url)
        if rp.can_fetch('*', abs_url):
            links.append(abs_url)
    links = set(links)
    next_tier.extend(links)
    to_crawl.update(links)
    to_crawl.remove(page[0])
    crawled.add(page[0])

def get_page(url):
    #return page html from url
    try:
        return [url,BeautifulSoup(urllib2.urlopen(url).read())]
    except:
        return [url,BeautifulSoup("")]

def get_page_words(page, word_index):
    #pulls words off page to be indexed
    url = page[0]
    words = page[1].get_text()
    to_remove = '{}[]«#$%^&*._,1234567890+=<>/\()":;!?'
    bad_words = set(['http','www','com','https','gov','org','edu','imgs'])
    for i in to_remove:
        words = words.replace(i, ' ').lower()
    words = words.split()
    for word in words:
        if word in word_index:
            word_index[word].add(url)
        elif len(word) < 15 and word not in bad_words:
            word_index[word] = set([url])

def crawl_web(url, maxdepth):
    #controls crawl depth, updates url lists and word_index
    depth = 0
    robot_set = set()
    crawled = set()
    to_crawl = set([url])
    word_index = {}
    tier_list = [[url]]
    while depth < maxdepth:
        next_tier = []
        for url in tier_list[depth]:
            page = get_page(url)
            t1 = threading.Thread(target = get_page_links, args = (page,
                                      crawled, to_crawl, next_tier, robot_set))
            t2 = threading.Thread(target = get_page_words, args = (page,
                                                                   word_index))
            t1.start()
            t2.start()
            t1.join()
            t2.join()
        tier_list.append(next_tier)
        depth += 1
    return crawled, to_crawl, word_index, tier_list

def search_engine(target_string, word_index):
    #searches word_bank for words in string, returns urls words are found at
    targets = list(set(target_string.split()))
    result =[]
    for word in targets:
        if word in word_index:
            result += word_index[word]
    ans = collections.Counter(result).most_common(3)
    return ans

result = crawl_web('http://xkcd.com',2)
word_index = result[2]
print result[0]
print search_engine('starting blogs about', word_index)

While I can't answer to your main question about crawling multiple pages at once,
I have a couple of comments about this code.

get_page_links

In this code, it seems that base_url and the if that follows should be outside of the for loop:

links = []
for link in page[1].find_all('a'):
    url = link.get('href')
    base_url = (uparse(page[0]).scheme + '://' + uparse(page[0]).netloc)
    if base_url not in robot_set:
        robot_set.add(base_url)
        rp.set_url(urlparse.urljoin(base_url,'robots.txt'))
        rp.read()
    abs_url = urlparse.urljoin(base_url, url)
    if rp.can_fetch('*', abs_url):
        links.append(abs_url)
links = set(links)

Also, instead of links = set(links),
it would be better to make links a set in the first place.

Rewriting the above code like this would be more efficient:

links = set()
base_url = uparse(page[0]).scheme + '://' + uparse(page[0]).netloc
if base_url not in robot_set:
    robot_set.add(base_url)
    rp.set_url(urlparse.urljoin(base_url, 'robots.txt'))
    rp.read()
for link in page[1].find_all('a'):
    url = link.get('href')
    abs_url = urlparse.urljoin(base_url, url)
    if rp.can_fetch('*', abs_url):
        links.add(abs_url)

get_page

It would be better to specify in the except clause the type of error that can be raised.

get_page_words

This expression contains the non-ascii character «,
and the posted code doesn't contain an encoding declaration:

to_remove = '{}[]«#$%^&*._,1234567890+=<>/\()":;!?'

You can declare utf-8 encoding by adding this line at the top of the file:

# coding=utf-8

to_remove = '{}[]«#$%^&*._,1234567890+=<>/\()":;!?'
for i in to_remove:
    words = words.replace(i, ' ').lower()

One problem here is that words is lowercased in every iteration,
when it would be enough to lowercase just once, before starting the loop.

But the bigger problem is the loop itself:
it would be much more efficient to use a regular expression to get rid of all the unwanted characters, for example like this:

import re
re_nonword = re.compile(r'[{}[\]«#$%^&*._,1234567890+=<>/\()":;!?]')
words = re_nonword.sub(' ', words)

set literals

Instead of this:

bad_words = set(['http','www','com','https','gov','org','edu','imgs'])

As of Python 2.7, you can write using set literals, like this:

bad_words = {'http', 'www', 'com', 'https', 'gov', 'org', 'edu', 'imgs'}

I also put spaces after the commas, to follow PEP8.


Looping text with directional control The code is an implementation of looping text (similar to a circular buffer - wraps around when it reaches the edge of the defined bounds) with directional control.

The code is functional and works as intended, but I'm curious if there is anyway to improve the code even if it is only a small improvement (e.g. less garbage produced per loop cycle as a micro-optimisation, etc).

using System;
using System.Text;
using System.Threading;

namespace LoopingTextScroll
{
    internal class Program
    {
        private enum Direction
        {
            LeftToRight = -1,
            RightToLeft = 1
        }

        private static StringBuilder PadText(string text, int padding = 5)
        {
            var stringBuilder = new StringBuilder(text);
            stringBuilder.Append(new string(' ', padding));
            return stringBuilder;
        }

        private static void ShiftText(StringBuilder textBuilder, Direction direction)
        {
            var directionOffset = (int)direction;
            var textLength = textBuilder.Length;

            switch (direction)
            {
                case Direction.LeftToRight:
                {
                    for(var i = textLength - 1; i >= 0; i--) textBuilder[i] = textBuilder[Modulo(i + directionOffset, textLength)];
                    break;
                }

                case Direction.RightToLeft:
                {
                    var firstElement = textBuilder[0];
                    for(var i = 0; i < textLength; i++) textBuilder[i] = textBuilder[Modulo(i + directionOffset, textLength)];
                    textBuilder[textLength - 1] = firstElement;
                    break;
                }
            }
        }

        private static void UpdateLoop(string text, Direction direction = Direction.LeftToRight, int updateRate = 100)
        {
            var textBuilder = PadText(text);

            while (true)
            {
                ShiftText(textBuilder, direction);
                Console.Write($"\r{textBuilder}");
                Thread.Sleep(updateRate);
            }
        }

        private static int Modulo(int x, int m)
        {
            var remainder = x % m;
            return remainder < 0 ? remainder + m : remainder;
        }

        public static void Main(string[] args)
        {
            UpdateLoop("Beautiful Lie", Direction.RightToLeft);
        }
    }
}

Your implementation can be greatly improved by using the System.Memory package. This will give you the new Memory<char> type (and if you're using .net-core it's already part of the SDK). With that you can create views of the string without creting any garbage.

At the same time you can make it more LINQ friendly. For this you can create the Slide extension that will return the slices of each part of the string and that accepts a Func for generating the offset for slicing. This one runs infinitely.

public static IEnumerable<(Memory<char> X, Memory<char> Y)> Slide(this string text, Func<int, int> calcOffset)
{
    var span = new Memory<char>(text.ToArray());
    while (true)
    {
        var offset = calcOffset(text.Length);
        var left = span.Slice(0, offset);
        var right = span.Slice(offset, text.Length - offset);
        yield return (right, left);
    }
}

You should also separate the logic for sliding and encapsulate each part in its own method. So you now have two of them.

public static Func<int, int> LeftToRight()
{
    var i = -1;
    return length =>
    {
        i++;
        if (i > length)
        {
            i = 0;
        }
        return i;
    };
}

public static Func<int, int> RightToLeft()
{
    var i = -1;
    return length =>
    {
        i--;
        if (i < 0)
        {
            i = length;
        }
        return i;
    };
}

To print it you can create another helper that encapsulates that part. Here you should replace the Thread.Sleep with Task.Delay which doesn't block/freeze the thread. With TimeSpan you can set the delay to whatever you want.

public static async Task PrintAsync(this IEnumerable<(Memory<char> X, Memory<char> Y)> source, TimeSpan delay)
{
    foreach (var element in source)
    {
        await Task.Delay(delay);
        Console.Clear();
        Console.Write(element.X);
        Console.WriteLine(element.Y);
    }
}

When you put everything together it'll look like that:

using  static Slider;

class Program
{
    static async Task Main(string[] args)
    {
        await
            "Hallo World"
                //.Slide(LeftToRight())
                .Slide(RightToLeft())
                .Take(20) // remove to run infinitely
                .PrintAsync(TimeSpan.FromMilliseconds(100));
    }
}

This is now garbage-free and does not create any additional sub-strings.


Coq proof that type isomorphism relationship is transitive The following code is in Coq, a proof assistant by INRIA implemented in OCaml.

I'm defining two types as being isomorphic when there exists two functions f : s -> t and g : t -> s such that, for all x, f (g x) = x. The function compose is just a helper function to make it easier to write terms in my typeclass.

I'm trying to prove that the isomorphism relation that I'm defining is transitive.

My proof is kind of ugly, I'm using inversion twice to get a whole bunch of terms "in scope".

Here's all the stuff that I introduce:

1 subgoal
A : Type
B : Type
C : Type
X : Isomorphism A B
X' : Isomorphism B C
from0 : A -> B
to0 : B -> A
from_to0 : forall x : B, from0 (to0 x) = x
to_from0 : forall x : A, to0 (from0 x) = x
from1 : B -> C
to1 : C -> B
from_to1 : forall x : C, from1 (to1 x) = x
to_from1 : forall x : B, to1 (from1 x) = x
______________________________________(1/1)
Isomorphism A C

Then I'm using refine with two holes _ in order to introduce two further goals:

refine({|
  from := compose from1 from0;
  to := compose to0 to1;
  from_to := _;
  to_from := _
|}).

I'm constructing from and to explicitly, and allowing from_to and to_from to become goals, like this:

______________________________________(1/2)
forall x : C, compose from1 from0 (compose to0 to1 x) = x
______________________________________(2/2)
forall x : A, compose to0 to1 (compose from1 from0 x) = x

And then for each of those cases I unfold to get rid of compose, and then get rid of adjacent froms and tos by rewriting.

This whole thing seems more complicated than it needs to be. Is there a more direct way to show that Isomorphism A B -> Isomorphism B C -> Isomorphism A C is inhabited?

Here is the complete source code for reference:

Class Isomorphism A B :=
  {
    from: A -> B;
    to: B -> A;
    from_to x: from (to x) = x;
    to_from x: to (from x) = x
  }.

Definition compose {T T' T''} (f : T' -> T'') (g : T -> T') (x : T) : T'' :=
  f (g x).

Definition trans { A B C }: Isomorphism A B -> Isomorphism B C -> Isomorphism A C.
intros X X'.
inversion X.
inversion X'.
refine({|
  from := compose from1 from0;
  to := compose to0 to1;
  from_to := _;
  to_from := _
|}).
{
  intros.
  unfold compose.
  rewrite -> from_to0.
  rewrite -> from_to1.
  reflexivity.
}
{
  intros.
  unfold compose.
  rewrite -> to_from1.
  rewrite -> to_from0.
  reflexivity.
}
Qed.

You can make the proof a lot shorter by giving a constructor for the class Isomorphism and by destructing your arguments instead of inverting them:

Class Isomorphism A B :=
  MkIsomorphism {
    from: A -> B;
    to: B -> A;
    from_to x: from (to x) = x;
    to_from x: to (from x) = x
  }.

Definition compose {A B C} (f : B -> C) (g : A -> B) (x : A) : C
  := f (g x).

Definition trans {A B C} :
  Isomorphism A B -> Isomorphism B C -> Isomorphism A C.
Proof.
intros [AtoB BtoA AtoBtoA BtoAtoB] [BtoC CtoB BtoCtoB CtoBtoC].
apply MkIsomorphism
  with (from := compose BtoC AtoB)
       (to   := compose BtoA CtoB);
intro x; unfold compose.
 - rewrite AtoBtoA, BtoCtoB; reflexivity.
 - rewrite CtoBtoC, BtoAtoB; reflexivity.
Defined.


Changing a user's password I wrote this class for the project I'm currently working on. It's basically used to change a user's password. I'd like to know your thoughts on the basic structure/design of this class and how it can be improved, or if you'd have written it differently.

class Recover_password{
    //class by sami
    private $conn;
    private $password_one;
    private $password_two;

    function initialize_connection($conn){
        if($conn instanceof PDO){
            $this->conn = $conn;
        }else{
            echo "Connection problem accured";
        }
    }

    function is_posted(){
        if(isset($_POST["change_password"])){
            $this->is_empty();
        }else{
            echo "nothing's being posted!";
        }
    }

    private function is_empty(){
        foreach($_POST as $post_value){
            if(!empty($post_value)){
                $is_empty = false;
            }else{
                echo "Field/s are empty";
                break;
            }
        }

        if(isset($is_empty)){
            $this->set_values();
        }
    }

    private function set_values(){
        $this->password_one = strip_tags(stripslashes(trim($_POST["password_two"], '"')));
        $this->password_two = strip_tags(stripslashes(trim($_POST["password_one"], '"')));

        if($this->password_one == $this->password_two){
            if( (strlen($this->password_one) > 10) or (strlen($this->password_two) > 10) ){
                $this->password_one = md5($this->password_one);
                $this->change_password_to_new();
            }else{
                $this->show_message("The password you provided is too weak");
            }
        }else{
            $this->show_message("Please make sure the passwords match");
        }
    }

    private function change_password_to_new(){
        $insert_pass_query = $this->conn->prepare("update registered_users set user_password = :user_password where username = :username");
        if( $insert_pass_query->execute(array('user_password'=>$this->password_one, 'username'=>$_SESSION["logged_in"])) ){
            $this->show_message("Password has been sucessfully changed!");
        }else{
            $this->show_message("Unknown error occured, please contact admin.");
        }
    }

    private function show_message($message){
        $_SESSION["message"] = $message;
        header("location: show_message.php");
    }
}

basic structure/design

I would not echo in a class, but throw an exception instead. That way, the calling class can decide how to handle these problems, and your class will thus be more flexible.

Your class also does two things: It changes a password, and it redirects. I would leave the redirection to the calling class as well (but this is probably a matter of taste).

is_empty

The is_empty function doesn't seem to be working correctly.

If only one of the two fields is filled in by the user, Field/s are empty is echoed, but set_values is called anyways.

In this case it doesn't really matter, because it will not do anything. But then your function could be simplified to:

private function is_empty(){
    foreach($_POST as $post_value){
        if(empty($post_value)){
            echo "Field/s are empty";
            break;
        }
    }

    $this->set_values();
}

An alternative way would be to move that check into set_values, where all the other checks also take place:

    if ($this->password_one == "" || $this->password_two == "") {
        $this->show_message("Field/s are empty");
    } else if($this->password_one != $this->password_two){
        $this->show_message("Please make sure the passwords match");
    }else if (strlen($this->password_one) <= 10) {
        $this->show_message("The password you provided is too weak");
    } else {
        $this->password_one = md5($this->password_one);
        $this->change_password_to_new();
    }

Note that I rearranged the if statements for more readability, and used the shoe_message method for all messages, instead of using echo.

Naming

is_posted doesn't seem like an optimal name, although I have problems coming up with a better name.
PHP class names should be in CamelCase

Misc

I would not change a user password. If a user wants their password to be "    <p>  \'  <!-- super good password --> why not let them? In addition to annoying users, this could also lead to real problems, because you have to use those exact filter functions everywhere where the user supplies a password, as otherwise some users will not be able to login.
in set_values, the or (strlen($this->password_two) > 10 check is unnecessary, as password_one and password_two are equal and password_one is already checked.
security: md5 isn't really considered good enough anymore. If possible, you should use bcrypt (password_hash in PHP) or pbkdf2 instead.


Building a Tree from Breadcrumbs I need to transform

List("home", "mens", "clothing", "jackets")

into

List(
  "/home",
  "/home/mens",
  "/home/mens/clothing",
  "/home/mens/clothing/jackets"
)

This is what I have so far:

val nodes = Array("home", "mens", "clothing", "jackets")
val (rootNode, otherNodes) = nodes.splitAt(1)
val tree = otherNodes.foldLeft(rootNode) { (list, token) =>
    list :+ (list.last + "/" + token)
}

What do you think? any obvious problems?

It looks like you need a variation of the prefix sum algorithm applied to your list. In Scala this can be accomplished with scan:

val xs = List("home", "mens", "clothing", "jackets")
xs.scan("")(_ + "/" + _).tail


Find all consecutive sequences in an ordered set I have the following set. a = [1,2,3,4,5] and i want to find all of the following sequences of consecutive elements:

1,
1,2
1,2,3
1,2,3,4
1,2,3,4,5
2,
2,3,
2,3,4,
2,3,4,5
3,
3,4
3,4,5
4
4,5
5

This is the following code that I use and works correctly. My only problem is with the complexity of the algorithm O(n2). Is there a better approach that I can use to reduce the complexity?

    int main()
    {
        int a[5] = {1,2,3,4,5};
           vector<vector<int>> all;
            for(int i = 0; i < 5; i ++)
            {
                vector<int> g;
                g.push_back(a[i]);
                all.push_back(g);
               for(int j = i+1; j < 5; j++)
               {
                    g.push_back(a[j]);
                    all.push_back(g);
               }
               g.clear();
            }

            for(int i = 0; i < all.size();i++)
            {
               for(int j = 0; j < all[i].size();j++)
               {
                    cout << all[i][j] << "  ";
               }
               cout << endl;
            }

        return 0;
    }

You cannot get better than O(n²) because the result has the size of O(n²)

The Output has (n+1)*n/2 elements therefore you will always need O(n²) time to do the output.

You can only try to reduce the constant factor.


Custom stream-like classes Consider the following code:

#include <iostream>
#include <vector>
using namespace std;

struct test {
    int j;
    test& operator>>(int & i) { i = ++j%26; return *this; }
    operator bool() { return j%26; }
};

int main () {
    test t;
    int i;
    while (t >> i) {
        cout << i << endl;
    }
}

I have implemented a simple class that can be used in a similar fashion as for example std::cin. I implemented it this way, because I wanted to have an object that hides some IO-operations from me and provide a nice interface to yield the data items each one-at-a-time.

Although it does want I want, I know there is at least one issue with the implementation:

ignorance w.r.t. the safe bool idiom

Are there probably any more?

For simple 10 lines 10 programs you can get away with this:

using namespace std;

Anything bigger and it starts to be a pest more than a help. Thus it best not to get into the habit of using it. Anyway it is simpler even in this case just to prefex cout with std:: (that is why standard is named std to make it short and easy to use).

You don't initialize the value of j in your class.

int j;

Thus any usage is undefined behavior.

You have two options:

Add a constructor that initaizes j
Always force zero-initialization of your class.

In anything but this trivial program I would go with (1) and add a constructor. But just to show zero initialization the alternative is:

 Test  t = Test();  //  The () brackets at the end of Test() force zero initialization;

Note: The equivalent for new is

 Test* t = new Test();  // force zero-initialization (when no constructor)
 Test* t = new Test;    // Use default-initialization (un-initialized)

Note 2: Normally you would think you could do this:

 Test  t(); // Unfortunately this is a forward declaration of a function.

Note 3: But you can get around the problems of (2) by using extra braces.
        As pointed out last time I tried to answer a question like this.

 Test  t(()); // Unfortunately this confuses people and maintainers have been
              // known to try and take out the extra braces. With mixed results.
              // This is why I still prefer method 1

 Test t = Test(); // This may look like there is an extra copy.
                  // But every compiler implements the optimization to do construction
                  // in place.
              // Thus I prefer method 1 (ie this method)

Described here: https://stackoverflow.com/questions/5914422/proper-way-to-initialize-c-structs/5914697#5914697

For such a simple class I don't think you need to worry about the safe bool idium. This just protects your class from being used in places where arithmatic would be done:

 Test t;
 int x = 5 + t;  // Here we get conversion to bool
                 // which is then converted to int (0 or 1)
                 // The safe bool idium would protect you from this:

To use it do this:

 operator bool() { return j%26; }

 // Remove the above and replace with

 operator void*() {return j%26 ? &j /* or this the value is not important as long as it is not NULL */ : NULL; }


Remove repeated words from a 2D character array This code removes repeated words from a word array (2D char array).  I want to optimize this as much possible for speed.

#include "stdio.h"
#include "string.h"

main()
{
    char words[2000][20];

   /*This array is populated by some other module. possible example of this array is given below, but in reality it would far more elements.
        char words[6][20] =
    {
        {"barrack"},
        {"david"},
        {"John"},
        {"david"},
        {"benjamin"},
        {"barrack"}

    };*/

    int names_found = 10;
    char out_words[6][20];
    int i,j;
    int act_names=0;

    for (i = 0; i < names_found; i ++)
    {
        j = i + 1;
        while (j < names_found)
        {
            if (strcmp(words[i], words[j]) == 0)
            {
                memmove(words + j, words + (names_found - 1), sizeof(words[0]));

                -- names_found;
            }
            else
                ++ j;
        }
    }
    for(i=0;i<names_found;i++)
    {
        printf("%s\n",words[i]);
    }

}

How can the 2 for loops for i and j above be optimized?
As you see, the above logic currently outputs to same input buffer (in-place processing).  Can it be optimized further if we allow a second buffer say char out_words[2000][20]?

Your algorithm is \$O(n^2)\$.  You can reduce this to \$O(n)\$ by first inserting them into a closed hash table (i.e., one with bucket chains).
I'd optimise is rather than copying the whole items, just copy pointers to the original strings (unless you intend to alter their contents later on).


Determining whether two two-digit numbers share a digit in common I have solved a CodingBat problem: Given two ints, each in the range 10..99, return true if there is a digit that appears in both numbers, such as the 2 in 12 and 23. (Note: division, e.g. n/10, gives the left digit while the % "mod" n%10 gives the right digit.)

shareDigit(12, 23) → true
shareDigit(12, 43) → false
shareDigit(12, 44) → false

My working code is:

public boolean shareDigit(int a, int b) {
      boolean answer = false;
      int lefta = a/10;
      int righta = a % 10;
      int leftb = b/10;
      int rightb = b % 10;
      if(lefta == leftb || lefta == rightb || righta == leftb || righta == rightb){
        answer = true;
      }
      return answer;
}

It feels extremely inefficient, and I was wondering if anyone could improve it.

You can simplify the boolean parts of your method.

public boolean shareDigit(int a, int b) {
    int lefta = a / 10;
    int righta = a % 10;
    int leftb = b / 10;
    int rightb = b % 10;
    return lefta == leftb || lefta == rightb || righta == leftb
            || righta == rightb;
}


"Lost in the Woods" text-based game This is my first Python text-based game. Let me know if there is something I should improve on or fix in the way that my code is written because I feel that I am being very redundant in my code, but I'm just learning this stuff.

Also note that note.txt just says:

  this cabin will self destruct in.....

#Lost In the Woods - A simple text based game
from sys import exit

def start():
    print "You are in the middle of the woods alone and scared!"
    print "There is no one around but yourself."
    print "You look around and wonder....."
    print "What will you do?"
    print "1.move forward\n2.stay still"

    choice = raw_input("> ")

    if "move" in choice:
        bear_fight()
    else:
        print "Well your still at the same spot."
        start()

def bear_fight():
    print "Ahead of you is a huge hungry bear with razor sharp claws"
    print "Hes looks angry too!"
    print "Do you....."
    print "1.throw rock\n2.play dead\n3.run away\n"

    choice = raw_input("> ")

    if "run" in choice:
        print "Duhhh.... Did you think you can out run a bear?"
        dead("The bear catches up and has you for dinner, Good Job")
    elif "play dead" in choice:
        print "The bear laughs at you and chews but off."
        dead("You lost your but and lost this game")
    else:
        print "You deafted the bear with a rock! Hows that possible?"
        print "Anyways, You contine to move forward untill......"
        cabins()

def cabins():
    print "You come acroos two cabins."
    print "One of the left and one on the right"
    print "Whcih cabin do you pick?"
    print "1.left\n2.right\n"

    choice = raw_input("> ")

    if "left" in choice:
        cabin_one()
    else:
        cabin_two()

def cabin_two():
    print "You apprach the cabin on the right."
    print "You notice a note on the cabin door."
    print "Will you......"
    print "1.read the note\n2.open the door"

    choice = raw_input("> ")

    if "read" in choice:
        letter = open("note.txt", 'r')
        print letter.read()
        letter.close()
        for count in range(10, 0, -1):
            print count
        dead("The cabin explodes and you die")
    else:
        print "As you walk into the cabin"
        print "You slip on a toy car and crack you skull!"
        dead("Good job, mabey you should have read the note first!")

def cabin_one():
    print "You enter the cabin and decide to spend then night."
    print "You wake up the next morning and go outside."
    print "You notice the bight sun and a dustty trail on the ground."
    print "Will you follow the sun or follow the trail"
    print "1.follow sun\n2.follow trail"

    choice = raw_input("> ")

    if "sun" in choice:
        the_sun()
    else:
        the_path()

def the_sun():
    print "You sweet to death and die!"
    dead("Follow the path next time dummy")

def the_path():
    print "the path leads to a long road."
    print "How will you get home from here?"
    print "1.hich-hike\n2.walk the road"

    choice = raw_input("> ")

    if "hich" in choice:
        print "No bobdy comes for you."
        dead("You starve to death and die")
    else:
        print "You see civiliztion and stop at the first buger king."
        win("Congrats you win this silly game!")

def dead(msg):
    print msg + "!!!"
    exit(0)

def win(msg):
    print msg
    exit(0)

start()

Your code is indented well, and appears to have no problems in this way.

When you start the game, you should have all code that would otherwise just run defined in a method like this:

if __name__ == "__main__":
    start()

You can learn about this more here: What does if __name__ == “__main__”: do?

It is very good that your main delegates all the work of starting a game to a start() method.

You should probably run your text through a spell checker, should this be "nobody", "civilization", "Burger King" (capitalized as a proper noun), and "sweat"?

  "No bobdy comes for you."
  "You see civiliztion and stop at the first buger king."
  "You sweet to death and die!"

There are many other misspellings in here.

I would probably create a method to print the options the user has, and you can pass the options as a tuple:

def print_options(options):
    for i in range(0, len(options)):
        print str(i + 1) + ". " + options[i]

This can now be called like this:

print_options(("t1", "t2", "t3"))

Your game will be a little boring once the user has played it a couple times.  There seems to be no element of randomness, so there is only one path to victory.  The user won't want to lose, so once they realize this, they probably won't play your game.  To defeat this problem, you should probably use a random number generator to randomly choose options, which can be weighted to give certain actions a higher probability of happening.

In start(), you recursively call start() if the user chooses to stay still.  In this case, it probably is not a big deal because you don't create a bunch of variables and the user won't choose to stay still forever, but it isn't always a good idea to do this because the call will be added to the stack recursively until the stack overflows (if this point is ever reached); instead, you should use a loop to display the prompt until the user chooses to move.

Your win() and dead() methods are essentially the same.  You should combine these into one method for ending the game.

When you open and read the note at cabin 2, I would not put that in a file by itself.  It does not describe what the program is doing any more than just printing the note with print - in fact, I think print shows what is happening better.  What it is doing is describing what the user is doing, which is very different than what the program itself is doing.


Removing leading new line from byte[] Given this piece of code, what is a better way to write it (cleaner).
It is taken for granted that data will not be null, nor its length will be 0, so this check is skipped.

private byte[] process(byte[] bytes) {
    int from = 0;
    if (bytes[0] == '\n') {
        from = 1;
    } else if (bytes[0] == '\r' && bytes[1] == '\n') {
        from = 2;
    }
    return Arrays.copyOfRange(bytes, from, bytes.length);
}

I thought about converting it to String, then applying a regex (^[\r\n]), but that means some encoding has to be used for the conversion and then I would have to convert it back to byte[], which does not comply with my needs.
Is there any clearer way to write this?

I don't think you can make this specifically better.

What you could do is make the function more generic, but that depends on what your data contains beyond the first two bytes. For example, you could simply skip any and all line feeds and carriage returns at the beginning:

private int skipLFandCR(byte[] bytes) {
    int pos = 0;
    while (pos < bytes.length && (bytes[pos] == '\r' || bytes[pos] == '\n')) {
        pos++;
    }
    return pos;
}

Also: Do you need a copy of the data? I'd just use a function that returns the start index such as mine and process the existing array starting at that index.


Command line google searching I have a bash script that is meant to start a google search from the command line. I am also looking to implement a way of getting source code for the search page possibly through a subscript. This is what I have so far:

if [[ $1 == "-n" ]]; then
    if [[ $# -lt 2 ]]; then
        echo "Usage: google [-n] term [, term [, term...]]"
        exit 1
    fi
fi

if [[ $1 != "-n" ]]; then
    if [[ $# -lt 1 ]]; then
        echo "Usage: google [-n] term [, term [, term...]]"
        exit 2
    fi

    if [[ $(node -v) != *"v"* ]]; then
        echo "This tool uses node.js to escape url entities"
        echo "To use this tool without escapes include -n as the first agument"
        echo "(THIS IS NOT RECOMMENDED!)"
        exit 3
    fi
fi

search=""

for i in ${BASH_ARGV[*]}; do
    search="$i+$search"
done

escapedSearch=`node -e "a = escape(\"$search\"); console.log(a)"`

open "http://www.google.com/search?q=$escapedSearch"

I'm not quite certain about the use of node and escaping, and I'm going to assume that it does exactly what you need, focusing on the code and style of your script only.

First, I notice that you use an if statement to check whether $1 is -n, then another where you check whether it is not -n.  You could, of course, use an if/then/else construction for that.

Secondly, instead of looping backwards through the command line arguments with ${BASH_ARGV[*]}, concatenating them with +es, could have unexpected side effects if you use quoted spaces, as in:

for i in ${BASH_ARGV[*]}; do
    search="$i+$search"
done
echo "$search"

Will do something like (I'm going to call the script google.sh here):

-> % ./google.sh -n this and that "thing at once"
-n+this+and+that+once+at+thing+

Instead, you could use the bash built-in replacement function and the all-parameters variable like this:

args="$*"
search=${args// /+}
echo "$search"

Which would produce:

-> % ./google.sh -n this and that "thing at once"
-n+this+and+that+thing+at+once

Thirdly, I do like "one line ifs", and would change the two checks for missing arguments to -n like follows:

[[ $# -lt 2 ]] && echo "Usage: google [-n] term [, term [, term...]]" && exit 1

But that is a matter of taste.  It does save three lines, though, and makes clear that it is some kind of precondition check (the lines end in exit).

Lastly, I would always start a script with a shebang, making it executable from the command line directly, but this also depends on how it is going to be used.

Applying these points, the code would look like:

#!/usr/bin/env bash

if [[ $1 == "-n" ]]
then
    [[ $# -lt 2 ]] && echo "Usage: google [-n] term [, term [, term...]]" && exit 1
else
    [[ $# -lt 1 ]] && echo "Usage: google [-n] term [, term [, term...]]" && exit 2

    if [[ $(node -v) != *"v"* ]]; then
        echo "This tool uses node.js to escape url entities"
        echo "To use this tool without escapes include -n as the first agument"
        echo "(THIS IS NOT RECOMMENDED!)"
        exit 3
    fi
fi

args="$*"
search=${args// /+}

escapedSearch=`node -e "a = escape(\"$search\"); console.log(a)"`

open "http://www.google.com/search?q=$escapedSearch"

You would probably do something with the -n flag between the then and else, like setting a variable, and deciding whether to call node or not, before calling open.


Check assistant's password I have two more methods the same as this except that they query a different table. Is there another way to know if the result of the query is null? The hasher.CompareStringToHash() method is not allowed to have a null value.

If it's not null, it proceeds to an else condition, and inside that it checks if the encrypted password and password in the textfield are the same. If it's the same it will go to another else statement.

Is there another way to shorten this code? Keep in mind that I should not have a null value in the hasher.CompareStringToHash(). Or, if possible, restrict calling that method if the result is null and lessen the if statements. How can I make this code cleaner and more efficient?

public void CheckAssistantsPassword()
{
    DbClassesDataContext myDb = new DbClassesDataContext(dbPath);

    var password = (from userAccounts in myDb.Assistants
                    where userAccounts.Ass_UserName== txtUserName.Text
                    select userAccounts.Ass_Password).FirstOrDefault();

    var hasher = new Hasher() { SaltSize = 16 };

    if (password == null)
    {
        MessageBox.Show("Invalid Account");
    }
    else
    {
        bool isOkay = hasher.CompareStringToHash(txtPassword.Text,password);
        if (isOkay)
        {
            MessageBox.Show("You May Now Login");
        }
        else
        {
            MessageBox.Show("INVALID PASSWORD");
        }
    }
}

I believe you're looking for String.IsNullOrEmpty.

// test for null or empty string
if (!String.IsNullOrEmpty(password) {
}

// -- OR --

// If the hasher call just can't accept null values, you can specify
// an empty string to fall back on like so:
(...LINQ...).FirstOrDefault(String.Empty);

I would also probably refactor this a bit:

public void CheckAssistantsPassword()
{
  using (DbClassesDataContext myDb = new DbClassesDataContext(dbPath))
  {
    var password = (from userAccounts in myDb.Assistants
                    where userAccounts.Ass_UserName == txtUserName.Text
                    select userAccounts.Ass_Password).FirstOrDefault(String.Empty);

    var hasher = new Hasher { SaltSize = 16 };
    if (hasher.CompareStringToHash(txtPassword.Text, password))
    {
      // Success
    }
    else
    {
      // Invalid password
    }
  }
}

Optionally you can place error checking on your context making sure you can establish a connection. Also, I recommend placing a using() block around your context to free it up after you're done with it.


Updating access records I spent an hour or so this morning compiling this, and since I have started reading Clean Code and The Pragmatic Programmer I figured I would let you help me get slightly better at this.

Due to some crappy limitations with the specifics I was forced to work in Excel VBA instead of Access (someone doesn't want to build tables for the two lists in the Excel sheet).

The code pulls a list of defects from a production table, checks a master list to see if it ever existed, then checks an open list to see if it's current and updates the table accordingly. This could be super easy and potentially 100% automated if they would make tables for the two lists. The log of what was found per defect# (writing to sheet) is something I added just in case they want a log.

Private Sub thisbetheshitmane()
    Dim db As DAO.Database
    Dim rst As DAO.Recordset
    Dim vAr As String
    Dim i As Integer
    Dim y As Integer
    Dim InCombined As Boolean
    Dim InOpen As Boolean

    Set db = DBEngine.OpenDatabase("C:\Users\dzcoats\Documents\Microsoft.accdb")
    Set rst = db.OpenRecordset("SELECT DISTINCT [VDefects].Defect FROM [VDefects] WHERE [VDefects].Defect IS NOT NULL;")

    Dim QResult() As Variant
    QResult = rst.GetRows(rst.RecordCount)
    For a = LBound(QResult, 2) To UBound(QResult, 2)
        vAr = QResult(0, a)
        Debug.Print ; vAr
    Next a

    Dim CombinedList() As Variant
    CombinedList = Application.Transpose(Worksheets(1).Range("b2:b2000").Value)
    Dim OpenList() As Variant
    OpenList = Application.Transpose(Worksheets(1).Range("a2:a2000").Value)

    For y = LBound(QResult, 2) To UBound(QResult, 2)
        vAr = Trim(QResult(0, y))
        InCombined = False

        For a = LBound(CombinedList) To UBound(CombinedList)
            If vAr = CombinedList(a) Then InCombined = True
        Next a

        InOpen = False

        For a = LBound(OpenList) To UBound(OpenList)
            If vAr = OpenList(a) Then InOpen = True
        Next a

        If vAr <> "Defect" And vAr <> vbNullString And vAr <> "" Then

            If InCombined = False And InOpen = False Then
                set rst = db.OpenRecordSet ("UPDATE [VDefects] SET [VDefects].Status ='Bad Defect Number' WHERE ((([VDefect].Defect)='"& vAr &"'));")
                Debug.Print "BAD "; vAr
                ThisWorkbook.Sheets("Sheet2").Cells(Sheet2.Rows.Count, 1).End(xlUp).Offset(1, 0).Value = "Bad"
                ThisWorkbook.Sheets("Sheet2").Cells(Sheet2.Rows.Count, 1).End(xlUp).Offset(0, 1).Value = vAr
            End If

            If InCombined = True And InOpen = False Then
                set rst = db.OpenRecordSet ("UPDATE [VDefects] SET [VDefects].Status ='Completed' WHERE ((([VDefects].Defect)='"& vAr &"'));")
                Debug.Print "CLOSED "; vAr
                ThisWorkbook.Sheets("Sheet2").Cells(Sheet2.Rows.Count, 1).End(xlUp).Offset(1, 0).Value = "Closed"
                ThisWorkbook.Sheets("Sheet2").Cells(Sheet2.Rows.Count, 1).End(xlUp).Offset(0, 1).Value = vAr
            End If

            If InCombined = True And InOpen = True Then
                Debug.Print "OPEN "; vAr
                ThisWorkbook.Sheets("Sheet2").Cells(Sheet2.Rows.Count, 1).End(xlUp).Offset(1, 0).Value = "Open"
                ThisWorkbook.Sheets("Sheet2").Cells(Sheet2.Rows.Count, 1).End(xlUp).Offset(0, 1).Value = vAr
            End If
        End If
    Next y

    rst.Close
    Set rs = Nothing
    db.Close
    Set db = Nothing

End Sub

Here is the code refined using these features:

rst.Filter
rst.Update
Scripting.Dictionary
Range("A1").CopyFromRecordset rst

Sub ThisBeTheShitMane()
    Const DBPath = "C:\Users\dzcoats\Documents\Microsoft.accdb"
    Const DebugMode As Boolean = False

    Dim db As DAO.Database
    Dim rst As DAO.Recordset
    Dim key As String
    Dim vAr
    Dim d As Object: Set d = CreateObject("Scripting.Dictionary")

    Set db = DBEngine.OpenDatabase(DBPath)
    Set rst = db.OpenRecordSet("SELECT [VDefects].Defect, [VDefects].Status FROM [VDefects] WHERE [VDefects].Defect IS NOT NULL;")

    'Combined List
    With Worksheets(1)
        For Each vAr In .Range("B2", .Range("B" & .Rows.Count).End(xlUp)).Value
            key = vAr
            d(key) = "Completed"
        Next
    End With

    'Open List
    With Worksheets(1)
        For Each vAr In .Range("A2", .Range("A" & .Rows.Count).End(xlUp)).Value
            key = vAr
            If d.Exists(key) Then
                d(key) = "OPEN"
            Else
                If DebugMode Then Debug.Print "vAr: "; vAr, "ID is in the Open List but is missing from the Combined List "
            End If
        Next
    End With

    With rst
        .MoveFirst
        Do Until .EOF
            key = ![Defect]
            .Edit
            ![Status] = IIf(d.Exists(key), d(key), "Bad Defect Number")
            .Update
            .MoveNext
        Loop
        .MoveFirst
    End With

    Worksheets("Sheet2").Range("A1").CopyFromRecordset rst

    rst.Close
    Set rst = Nothing
    db.Close
    Set db = Nothing

End Sub

But we could just let the database do the work for us by converting the Open and Combined list into a comma separated values list and using IN() to check the values.  If [Defect] is a text field you will has to wrap the values in quotes.

Sample Query:

UPDATE VDefects SET VDefects.Status = IIf([VDefects]![Defect] NOT IN (1,2,3,4,5,6) And [VDefects]![Defect] NOT IN (4,5,6,7,8,9),'Bad Defect Number',IIf([VDefects]![Defect] NOT IN (1,2,3,4,5,6),'Completed','OPEN'));

Sub JustDoIt()
    Const DBPath = "C:\Users\best buy\Desktop\Microsoft.accdb"
    Dim db As DAO.Database: Set db = DBEngine.OpenDatabase(DBPath, , True)
    Dim rst As DAO.Recordset
    Dim sSQL As String, t1 As String, t2 As String
    Dim arr1 As Variant, arr2 As Variant

    With Sheet1
        t1 = getValueList(.Range("A2", .Range("A" & rows.Count).End(xlUp)), False)
        t2 = getValueList(.Range("B2", .Range("B" & rows.Count).End(xlUp)), False)
    End With

    sSQL = "UPDATE VDefects SET VDefects.Status = IIf([VDefects]![Defect] NOT IN (" & t1 & ") And [VDefects]![Defect] NOT IN (" & t2 & "),'Bad Defect Number',IIf([VDefects]![Defect] NOT IN (" & t1 & "),'Completed','OPEN'));"

    db.Execute sSQL

    Set rst = db.OpenRecordSet("SELECT [VDefects].Defect, [VDefects].Status FROM [VDefects] WHERE [VDefects].Defect IS NOT NULL;")
    Worksheets("Sheet2").Range("A1").CopyFromRecordset rst

    rst.Close
    Set rst = Nothing
    db.Close
    Set db = Nothing
End Sub

Function getValueList(Target As Range, WrapInQuotes) As String
    Dim arr As Variant
    arr = Application.TRanspose(Target.Value)

    If WrapInQuotes Then
        getValueList = Join(arr, """,""") & """"
    Else
        getValueList = Join(arr, ",")
    End If
End Function


Method to find posts in children categories I use the gem act_as_votable to allow users to select categories they like. To get the categories a user likes:

 current_user.get_voted(Category)

These categories have children categories, and I need to find the posts in those children categories.

I use this line to do it:

@categories_posts = current_user.get_voted(Category).flat_map { |category| category.children.flat_map(&:posts)}.delete_if{ |post| post.status == "draft"}

But I noticed in my logs that this was doing way too much computation for this task.

Is there a way to make it lighter ?

Some notes:

You should limit the lines to 80/100 chars.
When a call chain seems to include too much logic, move it to methods/class-methods of models.

As a first refactor, I'd implement Category#with_children and then write:

@categories_posts = current_user.
  get_voted(Category).
  flat_map(&:with_children).
  flat_map(&:posts).
  reject { |post| post.status == "draft" }

On a second refactor, it's nice to have relations instead of arrays as output. This way you can use scopes, paginators and such, even if you cannot directly use has_many here (because of the recursive category requirement):

categories = current_user.get_voted(Category).flat_map(&:with_children)
@categories_posts = Post.where(category: categories).published

That's to show the implementation, on the controller you should still have less logic, something like this: @categories_posts = current_user.posts_of_voted_categories.published.


Delete node from linked list, using offsetof to set the tail pointer This is a follow on from this question - Delete node from singly linked list with tail pointer in C

I've attempted to implement node deletion using the offsetof() macro mentioned by the answerer and after a lot of debugging I found that I needed to cast my pointer to a char* in order to be able to increment or decrement by 1 byte instead of 1 whole unit. The relevant line is commented below as //(RELEVANT LINE)

Is this the correct way to go about achieve this with only one double pointer? Is there a better way, avoiding casting to char*? Have I missed some fundamental of pointer arithmetic and I just got lucky here that my code hasn't fallen over?

For reference offsetof(lrsll_node, next) is 8, sizeof(lrsll_node) is 16

typedef struct lrsll_node {
    char *data;
    struct lrsll_node *next;
} lrsll_node;

typedef struct lrsll_list {
    lrsll_node *head;
    lrsll_node *tail;
} lrsll_list;

 lrsll_node *lrsll_delete(lrsll_list *list, char *data) {
        lrsll_node **node = &list->head;
        while (*node && strcmp((*node)->data, data) != 0) {
            node = &(*node)->next;
        }
        if (*node == NULL)
            return NULL;
        lrsll_node *deleted = *node;
        *node = deleted->next;

        if (deleted == list->tail) {

            //RELEVANT LINE
            list->tail = (node == &list->head) ?
                    NULL : (lrsll_node*) ((char*)node - offsetof(lrsll_node, next));
        }

        return deleted;
    };

Can be simplified

You chose to use a "pointer to pointer" technique to find and delete your node, but this led to using an offsetof macro in order to back up your tail pointer to the previous node.  If you instead used a "previous pointer" technique, your code would be much simpler to read and you wouldn't have to use any strange pointer subtraction tricks in order to find the previous node.  Here is how it would look like:

lrsll_node *lrsll_delete(lrsll_list *list, const char *data)
{
    lrsll_node *prev = NULL;
    lrsll_node *node = list->head;

    while (node != NULL && strcmp(node->data, data) != 0) {
        prev = node;
        node = node->next;
    }

    if (node == NULL)
        return NULL;

    if (prev == NULL)
        list->head = node->next;
    else
        prev->next = node->next;

    if (node == list->tail)
        list->tail = prev;

    return node;
}

Addendum: Linus Torvalds' comments

Apparently this whole code was motivated by something Linus Torvalds wrote about using pointers to pointers.  Here is the example demonstrating what he meant, taken from this blog post:

Given this list type:

typedef struct list_entry {
    int val;
    struct list_entry *next;
} list_entry;

This is the "wrong way" to delete a node:

list_entry *entry = head; /* assuming head exists and is the first entry of the list */
list_entry *prev = NULL;

while (entry) {
    if (entry->val == to_remove)     /* this is the one to remove */
        if (prev)
           prev->next = entry->next; /* remove the entry */
        else
           head = entry->next;      /* special case - first entry */

    /* move on to the next entry */
    prev = entry;
    entry = entry->next;
}

And here is what Linus considered the "correct way":

list_entry **pp = &head; /* pointer to a pointer */
list_entry *entry = head;

while (entry) {
    if (entry->val == to_remove)
        *pp = entry->next;

    pp = &entry->next;
    entry = entry->next;
}

And I will agree with Linus that for this case, the "double pointer" solution is better than the "prev pointer" version.  But there is an important difference between this example and the OP's code, which is that the list in this example has no special List structure containing a head and tail pointer.

Because you may need to set the tail pointer to the previous node, the benefit of using the double pointer disappears, because now you need to convert the double pointer back into a node pointer using the awful offsetof trick.  So for the particular data structure used by the OP, I would argue that the "prev pointer" solution is preferable.

In other words, you shouldn't always use the double pointer solution just because Linus said to use double pointers.  You should consider whether or not your code would benefit from using double pointers and then use them if they provide a net benefit.


Cookie consent box on button click I'm just getting started with frontend development. I tried to make a pretty barebones cookie consent notice. I still don't fully understand flexbox and positioning, so the code is messy.
HTML:
    <button id="shownotice">Click to ask</button>

Are you fine with cookies?

        <button class="consent_button" id="yes">Yes</button>
        <button class="consent_button" id="no">No</button>

CSS:
#shownotice {
    background-color: cyan;
    border:0;
    padding: 5px;
}

[hidden] { display: none !important; }

.consent_notice {
    display: flex;
    align-items: center;
    justify-content: space-between;
    position: fixed;
    background-color: red;
    bottom:0px;
    left: 0px;
    right: 0px;
    overflow: auto;
}

#buttons {
    display: flex;
    margin-right: 10px;
    flex-direction: column;
    padding: 5px;
}

.consent_button {
    margin: 5px;
    background-color: gray;
    border: 0;
    padding: 3px;
}

JS:
show_button = document.querySelector("#shownotice");
consent_elem = document.getElementById("consent_elem");

show_button.onclick = (e) => {
    consent_elem.hidden = consent_elem.hidden ? false : true;
}

IDs
Avoid IDs for styling. It's ok to use them to identify elements with JavaScript, but the specificity in CSS can make structuring the styles tricky.
Names such as buttons, yes and no are very generic, that could easily be needed more than once in a document.
Also try to be more consistant with the style of classes/names/IDs. For example, shownotice has no underline, but consent_notice does. Consider using hyphens instead of an underlines in order to match the general style of CSS, which uses hyphens, as in background-color.
HTML
"true" is not a valid value for the attribute for the attribute hidden. As a boolean attribute hidden is either present without a value (or, if you prefer use the value "hidden" as in hidden="hidden") or not present. hidden="false", for example, would still hide the element.
JavaScript
Declare your variables with const or let (or var if you need to support very old browsers).
In JavaScript it is convention to write variable names in camelCase and not with underlines.
You could be more consistent when retrieving elements by ID and use either querySelector or getElementById, but not both.
Don't use on... properties to assign event handlers. Use addEventListener.
Use the boolean negator ! to toggle boolean values:
consent_elem.hidden = !consent_elem.hidden;


Parsing locally stored HTML files I am working with this code to parse through HTML files stored on my computer and extract HTML text by defining a certain tag that should be found:

from bs4 import BeautifulSoup
import glob
import os
import re
import contextlib

@contextlib.contextmanager
def stdout2file(fname):
    import sys
    f = open(fname, 'w')
    sys.stdout = f
    yield
    sys.stdout = sys.__stdout__
    f.close()

def trade_spider():
    os.chdir(r"C:\Users\Independent Auditors Report")
    with stdout2file("auditfeesexpenses.txt"):
        for file in glob.iglob('**/*.html', recursive=True):
            with open(file, encoding="utf8") as f:
                contents = f.read()
                soup = BeautifulSoup(contents, "html.parser")
                for item in soup.findAll("ix:nonfraction"):
                    if re.match(".*AuditFeesExpenses", item['name']):
                        print(file.split(os.path.sep)[-1], end="| ")
                        print(item['name'], end="| ")
                        print(item.get_text())
                        break
trade_spider()

The code works perfectly thanks to the help of the stackflow community! As I am not an expert in python coding, I am wondering whether there are some magic tricks some of you might know, to speed up my code and reduce processing time as it has to parse through ~ 4 Million files.

Perhaps in a nutshell what my code does:
-> open text file -> parse through all html documents in set directory -> if regex is found, print result into open text file -> break, no more than one match and continue to next file...

I am open to any suggestions on improving this code.

Update:

Further Explanation: Basically I want to find a certain name attribute (name=".+AuditFeesExpenses") in each HTML document and IF this attribute is found I want to have the name of the file, the Name Attribute and the correlating HTML text be printed into a separat text file.

An example string that I extracted from a single HTML file is:

<ix:nonFraction name="f:AuditFeesExpenses" contextRef="c201" unitRef="u5" decimals="0" format="ixt:numcommadot">8,930</ix:nonFraction>

I don't know if this would be significant, but a first suggestion would be to replace the relatively costly re operation with the basic string operationitem['name'].endswith("AuditFeesExpenses").

Another possible suggestion, based on @Dex'ter's comment would be to change the stdout redirection into a regular .write() on the output file.

But what I'd really recommend is to profile the script to figure out the hot spots. I suspect that the bottleneck is within BeautifulSoup, and if that's the case, (given that you're only searching for a substring and not parsing) perhaps you could find an alternative search method.


create generic method for two different anonymous types Function 1:

 public HttpResponseMessage Get340BValidations()
        {
            int partitionId = GetPartitionId();
            //int partitionId = 10054;
            this.db.Database.CommandTimeout = 180;
            List<Validation340B> result = this.db.VW_Validation_340B.Where(x => x.partition_id == partitionId).ToList();

var duplicateClaims = result.GroupBy(a => a.claim_id).Where(g => g.Count() > 1).Select(y => new { claimID = y.Key, records = y.ToList() });

            var newResult = result.GroupBy(a => a.claim_id).Select(g => g.First()).ToList();

            foreach (var claim in duplicateClaims)
            {
                List<byte?> testIds = new List<byte?>();
                foreach (var record in claim.records)
                {
                    testIds.Add(record.claim_validation_test_id[0]);
                }
                newResult.Where(x => x.claim_id == claim.claimID).ToList().ForEach(y => y.claim_validation_test_id = testIds);

            }

}

Function 2:

public HttpResponseMessage GetQuantityValidations()
        {
            int partitionId = GetPartitionId();
            List<ValidationQuantity> = this.db.VW_Validation_Quantity.Where(x => x.partition_id == partitionId).ToList();
var duplicateClaims = result.GroupBy(a => a.claim_id).Where(g => g.Count() > 1).Select(y => new { claimID = y.Key, records = y.ToList() });

            var newResult = result.GroupBy(a => a.claim_id).Select(g => g.First()).ToList();

            foreach (var claim in duplicateClaims)
            {
                List<byte?> testIds = new List<byte?>();
                List<string> testNames = new List<string>();
                List<string> resultValues = new List<string>();
                foreach (var record in claim.records)
                {
                    testIds.Add(record.claim_validation_test_id[0]);
                    testNames.Add(record.Test[0]);
                    resultValues.Add(record.Result[0]);
                }

                newResult.Where(x => x.claim_id == claim.claimID).ToList().ForEach(y => { y.claim_validation_test_id = testIds; y.Test = testNames; y.Result = resultValues; });
}
            }

Two functions contains same set of code for finding the duplicate records and modification below part

var duplicateClaims = result.GroupBy(a => a.claim_id).Where(g => g.Count() > 1).Select(y => new { claimID = y.Key, records = y.ToList() });

            var newResult = result.GroupBy(a => a.claim_id).Select(g => g.First()).ToList();

            foreach (var claim in duplicateClaims)
            {
                ........
}

how can i simplify above two functions which uses the same set of code for finding the duplicates. need one common function which should behave like generic method accept different model as parameter and implement the same functionalities.

If you want to have only one method  you need to do the following:

Create an interface for properties that are common for both models like:

public interface IValidationQuality
{
    int partition_id { get; }
    int claim_id { get; }
    int claim_validation_test_id { get; }
}

(those are just few of them, you need to find the others)

Add this interface to both models:

public class VW_Validation_Quantity : IValidationQuality { }
public class VW_Validation_340B : IValidationQuality { }

finally you make a few small adjustments to one of the methods:

public HttpResponseMessage GetValidations(ICollection<IValidationQuality> items)
{
    int partitionId = GetPartitionId();
    List < IValidationQuality > = items.Where(x => x.partition_id == partitionId).ToList();
    var duplicateClaims = result.GroupBy(a => a.claim_id).Where(g => g.Count() > 1).Select(y => new { claimID = y.Key, records = y.ToList() });

    var newResult = result.GroupBy(a => a.claim_id).Select(g => g.First()).ToList();

    foreach (var claim in duplicateClaims)
    {
        List<byte?> testIds = new List<byte?>();
        List<string> testNames = new List<string>();
        List<string> resultValues = new List<string>();
        foreach (var record in claim.records)
        {
            testIds.Add(record.claim_validation_test_id[0]);
            testNames.Add(record.Test[0]);
            resultValues.Add(record.Result[0]);
        }

        newResult.Where(x => x.claim_id == claim.claimID).ToList().ForEach(y => { y.claim_validation_test_id = testIds; y.Test = testNames; y.Result = resultValues; });
    }
}

Which means that you pass each collection to this method and becasue both models share the same interface it can work with both of them.

Then you can call it like this:

var result1 = GetValidations(this.db.VW_Validation_Quantity);
var result2 = GetValidations(this.db.VW_Validation_340B);


Seemingly complicated way of displaying page views I have this code

//TRIGGER MULTIPLE VIEWS, WALL AND FORUM
if(isset($_COOKIE["projects-sorting"])) {
    $_SESSION['projectsDisplayMode'] = $_COOKIE["projects-sorting"];
}
if(isSet($_GET['mode'])) {
    $_SESSION['projectsDisplayMode'] = $_GET['mode'];
}
if(!isSet($_SESSION['projectsDisplayMode'])) {
    $_SESSION['projectsDisplayMode'] = "wall"; //default is wall mode
}
$mode = $_SESSION['projectsDisplayMode'];
setcookie("projects-sorting", $mode, time() + (86400 * 30), "/"); // 86400 = 1 day

Which to me looks overly complicated, but it does work.

What it does:

Checks  if a specific cookie exists, if it does, set session variable projectsDisplayMode to whatever that cookie's content is.
Check if _GET variable mode exists, if it does, set projectsDisplayMode to whatever that variable is (Would overwrite the last variable set if both exist)
Last check if the session variable is set at all, if it's not set it to "wall", as that is the default.
Now set $mode (A specific page variable) to be processed a bit more to show the page properly.

I'm not concerned with the afterwards processing, I just want to see if there is a better way that I can do the first bit that may be faster or more standard, or more pleasant to look at.

First I think that you need to reconcile your use of session vs. cookie.  In your answer you explain technically what is happening in your code, but not what the overall user experience requirement is.  You need to match your usage of cookies vs. sessions vs. HTML local storage (though not used here), which are all means to convey application state to what the actual user experience needs of the application are.

Does the cookie represent a default user preference?  If so, then should you be overriding it when the user passes parameters or otherwise changes mode setting in a session only context?

Why use session at all if you are always going to set a new "mode" cookie which overrides whatever is in session and there is not a session-specific override context?  Most likely you are propagating session ID by cookie anyway, so if user deletes cookies, you are going to lose state anyway.

If for example you remove session from the equation altogether, you may get something like:

$mode = 'wall';
if(!empty($_COOKIE['projects-sorting'])) {
    $mode = $_COOKIE['projects-sorting'];
}
if(!empty($_GET['mode'])) {
    $mode = $_GET['mode'];
}
if($mode !== $_COOKIE['projects-sorting']) {
    setcookie("projects-sorting", $mode, time() + (86400 * 30), "/");
}

You are doing nothing in code shown here to validate/sanitize the inputs. Both $_COOKIE and $_GET are use input and should be validated/sanitized accordingly before passing these values further into the application (or especially placing back into user cookie).

Maybe something as simple as validateMode() function.

function validateMode($array, $key) {
    if(empty($array[$key]) || !is_string($array[$key])) {
        return false;
    }
    switch($array[$key]) {
        case 'wall':
        case 'forum':
            return true;
        default:
            return false;
    }
}

And usage with my suggestion like:

$mode = 'wall';
if(validateMode($_COOKIE, 'projects-sorting')) {
    $mode = $_COOKIE['projects-sorting'];
}
if(validateMode($_GET, 'mode')) {
    $mode = $_GET['mode'];
}
if($mode !== $_COOKIE['projects-sorting']) {
    setcookie("projects-sorting", $mode, time() + (86400 * 30), "/");
}


Electronic Components in Java I've been watching digital technology in the last few days. This has aroused my interest. I would like to realize a program that simulates basic components of circuit technology and their functions. I have developed a small approach on how to implement the elements of circuit technology on an object-oriented level. Is that a good way of thinking, with which you can continue working? Or could one formulate the building blocks more elegantly in code?

Output.java

/* objects of classes that implement this interface can be used as signaler */

public interface Output {
    public boolean getOutput();
}

Signal.java

public class Signal implements Output {
    private boolean signal;

    public Signal(boolean value) {
        setSignal(value);
    }

    public void setSignal(boolean value) {
        signal = value;
    }

    public boolean getOutput() {
        return signal;
    }
}

LinkingElements.java

abstract public class LinkingElement {
    Output signal1;
    Output signal2;

    public LinkingElement(Output signal1, Output signal2) {
        this.signal1 = signal1;
        this.signal2 = signal2;
    }
}

And.java

public class And extends LinkingElement implements Output {
    public And(Output signal1, Output signal2) {
        super(signal1, signal2);
    }

    public boolean getOutput() {
        return signal1.getOutput() && signal2.getOutput();
    }
}

Or.java

public class Or extends LinkingElement implements Output {
    public Or(Output signal1, Output signal2) {
        super(signal1, signal2);
    }

    public boolean getOutput() {
        return signal1.getOutput() || signal2.getOutput();
    }
}

LightBulb.java

public class LightBulb {
    private Output signal;

    public LightBulb(Output signal) {
        this.signal = signal;
    }

    public void isActive() {
        if(signal.getOutput()) {
            System.out.println("The lightbulb is shining.");
        } else {
            System.out.println("The lightbulb is switched off.");
        }
    }
}

Main.java

This class is just for testing:

public class Main {
    public static void main(String[] args) {
        Signal s1 = new Signal(true);
        Signal s2 = new Signal(true);
        Signal s3 = new Signal(true);
        Signal s4 = new Signal(false);
        And a1 = new And(s1, s2);
        Or a2 = new Or(s3, s4);
        And a3 = new And(a1, a2);
        LightBulb lightBulb = new LightBulb(a3);
        lightBulb.isActive();
    }
}

Thanks for sharing your code.

What you posted is a nice data structure.

But OOP is about behavior. (we still need data structures to store state but usually objects have behavior.

Let see is this way: There are 3 Types of component in an electronic system:

signal source (such as switches and sensors)
signal receivers (such as Light bulbs, LEDs or electric engines)
logical elements (such as And, OR NOT).

The signals are no objects (with behavior since they are just messages transfered between the objects...

So I'd suggest classes like this:

// signal are still data structure
enum Signal { OFF, ON }

// some interfaces for abstraction
interface SignalReceiver {
    void consume(Signal signal, SignalSource from);
}
interface SignalSource {
    void triggers(SignalReceiver r);
}
interface LogicElement extends SignalReceiver, SignalSource {}

// a switch is a typica "signalSource"
class Switch implements SignalSource {
    private final Collection<SignalReceiver> receivers = new HashSet<>();
    private Signal state = Signal.OFF;
    private final String name;

    public Switch(String name) {
        super();
        this.name = name;
    }

    @Override
    public void triggers(SignalReceiver r) {
        receivers.add(r);
    }

    public void doSwitch() {
        state = Signal.values()[1 - state.ordinal()]; // get the other signal
        System.out.print("Switch "+name+" is "+state +" ");
        for (SignalReceiver signalReceiver : receivers) {
            signalReceiver.consume(state, this);
        }
    }
}

class LightBulb implements SignalReceiver {
    public LightBulb(SignalSource signalSource) {
        signalSource.triggers(this); // tells the source "trigger me!"
    }

    @Override
    public void consume(Signal signal, SignalSource from) {
        System.out.println("light is " + signal);
    }
}

class Or implements LogicElement {
    private final Collection<SignalReceiver> receivers = new HashSet<>();
    private final Map<SignalSource, Signal> inputs = new HashMap<>();

    Or(SignalSource... signalSources) { // "vararg" parameter, accepts many sources
        for (SignalSource signalSource : signalSources) {
            signalSource.triggers(this); // tells the source "trigger me!"
            inputs.put(signalSource, Signal.OFF);
        }
    }

    @Override
    public void triggers(SignalReceiver r) {
        receivers.add(r);
    }

    @Override
    public void consume(Signal signal, SignalSource from) {
        inputs.put(from, signal);
        Signal output = inputs.values().stream() // Java8 functional programming
                .max((s1, s2) -> s1.compareTo(s2))// depends on order in enum!
                .get();
        for (SignalReceiver signalReceiver : receivers) {
            signalReceiver.consume(output, this);
        }
    }
}

And this is the test:

public class Electronics {
    public static void main(String[] args) {
        // circuit configuration
        Switch s1 = new Switch("left");
        Switch s2 = new Switch("right");
        new LightBulb(new Or(s1, s2));

        // operation
        s1.doSwitch(); // light on
        s1.doSwitch(); // light off
        s2.doSwitch(); // light on
        s1.doSwitch(); // light on
        s2.doSwitch(); // light on
        s1.doSwitch(); // light off
    }
}

Yo may notice that I my approach has no "getter" method.
This is because "getter" as well as "setter" violate one of the most important OO principles: information hiding / encapsulation. You should avoid them on classes implementing actual behavior (on pure data objects they are OK...) The "rule of thumb" is: "Tell!, don't ask!"