Array initialization from text string I have a block of text where several arrays are printed out, with a name and values:

VAL1=10 20 30 40 50
VAL2=4 8 15 16 23 42

I have a function that looks for the value name, and sets an array of these values:
bool FetchValueArray(char* source, char* name, char* typeFormat, int count, void* destination, size_t destElementSize)
{
    int i;
    char *t;
    t=strstr(source,name);
    if (t)
        if (destination != NULL)
        {
            for (i = 0;i < count;i++)
                sscanf(t, typeFormat, (char*)destination + destElementSize * i);
            return true;
        }
    return false;
}

Called thus:
FetchValueArray(source, "VAL1=", "%d", 5, val1array, sizeof(val1array[0]);
FetchValueArray(source, "VAL2=", "%d", 6, val1array, sizeof(val1array[0]);

This is from a StackOverflow question. As the answerer wrote, it works, but it's not safe. If I call it with a count that is higher than the size of the array, I will corrupt the memory. I'm trying to wrap my head around this one and having a tough time. How can I have a function that will set values to a fixed array and be "safe"?

I'm not quite sure what you are asking for, but I guess you are looking for a solution similar to this (not compiled nor tested):

#include <stdio.h>   /* sscanf */
#include <stddef.h>  /* NULL */
#include <stdbool.h> /* C99 bool type */

typedef enum
{
  TYPE_CHAR  = 'c',
  TYPE_INT   = 'd',
  TYPE_FLOAT = 'f',
  // etc
} Type_t;

/* I placed destination first to use the same parameter order as standard library
   functions (strstr for example). Note the const correctness for read-only parameters. */

bool FetchValueArray (void*       dest,
                      const char* source,
                      const char* name,
                      size_t      count,
                      Type_t      type)

{
  const char* strptr;                  /* Some string pointer (not sure what it does) */
  BOOL is_found;                       /* Is the name found? */

  if(dest == NULL)                     /* No need to do anything if this parameter is NULL */
  {
    return false;
  }

  strptr = strstr(source, name);
  is_found = strptr != NULL;           /* Boolean arithmetic */

  if(is_found)
  {
    size_t i;
    size_t item_size;
    const char format[2] = {'%', (char)type};

    switch(type)
    {
      case TYPE_CHAR:  item_size = 1; break;
      case TYPE_INT:   item_size = 4; break;
      case TYPE_FLOAT: item_size = 4; break;
    }

    for(i=0; i<count; i++)
    {
      sscanf(strptr, format, (char*)dest + item_size*i);
    }
  }

  return is_found;
}

EDIT:
Regarding array bounds checking. First you have to decide whether this is actually your concern or if you should leave it up the caller. Most commonly, you document every parameter in the .h file and state how the function should be called. If someone still decides to call it with incorrect parameters, it is then their own fault. That's how most C functions work and in this particular case, that's what I would recommend.

Since C does not store the array size together with the array as one single data type, you can't really pass an array size "safely" between the caller and a function, the caller is the one with the knowledge about the size, so you simply have to trust the programmer writing the caller.

You could write something like this

bool FetchValueArray(char dest[25], ...

but there are still no guarantees by the standard that this is safe. Some compilers will toss a warning if you try to pass anything but a fixed char array of 25 items to this function, but they don't have to. And we'd lose the generic type, so it isn't helpful.

You could do something like this, but I would not recommend it:

#define SIZE 25

typedef union
{
  char  char_array[SIZE];
  int   int_array[SIZE];
  float float_array[SIZE];

} GenericSafeArray_t;

This will allocate SIZE*sizeof(float), since float happend to be the largest type. So it is memory-ineffective and not particularly flexible either.


regex check variable amount of digits Check for Expiry

Expiry can go from 1 to 299
Expiry can be 1 digit, 2 digits or 3 digits

This will be the Input I check my regExp against.
Rewritet 192.168.3.99, 57714, 192.168.3.3, 22) [*0 2] i0 exp276
Rewritet 192.168.3.99, 57714, 192.168.3.3, 22) [*0 2] i0 exp82
Rewritet 192.168.3.99, 57714, 192.168.3.3, 22) [*0 2] i0 exp2

exp will decrease every second by one
My RegExp
let regExp = \exp[0-2]{0,1}[0-9]{1,2}\

The regeExp matches everything up from 0 up to 299.
Try it here
What do you think about this regExp.
I'm looking forward to any advice about Regular Expressions

Shortcuts You can use 2 special regex syntax rules to make the pattern more concise:

When a token may or may not be matched, instead of {0,1}, it can be done with ? instead - they're exactly equivalent.

When you want to match a digit from 0 to 9, rather than writing out [0-9], you can use \d instead (they're exactly equivalent):

Typo? Regular expressions in JS are delimited by forward slashes, not backslashes.
Prefer const - Unless you plan on reassigning the regExp variable name, better to use const - only use let when you need to warn readers that the variable may be reassigned.
Missing semicolon Unless you're an expert and can avoid the pitfalls of Automatic Semicolon Insertion, I'd highly recommend using semicolons whenever appropriate, else you will occasionally run into hard-to-understand bugs.
Bug? You probably only want to match digits if they fulfill all the rules - I doubt you want to match the 30 part of 300, for example. I'd expect that you want a line that ends with 300 to not match at all, right? Fix it by adding a word boundary at the end:
const regExp = /exp[0-2]?\d{1,2}\b/;

Or, if the digits will always come at the end of the line, use $ and the m flag instead:
const regExp = /exp[0-2]?\d{1,2}$/m;

https://regex101.com/r/udp9BX/7


First domain-driven User entity class I'm still learning about domain driven design, and I have created my first entity class User.

Is this fully compliant with the DDD principles?  Particularly my setters because I have business rules in them.  Any other suggestions would be highly appreciated as well.

namespace Models\Entities;

class User
{
    private $id;
    private $firstName;
    private $lastName;
    private $email;
    private $password;
    private $registerDate;

    public function setId($id)
    {
        if (!$this->id) {
            if (is_int($id) && $id >= 1) {
                $this->id = $id;

                return $this;
            }

            throw new \InvalidArgumentException('ID must be a positive integer.');
        }

        throw new \InvalidArgumentException('ID has already been set.');
    }

    public function getId()
    {
        return $this->id;
    }

    public function setFirstName($firstName)
    {
        if (is_string($firstName) && !empty($firstName) && ctype_alpha($firstName)) {
            $length = $this->stringLength($firstName);

            if ($length >= 1 && $length <= 35) {
                $this->firstName = $firstName;

                return $this;
            }
        }

        throw new \InvalidArgumentException('First name must be an alpha string inclusively between 1 and 35 characters.');
    }

    public function getFirstName()
    {
        return $this->firstName;
    }

    public function setLastName($lastName)
    {
        if (is_string($lastName) && !empty($lastName) && ctype_alpha($lastName)) {
            $length = $this->stringLength($lastName);

            if ($length >= 1 && $length <= 35) {
                $this->lastName = $lastName;

                return $this;
            }
        }

        throw new \InvalidArgumentException('Last name must be an alpha string inclusively between 1 and 35 characters.');
    }

    public function getLastName()
    {
        return $this->lastName;
    }

    public function getFullName()
    {
        return $this->firstName . ' ' . $this->lastName;
    }

    public function setEmail($email)
    {
        if (is_string($email) && !empty($email)) {
            $sanitized = filter_var($email, FILTER_SANITIZE_EMAIL);
            $isValid   = filter_var($sanitized, FILTER_VALIDATE_EMAIL);
            $isValid   = $isValid ? $this->stringLength($email) <= 254 : false;

            if ($email === $sanitized && $isValid) {
                $this->email = $email;

                return $this;
            }
        }

        throw new \InvalidArgumentException('Email must be a string in this format: "[local]@[domain].[ext]".');
    }

    public function getEmail()
    {
        return $this->email;
    }

    public function setPassword($password)
    {
        if (is_string($password) && !empty($password)) {
            $length = $this->stringLength($password);

            if ($length >= 6 && $length <= 60) {
                $this->password = $password;

                return $this;
            }
        }

        throw new \InvalidArgumentException('Password must be a string inclusively between 6 and 60 characters.');
    }

    public function getPassword()
    {
        return $this->password;
    }

    public function setRegisterDate($date)
    {
        if (is_int($date) && !empty($date)) {
            // TO DO: date validation unix timestamp is_int

            $this->registerDate = $date;

            return $this;
        }

        throw new \InvalidArgumentException('Register date must be an integer and valid unix timestamp.');
    }

    public function getRegisterDate()
    {
        return $this->registerDate;
    }

    private function stringLength($input)
    {
        $encoding = mb_detect_encoding($input);
        $length   = mb_strlen($input, $encoding);

        return $length;
    }
}

Your class can absolutely be considered like a DDD domain entity.
However, I would suggest some improvements:

DDD likes to use an ObjectValue for the unique identifiers, you could use one instead of a simple integer.
You could separate the validation logic from the entity (not specific to DDD).
You should not have a getPassword() method but a verifyPassword($password) instead. Don't expose the password in plain text.
You could also separate the stringLength() into its own string utility class.
Regarding the use of a Uniw timestamp, keep in mind that DateTime is a better option since it embeds the time zone. Timestamps don't, which is always confusing when handling dates from all over the world...


Show different type of fonts I have the following code which return some text in different fonts (for the moment only TimesNewRoman and Arial).
It works fine but if I want to make it work for 20 or more fonts (but not necessarily font, also for font-size, or color, etc) I would have to declare a ton of variable and a very long Js code. Not to speak about CSS code.

I am wondering if there is another way to do this in an easier way and more closed to a deliverable code.

var field, theDivTNR, theDivA;

function init() {
  console.log("page loaded and DOM is ready");
  field = document.querySelector("#inputField");
  theDivTNR = document.querySelector("#theDivTNR");
  theDivA = document.querySelector("#theDivA");
}

// The next function is called each type a key has been
// typed in the input field
function showWhatITyped() {
  // fill the div with the content of the input field
  theDivTNR.innerHTML = field.value;
  theDivA.innerHTML = field.value;
}
#theDivTNR{
 font-family: timesnewroman;
 font-size: 20px;
}
#theDivA{
 font-family: arial;
 font-size: 20px;
}

 <title>JavaScript Course</title>

 <link rel="stylesheet" type="text/css" href="style.css">

Type text here: <input type="text" oninput= "showWhatITyped();" id="inputField" >

What you typed:

TimesNewRoman

Arial

JavaScript

var field, theDivTNR, theDivA;

Why are these in a global scope? It's a good habit to always create your own scope so that you don't have unwanted clashes between scripts which use the same names.

  console.log("page loaded and DOM is ready");

This is debug code, and should be deleted before the code is ready for review.

// The next function is called each type a key has been
// typed in the input field
function showWhatITyped() {
  // fill the div with the content of the input field
  theDivTNR.innerHTML = field.value;
  theDivA.innerHTML = field.value;
}

What if field.value is not valid HTML? I think this should set textContent instead of innerHTML.

HTML

If you have src you don't need type, because the type can be determined from the HTTP headers.

This is quite ugly and very old-fashioned. See e.g. https://stackoverflow.com/q/799981/573420

Type text here: <input type="text" oninput= "showWhatITyped();" id="inputField" >

That oninput is even more ugly, and wholly unnecessary. Just refactor the init method to say

  document.querySelector("#inputField").oninput = showWhatITyped;

You may notice that this eliminates field. It's not necessary to store it, provided that you refactor the listener to handle the event which is passed to it. And, in fact, you can also refactor it to be an anonymous function.

TimesNewRoman

Arial

You asked how to expand to more fonts etc. The simple answer is to use a loop rather than implementing it by hand. Add class="preview" to each of the preview divs, and then use document.querySelectorAll('.preview') to get hold of them.


Leetcode: Find-bottom-left-tree-value https://leetcode.com/problems/find-bottom-left-tree-value/

Given a binary tree, find the leftmost value in the last row of the tree.

Example 1:
Input:

  2
 / \
1   3

Output:
1
Example 2:
Input:

    1
   / \
  2   3
 /   / \
4   5   6
   /
  7

Output:
7
Note: You may assume the tree (i.e., the given root node) is not NULL.

Please review for performance. Thanks

using System;
using System.Collections.Generic;
using Microsoft.VisualStudio.TestTools.UnitTesting;

namespace GraphsQuestions
{
    /// <summary>
    /// https://leetcode.com/problems/find-bottom-left-tree-value/
    /// </summary>
    [TestClass]
    public class FindBottomLeftTreeValue
    {
        //Input:

        //   2
        //  / \
        // 1   3
        [TestMethod]
        public void SmallTreeTest()
        {
            TreeNode root = new TreeNode(2);
            root.left = new TreeNode(1);
            root.right = new TreeNode(3);
            Assert.AreEqual(1, FindBottomLeftValue(root));
        }

        //Input:

        //         1
        //        / \
        //       2   3
        //      /   / \
        //     4   5   6
        //        /
        //       7
        [TestMethod]
        public void BigTreeTest()
        {
            TreeNode root = new TreeNode(1);
            root.left = new TreeNode(2);
            root.left.left = new TreeNode(4);

            root.right = new TreeNode(3);
            root.right.left = new TreeNode(5);
            root.right.right = new TreeNode(6);
            root.right.left.left = new TreeNode(7);
            Assert.AreEqual(7, FindBottomLeftValue(root));
        }

        public int FindBottomLeftValue(TreeNode root)
        {
            if (root == null)
            {
                throw new NullReferenceException("root is empty");
            }
            Queue<TreeNode> Q = new Queue<TreeNode>();
            Q.Enqueue(root);
            int left = root.val;
            while (Q.Count > 0)
            {
                // we always push the left node first then we peek, so the first item is the most left
                //for the entire level of the tree
                int qSize = Q.Count;
                left = Q.Peek().val;
                for (int i = 0; i < qSize; i++)
                {
                    var current = Q.Dequeue();
                    if (current.left != null)
                    {
                        Q.Enqueue(current.left);
                    }

                    if (current.right != null)
                    {
                        Q.Enqueue(current.right);
                    }
                }
            }

            return left;
        }
    }

    /*
      Definition for a binary tree node.*/
    public class TreeNode
    {
        public int val;
        public TreeNode left;
        public TreeNode right;
        public TreeNode(int x) { val = x; }
    }
}

Please review for performance.

The performance looks fine to me. Some key observations:

It's clear that all nodes must be visited to compute the correct answer, so the solution cannot be better than \$O(n)\$ time.
Traversing in level order as you did will require as much additional space as the number of nodes on the most dense level.
Traversing depth first would require as much additional space as the longest path from root to leaf.

Without knowing in advance the shape of the tree (whether it's deep or wide),
and given TreeNode as defined and no additional helper data structures,
it's not possible to tell whether DFS or BFS is better.
So they are both equally fine, as neither uses space excessively.


Creating an inverted index in Python Here is the code I have written to create an inverted index dictionary for a set of documents:

inv_indx = {i:[] for i in corpus_dict}
for word in corpus_dict:
    for i in range(len(docs)):
        if word in docs[i]:
            inv_indx[word].append(i)

docs is a list of sets of the words in various documents:

[{'once','upon','a','time',...},{'lorum','ipsum','time'...},...]

corpus_dict is a set of all the words that appear in any of the documents:

{'once','upon','a','time','lorum','ipsum',...}

inv_index becomes a dictionary with each word in the corpus_dict as a key for a list of the document ids that contain that word:

{'once':[0],'time':[0,1],...}

The problem is this becomes very slow if the number of documents gets too big. How can I make this code more efficient?

Suggestions

You check all words in all documents. Try iterate only over docs (without unnecessary checks)
instead of create empty inv_indx = {i:[] for i in corpus_dict} you can use defaultdict

Code

from collections import defaultdict

inv_indx = defaultdict(list)
for idx, text in enumerate(docs):
    for word in text:
        inv_indx[word].append(idx)


Text tokenizer example Here is a partial example of a text tokenizer. I'm looking for ways to improve one particular line in this code:

  implicit class textFile(val fileName: String)  {

    def toDict() = {
      io.Source.fromFile(fileName).getLines.flatMap(_.split("\\\\r?\\\\n")).toList
    }
  }

  def filterComments(m: List[String]) : List[String] = m match {
    case Nil => List()
    case x :: xs => x.takeWhile(c => c != '#').trim :: filterComments(xs)
  }

  def filterEmptyLines(m: List[String]) : List[String] = m match {

    case Nil => List()
    case x :: xs => if(x.isEmpty) filterEmptyLines(xs) else x :: filterEmptyLines(xs)
  }

  def splitParameters(m: List[String]) : List[String] = m match {

    case Nil => List()
    case x :: xs => x.split("\\s*(=>|,|\\s)\\s*").map(_.trim).toList ++ splitParameters(xs)

  }

  val dict = "test.txt".toDict()
  println(splitParameters(filterEmptyLines(filterComments(dict))))

The line I would like to break up is the last one:

println(splitParameters(filterEmptyLines(filterComments(dict))))

Is there any idiomatic (Scala) way to make this more readable but keeping it compact, on the same line preferable?

I know you've found a solution you like for the final line of your code, but I'm a bit concerned with the overall performance and characteristics.of your code.

Oh, I do have an alternative to your compactness solution but I will keep that for a different, shorter answer.

Confusion of metaphors

filterEmptyLines filters the lines from dict.  filterComments does not.  It transforms them from possibly-commented lines into lines with no comments.  This may seem like a minor or pedantic point, but calling two different things by the same name can cause errors.  stripComments would be a better name, I think.

Overengineered and unsafe functions

None of your three functions are stack safe. Each one of them is recursive but none of them is tail-recursive.  This means that processing a large file could blow the stack.

Each one of them can be rewritten safely using combinator functions, which are usually more efficient than pattern matching and explicit recursion (and gives an extra bonus that I'll explain later).

filterComments

This is not tail recursive because it prepends the transformed x to the beginning of the list returned by xs.filterComments.  It could be rewritten with a tail-recursive inner helper function, but this function can be done much more simply as a map...

def filterComments(m: List[String]): List[String] =
  m map (_.takeWhile(c => c != '#').trim)

filterEmptyLines

Unsafe for the same reason as filterComments.  Can be written safely as

def filterEmptyLines(m: List[String]): List[String] =
  m filter (! _.isEmpty)

splitParameters

Same as the other two, but multiple recursive applications of ++ is even more expensive.  This can be rewritten safely as

def splitParameters(m: List[String]): List[String] =
  m flatMap (_.split("\\s*(=>|,|\\s)\\s*").map(_.trim))

Oh, what happened to .toList?  Why does that work without it?  The answer is that Scala's flatMap does an implicit conversion of the array to a list.  Google canBuildFrom if you want to learn something about Scala collection internals.

Over-specific types

Once those three functions are rewritten to use combinators rather than pattern matching, they don't need to take or return List[String].  They could take and return Seq[String].  This gives you more freedom abut what you pass in (could be List[String], could be some other Seq-based collection).  No performance penalty (the appropriate filter/map/flatmap of the actual type will be called) and much more flexibility.  OK, at the end you would have to convert the sequence back into a list (or whatever you want the final form to be), but this allows you delay that decision till it is important.  This is that extra bonus I mentioned before.

And I'm about to explain why using Seq - or possibly Iterator could give a big performance boost.

Multiple traversals and intermediate collections

getLines returns an iterator (a lazy, one-pass collection which only processes each element on demand).  But you immediately convert it to a list, reading the whole (potentially large) file into memory.  Then each transformation in turn creates an entirely new collection.  So you actually create 4 collections in a row, traversing the entire contents of the file 3 times (possibly 4 if there are no empty or comment-only lines).  But I'm pretty sure you only care about the final one.

Even if you always want to process the entire file, that's expensive (the bigger the file, the worse it gets).  And what if you only want to process the first n lines or process the file in chunks, not wasting memory on processed and not-yet-processed chunks?

There's a pretty simple solution which will give you all those options (but which doesn't force you to overcomplicate things just because you might want those options later).

Be lazy (use views, iterators or streams)
Don't keep a reference to any of the intermediate transformations.

Being Lazy

Option 1: List View

If you're sure you always want to read the whole file into a list, turn that list into a view.  When you apply map, filter' orflatMap` to a view, it doesn't process the whole collection yet.  It returns the original collection wrapped in a delayed transformation and only applies the transformation when you ask for the elements and only to the elements you ask for.  If you just apply another transformation to the new view, again, no processing is done, you just get a new view.  So if you apply all three functions to the view and only then ask for the results as a list, the original collection will only be traversed once, applying all three transformations to one element before proceeding to the next.

If you only take the first 10 lines from the final view, it will only process lines until it has 10 non-empty, non-comment-only lines.  The rest of the collection inside the view will remain untouched till you ask for it.

And none of the functions needs to know that laziness is happening, if they take and return Seq[String#, because a view is a Seq.  Bonus.

The drawback, compared to iterators, is that the entire original collection stays in memory until there is no reference to it and it can safely be garbage-collected.

Option 2: Iterators

getLines returns an iterator.  Why not stick with it.  map, filter and flatMap work the same with with iterators as with views.  So if your three functions take and return Iterator[String], it just works.

The bonus that 1) the whole file isn't even read until you ask for it all to be processed and 2) the original, pre-processed lines are definitely thrown away and garbage collected.

The danger is that you have to be very careful to only traverse each intermediate iterator once.  So don't keep any references around.

Option 3: Stream

You could convert the getLines iterator into a stream.  This has the lazy file-reading advantage of the iterator and doesn't have the touch-only-once limitation of iterators.  However, it's actually a touch trickier than with iterators to make sure you don't keep the whole file in memory.  Oh, and Stream is a Seq.

Whichever of the 3 options you choose, you can still have filterEmptyLines and friends take/return either Seq[String] or Iterator[String], because all Seq descendants have a toIterator method and vice versa.  So you can write them not to care about laziness or how you actually implement it.

Don't keep references (till you really need one)

For different reasons, you lose some of the benefits of laziness if you keep references to any of the intermediate collections (including the output of toDict).  With iterators, it's dangerous.  So

"test.txt".toDict().filterComments.filterEmptyLines.splitParameters

is batter than

val dict = "test.txt".toDict()
dict.filterComments.filterEmptyLines.splitParameters

(assuming the implicit class added)


Validating various input HTML forms I'm looking for a review of my code for better design (to make sure it's more readable and legible).

<?php

// define variables and set to empty values
$nameErr = $phoneErr = $streetErr = $cityErr = $stateErr = $zipErr = "";
$name = $phone = $street = $city = $state = $zip = "";

// start script if the form is submitted
if ($_SERVER["REQUEST_METHOD"] == "POST") {

    if (empty($_POST["name"])) {
    $nameErr = "Name is required";
    }
    else {$name = test_input($_POST["name"]);
            // check if name only contains letters and whitespaces
        if (!preg_match("/^[a-zA-Z ]*$/",$name)) {
            $nameErr = "Only letters and white space allowed";
        }
}

    if (empty($_POST["phone"])) {
        $phoneErr = "Phone # is required";
    }
    else {$email = test_input($_POST["phone"]);
        // check if phone only contains numbers and dashes in correct format
        if (!preg_match("/^[0-9]{3}-[0-9]{3}-[0-9]{4}$/",$phone)) {
            $phoneErr = "Please resubmit Phone # in the following format: 555-     555-5555";
        }
    }

    if (empty($_POST["street"])) {
    $streetErr = "Address is required";
    }
    else {$street = test_input($_POST["street"]);
     // check if address syntax is valid
      if (!preg_match("/^[0-9a-zA-Z. ]+$/",$street)) {
       $streetErr = "Address appears to be invalid.";
      }
    }

    if (empty($_POST["city"])) {
      $cityErr = "City is required";
    }
    else {$city = test_input($_POST["city"]);
 // check if city syntax is valid
      if (!preg_match("/^[a-zA-Z ]*$/",$city)) {
      $cityErr = "City appears to be invalid." ;
     }
    }

    if (empty($_POST["state"])) {
     $stateErr = "State is required";
}
    else {$state = test_input($_POST["state"]);
 // check if state is valid uppercase abbv. and two letters
 if (!preg_match("/^(A[LKSZRAP]|C[AOT]|D[EC]|F[LM]|G[AU]|HI|I[ADL N]
     |K[SY]|LA|M[ADEHINOPST]|N[CDEHJMVY]|O[HKR]|P[ARW]|RI|S[CD]
     |T[NX]|UT|V[AIT]|W[AIVY])$/",$state)) {
     $stateErr = "State appears to be invalid. (Hint...Use Upper-case!)" ;
   }
}

if (empty($_POST["zip"])) {
    $zipErr = "Zip Code field is required";
}
else {$zip = test_input($_POST["zip"]);
    // check if zip only contains numbers and dashes in correct format
    if (!preg_match("/^[0-9]{5}-[0-9]{4}$/",$phone)) {
        $phoneErr = "Please resubmit Zip Code in the following format: 55555-5555";
    }
}
}

function test_input($data) {
   $data = trim($data);
   $data = stripslashes($data);
   $data = htmlspecialchars($data);
   return $data;
}
?>

you should be using arrays to store your variable.
your coding if syntax are not indented and formatted well.
https://stackoverflow.com/questions/253030/best-way-to-format-if-statement-with-multiple-conditions
it is better to use

if ($_POST) { }

/** rather than **/

if ($_SERVER["REQUEST_METHOD"] == "POST") { }

in your case.

if anything else read about PSR1 or PSR2 coding standard.

http://www.php-fig.org/psr/psr-1/
http://www.php-fig.org/psr/psr-2/


Finding the longest substring in common with two large strings I want to build a function which finds the longest substring that is common between two large strings. I want to do this using the first two functions.
I'm running it on very large strings (containing more than million characters) and it's taking a lot of time. How can I optimize the function findlongest?

def hash_sequence(string, k):

    dictionary={}
    for i in range(len(string)-(k-1)):
        sequence = string[i:i+k]
        dictionary.setdefault(sequence,[]).append(i)
    return dictionary

def intersects(string1, string2, k): #what if k=0?
    dictionary = hash_sequence(string1, k)

    for i in range(len(string2)-1): #O(n) for sybstring in string

        if string2[i:i+k] in dictionary: #O(n
            return string2[i:i+k]
    return None

def findlongest(string1,string2):
    if intersects(string1,string2,1)==None:
        return None
    for i in range(2,min(len(string1),len(string2))):

        if intersects(string1,string2,i)==None:
            return intersects(string1,string2,i-1),len(intersects(string1,string2,i-1))

One big problem is that when you reach the longest common sequence and are ready to return, you re-run the previous iteration twice. Variables are your friends:

def find_longest(string1, string2):
    longest_seq = None

    for i in range(1, min(len(string1), len(string2))):
        # Store the current iteration's intersection
        current_seq = intersects(string1, string2, i)

        # If this was one character too long, return.
        # Else, a longer intersection was found. Store it.
        if current_seq == None:
            return longest_seq, i-1
        else:
            longest_seq = current_seq

    # If we get here, the strings were the same.
    # For consistency, return longest_seq and its length.
    return longest_seq, len(longest_seq)

By adding in the two variables, we were able to remove the if-statement at the beginning and now only have to call intersects() once in the for-loop.

Also, take a look at PEP8, the official Python style guide. It will help you fit Pythonic conventions more closely.

If you wanted to use a more complex, and quite possibly faster, solution, take a look into implementing a binary search algorithm. They provide search times that are very efficient for large data sets.


Horizontal mega menu with flexbox I created a mega menu using flexbox, and because I'm not really into CSS, a review would be helpful.

I know it would be better to avoid the third ul-level but this is not possible, because the HTML is generated by another system. And we decided to use the second ul level as a marker for a new row in the mega menu.

With saying "Mega Menu" I mean that when you hover over "products" you will not just see a single list of sub items, but instead multiple lists. Often in "Mega Menus" they also add more content like text or pictures (not needed for me).

ul {
  width: 100%;
  display: flex;
  flex-direction: column;
  list-style: none;
  margin: 0 0 0 0;
  padding: 0;
  position: relative;
  background: red;
}
ul li {
  flex: 1 1 auto;
  display: block;
  text-align: left;
}
ul li:hover {
  background: tomato;
}
ul li:hover ul {
  display: flex;
}
ul li a {
  display: block;
  padding: 1rem 0;
}
ul li ul {
  display: none;
  left: 0;
  width: 100%;
}
ul li ul li {
  flex: 0 0 auto;
}

@media screen and (min-width: 37.5em) {
  ul {
    flex-direction: row;
  }
  ul li {
    text-align: center;
    background: green;
  }
  ul ul {
    position: absolute;
  }
  ul ul a {
    padding: 1rem;
  }
  ul ul li {
    position: relative;
    text-align: left;
  }
  ul ul li ul {
    position: relative;
  }
  ul ul li ul li {
    flex-basis: 100%;
  }
}

home

products

short

long

item 1

long item 2

whatever

superlong sub 1

s 2

    </l>

go

on

I'd change the color scheme,  the colors are a retinal burning  :)   Also, it would make the colors easier to read.   Also you have the sub menu over to the left irregardless of what choice is selected.  I'm a fan of the choices being under the selection I mouse over myself.   Other than that, it looks good!


Parenthesis-realigning program This is my code for an assignment. The task is to take a user input in form of a sequence of parenthesis' followed by an E to mark its end and determine whether it is a correct sequence (as an example (()())E is correct, )(()((E is incorrect) and if incorrect determine how many parentheses at the start you need to remove and how many to add at the end for it to be correct.
The code works. But it's super long and I wondered if there were any improvements I could do on it.
public class Main {

  public static String enter(){
    String enter = "";
    char nextValue ='A';
    while (nextValue!='E'){
      nextValue = In.readChar();
      enter += nextValue;
    }
    return enter;
  }
/*User Input. I know this could be done with ArrayList or scanner, but we haven't had these yet in classes.
So I did it by converting the user inputs into a string,
then storing that string into an array in main with "char []sequence = enter.toCharArray();"*/

  public static char[] removeParenthesisPairs(char[] a){
    for(int i=0; i<a.length; i++){
      if(a[i]=='('){
        a[i]='F';
        outerloop:
        for(int j=i+1; j<a.length;j++){
          if(a[j]==')'){
            a[j]='F';
            break outerloop;
          }
          if(j==a.length-1){
            a[i]='(';
          }
        }
      }
    }
    return a;
  }
/*I remove all the correct pairs from the sequence. I look for a ( and turn it into an F [to signal it's removed]
and search the rest of the sequence for a ) and turn that into an F too. If there is no ) I turn the F back into (.
This part I really don't like, it looks super clunky. Also I have read that the "break outerloop"
thing I'm using here is bad practice, but I havent figured out any other way to accomplish the same result.*/

  public static boolean checkIfCorrect(char[] a){
    for(int i=0;i<a.length-1;i++){
      if(a[i]!='F'){
        return false;
      }
    }
    return true;
  }
/* I check if the sequence is correct. If there are only F left in the array,
that means all entries were able to form pairs of () and the original sequence was correctly parenthesised.*/

   public static int subtract(char[] a){
    int count = 0;
    for(int i=0; i<a.length;i++){
      if(a[i]==')'){
        count += 1;
      }
    }
    return  count;
  }
// Check how many ) have to be subtracted at the beginning of the sequence.

  public static int add(char[] a){
    int count = 0;
    for(int i=0; i<a.length;i++){
      if(a[i]=='('){
        count += 1;
      }
    }
    return count;
  }
/*Check how many ( are still left in the sequence and need a ) added to form a pair.
I am kinda repeating myself here in the subtract() and add() methods, they are essentially doing
the same thing. The only way I have figured out for a method to return two values is by making
a class. But it seems even more convoluted to add new classes or is it preferable to this solution?*/

  public static void main(String[] args) {
    String enter= enter();
    char []sequence = enter.toCharArray();
    removeParenthesisPairs(sequence);
    boolean correct = checkIfCorrect(sequence);
    int additional = add(sequence);
    int removed = subtract(sequence);
    Out.println(correct);
    Out.println("removed = " + removed + " und additional = " + additional);
  }
/*I'm required by the assignment to have the boolean correct, int additional and int removed like this.
So I can't just do "Out.println("removed = " + subtract(sequence)+ " und additional = " + add(sequence));"*/
}

break is not a bad thing. Labels and goto are normally avoided. You are just breaking out of one loop, so you can just delete the outerloop: label and break; normally.

Comments at a method level are normally given with /** ... */ (JavaDoc syntax) above the method.

The trend is to avoid updating arrays in place (mutating), and instead just prefer returning new copies of the array (think of things as immutable), because when you mutate things it can sometimes be harder to keep track of state and easier to write incorrect code.
That's not to say it's wrong to do it with mutation. However, it might be better to keep mutation under control in the private parts of your code, so that if somebody re-uses your code, they don't have to worry about sequencing the method calls in the right order.

Presumably In is something provided by your course? Since I didn't have that, I just did the Scanner implementation to test with.

Here's your own code again, just with small changes as per the above comments, and formatted with 4 spaces (which seems to be more common these days).
import java.util.Scanner;

public class Main {

    public static void process(String s) {
        char[] sequence = s.toCharArray();
        removeParenthesisPairs(sequence);

        /*
         * I'm required by the assignment to have the boolean correct, int additional
         * and int removed like this. So I can't just do "Out.println("removed = " +
         * subtract(sequence)+ " und additional = " + add(sequence));"
         */
        boolean correct = checkIfCorrect(sequence);
        int additional = add(sequence);
        int removed = subtract(sequence);
        System.out.println(correct);
        System.out.println("removed = " + removed + " und additional = " + additional);
    }

    /**
     * I remove all the correct pairs from the sequence. I look for a ( and turn it
     * into an F [to signal it's removed] and search the rest of the sequence for a
     * ) and turn that into an F too. If there is no ) I turn the F back into (.
     * This part I really don't like, it looks super clunky.
     */
    private static char[] removeParenthesisPairs(char[] a) {
        for (int i = 0; i < a.length; i++) {
            if (a[i] == '(') {
                a[i] = 'F';
                for (int j = i + 1; j < a.length; j++) {
                    if (a[j] == ')') {
                        a[j] = 'F';
                        break;
                    }
                    if (j == a.length - 1) {
                        a[i] = '(';
                    }
                }
            }
        }
        return a;
    }

    /**
     * I check if the sequence is correct. If there are only F left in the array,
     * that means all entries were able to form pairs of () and the original
     * sequence was correctly parenthesised.
     */
    private static boolean checkIfCorrect(char[] a) {
        for (int i = 0; i < a.length - 1; i++) {
            if (a[i] != 'F') {
                return false;
            }
        }
        return true;
    }

    /**
     * Check how many ) have to be subtracted at the beginning of the sequence.
     */
    private static int subtract(char[] a) {
        int count = 0;
        for (int i = 0; i < a.length; i++) {
            if (a[i] == ')') {
                count += 1;
            }
        }
        return count;
    }

    /**
     * Check how many ( are still left in the sequence and need a ) added to form a
     * pair. I am kinda repeating myself here in the subtract() and add() methods,
     * they are essentially doing the same thing. The only way I have figured out
     * for a method to return two values is by making a class. But it seems even
     * more convoluted to add new classes or is it preferable to this solution?
     */
    private static int add(char[] a) {
        int count = 0;
        for (int i = 0; i < a.length; i++) {
            if (a[i] == '(') {
                count += 1;
            }
        }
        return count;
    }

    public static void main(String[] args) {
        try (Scanner scanner = new Scanner(System.in)) {
            process(scanner.next());
        }
    }

}

Before I looked at your code, I attempted the problem myself. Here's what I came up with - take from it what you will or leave it. I think our two versions produce different outputs for some inputs - that could be different interpretations of the problem, or one of us has got bugs! (Edit: Actually, they do seem to be the same after all).
import java.util.Scanner;

public class Parens {

    public static void main(String[] args) {
        try (Scanner scanner = new Scanner(System.in)) {
            System.out.print("Input: ");
            Parens.displayBalancing(scanner.next());
            System.out.println();
        }

        System.out.println("Extra tests cases");
        for (String test : new String[] { "(()())E", ")(()((E", "())(E" }) {
            Parens.displayBalancing(test);
        }
    }

    public static void displayBalancing(String s) {
        Parens.Changes changes = balance(s);
        System.out.format("removed: %-2d  additional:  %-2d correct: %-5b  input: %s\n",
                changes.removed, changes.additional, changes.isNoChanges(), s);
    }

    public static class Changes {
        int removed;
        int additional;

        Changes(int remove, int add) {
            this.removed = remove;
            this.additional = add;
        }

        public boolean isNoChanges() {
            return removed == 0 && additional == 0;
        }
    }

    public static Changes balance(String s) throws IllegalArgumentException {
        return balance(s.toCharArray(), 0, 0);
    }

    private static Changes balance(char[] characters, final int remove, final int add)
            throws IllegalArgumentException {
        int open = 0;
        for (char c : characters) {
            switch (c) {
            case '(':
                open++;
                break;
            case ')':
                if (open == -remove) {
                    // got close paren but none are open
                    return balance(characters, remove + 1, add);
                }
                open--;
                break;
            case 'E':
                if (open != add - remove) {
                    // got E but some parens are still open
                    return balance(characters, remove, add + 1);
                }
                // balanced
                return new Changes(remove, add);
            default:
                throw (new IllegalArgumentException(
                        String.format("Unexpected character '%c'", c)));
            }
        }
        throw (new IllegalArgumentException("String didn't contain an E"));
    }

}

Then I realised your approach was actually pretty good. This should work too:
    public static Changes balance2(String s) {
        while (s.contains("()")) {
            s = s.replace("()", "");
        }
        int remove = 0;
        int add = 0;
        for (char c: s.toCharArray()) {
            if (c == 'E') {
                break;
            } else if (c == '(') {
                add++;
            } else if (c == ')') {
                remove++;
            }
        }
        return new Changes(remove, add);
    }


Create Xml to string serializer I don't want to serialize everything. I created a custom attribute DoNotSerializeAttribute. If some property in info contain that attribute then ignore it and do not serialize it.

Is my code bug-free? Maybe I can improve it or I miss something?

class Program
{
    static void Main(string[] args)
    {
        var s = Serialize(new BackgroundJobInfo {Text = "toto", BackgroundJob = new BackgroundJob { Password = "pass"}});
        var myJob = Deserialize(s);
    }

    public static string Serialize(BackgroundJobInfo info)
    {
        using (var stringWriter = new StringWriter(CultureInfo.InvariantCulture))
        {
            var writer = XmlWriter.Create(stringWriter);
            var dataContractSerializer = new DataContractSerializer(typeof(BackgroundJobInfo),
                                                                    null,
                                                                    int.MaxValue,
                                                                    true,
                                                                    true,
                                                                    new MySurrogate());
            dataContractSerializer.WriteObject(writer, info);
            writer.Flush();

            return stringWriter.ToString();
        }
    }

    public static BackgroundJobInfo Deserialize(string info)
    {
        using (var stringReader = new StringReader(info))
        {
            try
            {
                var xmlReader = XmlReader.Create(stringReader);
                var dataContractSerializer = new DataContractSerializer(typeof(BackgroundJobInfo));
                return (BackgroundJobInfo) dataContractSerializer.ReadObject(xmlReader);
            }
            catch (Exception e)
            {
                // hopefully, will never happen
                return null;
            }
        }
    }
}

internal class MySurrogate : IDataContractSurrogate
{
    public Type GetDataContractType(Type type)
    {
        return typeof (BackgroundJobInfo);
    }

    public object GetObjectToSerialize(object obj, Type targetType)
    {
        var maskedProperties = obj.GetType().GetProperties();
        var b = maskedProperties.Where(m => m.GetCustomAttributes(typeof(DataMemberAttribute), true).Any() &&
                                            m.GetCustomAttributes(typeof(DoNotSerializeAttribute), true).Any());
        foreach (var member in b)
        {
            member.SetValue(obj, null, null);
        }
        return obj;
    }

internal class DoNotSerializeAttribute : Attribute
{
}

[KnownType(typeof(BackgroundJob))]
[DataContract]
public class BackgroundJobInfo
{
    [DataMember(Name = "text")]
    public string Text { get; set; }

    [DataMember(Name = "backgroundJob")]
    public BackgroundJob BackgroundJob { get; set; }
}

[DataContract]
public class BackgroundJob
{
    [DataMember(Name = "password")]
    [DoNotSerializeAttribute]
    public string Password { get; set; }
}

Although you are using the using statement with StringReader and StringWriter both the XmlReader and XmlWriter are implementing IDisposable as well, so the usage of it should be enclosed in a using statement too.
Catching an exception without doing anything with it is usually bad practice. You should at least log the exception.
Although you have named your variables and methods well, I am a little bit concerned about the info parameter because it isn't telling anything about what it is.


Multiple if conditions in one block - looking for faster and more efficient code i am looking for a faster and more efficient way of this code. What the code does is simply compares message context of (user, application, service and operation) and compares it with some rules.

private boolean match(Context messageContext, ContextRule contextRule) {
        if(contextRule.getMessageContext().getUser().equals(ContextRuleEvaluator.WILDCARD)
            || (contextRule.getMessageContext().getUser().equals(messageContext.getUser()))) {
          if(contextRule.getMessageContext().getApplication().equals(ContextRuleEvaluator.WILDCARD)
              || (contextRule.getMessageContext().getApplication().equals(messageContext.getApplication()))) {
            if(contextRule.getMessageContext().getService().equals(ContextRuleEvaluator.WILDCARD)
                || (contextRule.getMessageContext().getService().equals(messageContext.getService()))) {
              if(contextRule.getMessageContext().getOperation().equals(ContextRuleEvaluator.WILDCARD)
                  || (contextRule.getMessageContext().getOperation().equals(messageContext.getOperation()))) {
                return true;
              }
            }
          }
        }

        return false;
      }

Edit: Context and ContextRule are an interfaces

public interface Context {
   public String getUser();
   public void setUser(String user);
   public String getApplication();
   public void setApplication(String application);
   public String getService();
   public void setService(String service);
   public String getOperation();
   public void setOperation(String operation);
}

public interface ContextRule {
  public Context getMessageContext();
  public int getAllowedConcurrentRequests();
}

Any suggestions appreciated :)

First of all, your formatting seems messy, please fix it.

Did you rip it out or is there really zero documentation in there (JavaDoc)? Consider adding some, it will make your life and that of others easier.

First step would be to add an overload to match() which accepts two strings, like this:

private boolean match(Context messageContext, ContextRule contextRule) {
    return match(contextRule.getMessageContext().getUser(), messageContext.getUser())
            && match(contextRule.getMessageContext().getApplication(), messageContext.getApplication())
            && match(contextRule.getMessageContext().getService(), messageContext.getService())
            && match(contextRule.getMessageContext().getOperation(), messageContext.getOperation());
}

private boolean match(String value, String contextValue) {
    return value.equals(ContextRuleEvaluator.WILDCARD) || value.equals(contextValue);
}

This makes it already a lot easier to read. Naming of the variables is a little bit suboptimal, that's because I could not come up with better names right now.

Ultimately you should see if you can't extend and change Context to contain a match() function. This would require to change Context into an abstract class, so it might not be possible.

public abstract class Context {

    public abstract String getUser();

    public abstract void setUser(String user);

    public abstract String getApplication();

    public abstract void setApplication(String application);

    public abstract String getService();

    public abstract void setService(String service);

    public abstract String getOperation();

    public abstract void setOperation(String operation);

    /**
     * Matches this context against the given Context.
     *
     * @param context
     * @return
     */
    public boolean match(Context context) {
        return match(getUser(), context.getUser())
                && match(getApplication(), context.getApplication())
                && match(getService(), context.getService())
                && match(getOperation(), context.getOperation());
    }

    private boolean match(String value, String otherValue) {
        return value.equals(ContextRuleEvaluator.WILDCARD) || value.equals(otherValue);
    }
}

That would allow you to do this in your code:

return contextRule.getMessageContext().match(messageContext);

Which is very readable, even if you don't pack it into a function but call it directly.


Nearest Neighbor Algorithm (general nearest, forward-nearest, or backward-nearest) I am trying to write an algorithm that can find user-specified nearest neighbors. By user-specified, I mean that the user can specify whether it's a general nearest neighbor, a forward-nearest neighbor, or a backward-nearest neighbor.

The idea for this code was inspired from this SO post. While it isn't ideal to search the entire array (perhaps use searchsorted as an alternative), I want to find all occurrences of the user-specified nearest value in the given data array. While there are other techniques that can be used to achieve the same goal (such as using the cumulative sum of differences of argsorted values), I feel the code below is easier to read/understand and is likely quicker since it performs less operations that require traversing the entire data array. That said, I would like to know if there are better approaches (in terms of speed) to achieve the same output, as this code will be applied to a dataset of at least ~70,000 data points. More than the value itself, I am concerned with the indices at which the values occur.

import numpy as np

Sample Data

sample = np.array([300, 800, 200, 500, 600, 750, 700, 450, 400, 550, 350, 900])
# sample = np.array([300, 800, 200, 500, 600, 750, 700, 450, 400, 550, 350, 900] * 2)

Main algorithm

def search_nearest(data, search_value, direction=None):
    """
    This function can find the nearest, forward-nearest, or
    backward-nearest value in data relative to the given search value.
    """

    if not isinstance(data, np.ndarray):
        data = np.array(data)

    print("\n>>     DATA\n{}\n".format(data))
    print(">>     SEARCH VALUE\n{}\n".format(search_value))

    if direction is None:
        delta = np.abs(data - search_value)
        res = np.where(delta == np.min(delta))[0]

    elif direction == 'forward':
        delta = data - search_value
        try:
            res = np.where(delta == np.min(delta[delta >= 0]))[0]
        except:
            raise ValueError("no forward nearest match exists")

    elif direction == 'backward':
        delta = search_value - data
        try:
            res = np.where(delta == np.min(delta[delta >= 0]))[0]
        except:
            raise ValueError("no backward nearest match exists")

    print(" .. INDEX OF NEAREST NUMBER\n{}\n".format(res))
    print(" .. NUMBER AT THAT INDEX\n{}\n".format(data[res]))
    print("--------------------")

Call the main function

# crd = None
crd = 'forward'
# crd = 'backward'

for val in (799, 301, 800, 250, 8, 901):
    search_nearest(sample, search_value=val, direction=crd)

code

Some of your program is copy-n-pasted code; there is an opportunity to refactor.

Your docstring offers little guidance on valid direction values. Consider using enum.Enum. If None becomes 0, then 1 and -1 could represent the directions, with delta multiplied by the sign, killing two cases with one stone.

The identifier res for result is pretty OK, but consider using an abbreviation like idx for index, clarifying that it is not the "value" that the docstring said would be computed. Better yet, turn it into a function that is documented to return an index, and push the print statements into a calling function.

crd = 'forward'

I'm willing to believe that "d" denotes "direction", but the "cr" part of that identifier is obscure.

algorithm

  if there are better approaches (in terms of speed) to achieve the same output

Your main function suggests that you will repeatedly interrogate a large (70k) sample with diverse search values. In which case your data structure is not well suited to your task.

You are forcing pandas to read all N values on each query. Don't do that.

Build a two-column copy of the input that stores (index, value). Sort it by value. So far the cost is N log N.

Now for each query, binary search for search_value to land on result_index, at cost of log N. The desired answer is at result_index, or the entry before it, or the entry after it. Then use the stored index to report the corresponding index in the original input data.


Backbone/Marionette form with list of inputs Using Backbone/Marionette, I'm building a simple widget that looks like:

I'm hoping to get some feedback on my approach because I think I may be doing it wrong.

jsfiddle

// widget/invites/person.js
var Person = Backbone.Model.extend({
    defaults: {
        name: ''
    }
});

// widget/invites/people.js
var People = Backbone.Collection.extend({
    model: Person
});

// widget/invites/personField.js
var PersonView = Marionette.ItemView.extend({
    template: _.template(...),
    hasSiblings: false,
    initialize: function(opts) {
        this.hasSiblings = opts.hasSiblings || false;
    },
    templateHelpers: function() {
        return {
            id: this.model.cid,
            showDel: this.hasSiblings
        }
    },
});

// widget/invites/form.js
var FormView = Marionette.CompositeView.extend({
    template: _.template(...),
    childView: PersonView,
    childViewOptions: function() {
        return {
            hasSiblings: (this.collection.length > 1)
        };
    },
    childViewContainer: '.values',
    ui: {
        'addBtn': '.add-name',
        'removeBtn': '.del-name',
        'wrap': '.values'
    },
    events: {
        'click @ui.addBtn': 'addFormRow',
        'click @ui.removeBtn': 'removeFormRow'
    },
    addFormRow: function(ev) {
        var name;
        ev.preventDefault();

        if (this.collection.length === 1) {
            // also rerender first input to include remove button
            name = this.ui.wrap.find('[name="person[]"]').get(0).value;
            this.collection.reset([{name: name}, {name: ''}]);
        }
        else {
            this.collection.add({name:''});
        }
    },
    removeFormRow: function(ev) {
        var id, model;
        ev.preventDefault();

        id = $(ev.currentTarget).data('id');
        model = this.collection.get(id);
        this.collection.remove(model);

        if (this.collection.length === 1) {
            // rerender first input so it doesn't have remove button
            name = this.ui.wrap.find('[name="person[]"]').get(0).value;
            this.collection.reset([{name: name}]);
        }
    }
});

Note, the list is saved by clicking a Submit button (not shown in mockup), as there will be other fields in the form as well.

The code works, but consider the following workflow:

User enters "John Doee" in first input

Collection: ['']

User clicks "add", and enters "Sally Something" in the second input

Collection: ['', '']

User realized they spelled "John Doe" (two e's) and fixes it

Collection: ['', '']

or consider an existing list being edited:

Collection: ['John Doe', 'Sally Something', 'Austin Powers']
User removes "Austin Powers"

Collection: ['John Doe', 'Sally Something']

User updates "John Doe" to "Dr. John Doe"

Collection: ['John Doe', 'Sally Something']

I'm using collection.add() and collection.remove() to trigger the view to update itself, but the collection doesn't always reflect the data on the screen. Is that okay?

Additionally, I did try adding a hook for blur which grabbed every value and reset the collection, but it seemed too brute force. Hence, my doubt about this approach.

Any thoughts or other approaches?

Answering my own question. A View's Model represents "state", not data. This is a subtle distinction that wasn't apparent to me at first. The Collection should match the View, blank values and all.

When it comes time to save the list, the Collection's toJSON method can be overridden to omit any blank models.

I updated my original jsfiddle to show values being saved on blur.

var Person = Backbone.Model.extend();

var People = Backbone.Collection.extend({
  model: Person,
  toJSON: function() { /*filter out empty models*/}
});

var PersonView = Marionette.ItemView.extend({
  ...
  events: {
    'blur input': 'onChange', //calls model.set('name', input value)
  },
  triggers: {
    'click .del-name': 'remove:form:row' //calls collection.remove(...)
  },
  ...
});

var FormView = Marionette.CompositeView.extend({
  events: {
    'click @ui.addBtn': 'addFormRow' //calls collection.add({name: ''})
  },
  collectionEvents: {
    add: 'onRowAdded'  //rerender first row to show delete button
  },
  ...
});


Simple hashing and salting algorithm I have devised what I feel to be a simple hash/salting algo for Java, and am looking for some feedback from people more skilled than myself. Please find below my code;

import java.security.MessageDigest;
import java.security.NoSuchAlgorithmException;
import java.security.NoSuchProviderException;
import java.security.SecureRandom;

public class PassSecureSuite
{
    /**
     * Method:          RecoverPass
     * Function:        This class contains methods for authenticating the user against the database and creating
     *                  a new user with a secure password.
     */
    public static String verifyIput(String submittedPass, String salt)
    {
        MessageDigest soMd = null;
        try
        {
            soMd = MessageDigest.getInstance("SHA-256");
        }
        catch (NoSuchAlgorithmException e)
        {
            e.printStackTrace();
        }

        soMd.update(salt.getBytes());
        byte[] abytesToString = soMd.digest(submittedPass.getBytes());
        StringBuilder buildThis = new StringBuilder();

        for(int i=0; i<abytesToString.length; i++)
        {
            buildThis.append(Integer.toString((abytesToString[i] & 0xff) + 0x100, 16).substring(1));
        }
        String passToVerify = buildThis.toString();
        return passToVerify;
    }
    /**
     * Method:          getSalt
     * Function:        Generate a secure, randomly-generated string to add extra security to the user password
     */
    public static String getSalt()
    {
        SecureRandom sosRand = null;
        try
        {
            sosRand = SecureRandom.getInstance("SHA1PRNG", "SUN");
        }
        catch (NoSuchAlgorithmException e)
        {
            e.printStackTrace();
        }
        catch (NoSuchProviderException e)
        {
            e.printStackTrace();
        }

        byte[] baniceAndSalty = new byte[16];
        sosRand.nextBytes(baniceAndSalty);
        return baniceAndSalty.toString();
    }
    /**
     * Method:          agenerateSecurePass
     * Function:        Generates a password string based on a byte feed
     */
    public static String sgenSecurePass(String password, String salt)
    {
        String ageneratedPass = null;
        MessageDigest digest = null;
        try
        {
            digest = MessageDigest.getInstance("SHA-256");
        }
        catch (NoSuchAlgorithmException nsae)
        {
            nsae.printStackTrace();
        }

        digest.update(salt.getBytes());
        byte[] babytes = digest.digest(password.getBytes());

        StringBuilder obuildThis = new StringBuilder();

        for(int i=0; i < babytes.length; i++)
        {
            obuildThis.append(Integer.toString((babytes[i] & 0xff) + 0x100, 16).substring(1));
        }
        ageneratedPass = obuildThis.toString();
        return ageneratedPass;
    }
}

I am looking to make this as secure as possible without using other people's APIs, as I enjoy having total control over my security within applications.

The above example appears fairly robust to me, as I have been unable to break the hash/salt on any application I have built, but I feel that I can improve this, I am just not sure how.

I spotted the following problems:

Exception handling When you catch an exception, you just print its stack trace but then keep going as if nothing happened, forcing another exception a few lines down the code. You should change this
Byte conversion: Your getSalt method converts the byte array holding the salt into a string, the other 2 methods try to convert that string back into a byte array. Firstly, I think that you are doing that conversion wrong. But event if that was not the case, you should not do that. A byte array should be kept as a byte array. Unneccesary conversion can always cause errors. If you absolutely need a string, use Base64 encoding.
You should not simply use SHA256(Salt | password). Use HMAC-SHA256 instead.
Actually, SHA-256 or HMAC-SHA256 are not good functions to hash a password. You should use a dedicated password hashing KDF, which are much better at resisting brute force attacks. Good examples are scrypt, bcrypt, PKBKDF2, Argon2.
I still don't get the strange way you are converting your password hash into a string at the end. Use Base64 instead.
Your essential code (the password hashing) is duplicated in two methods. Don't do that.


Format a phone number function Here is a function that formats a phone number and adds a country code prefix if it is not included in it.

So, as an in input value it could be, for example, '23581010' or '4923581010' and in output it would become via the formatNumber function '(02) 358 10 10'.

Is the positioning of the let assignment ok in this code ? If something else feels not ok can you tell me ?

import { formatNumber } from 'libphonenumber-js';

export const formatGermanPhoneNumber = phoneNumber => {
  if (!phoneNumber) {
    return '';
  }

  const phoneNumberString = `${phoneNumber}`;

  let prefix = '';
  if (!phoneNumberString.startsWith('49')) {
    prefix = '49';
  }

  return formatNumber(`+${prefix}${phoneNumberString}`, 'National');
};

I don't know of any coding standards or other reasons why you should have to move it higher. Generally it's just personal preference and whichever is more readable.

As for the fact that you'd need to create a variable unnecessarily in some cases, creating a variable like this uses very little memory so it shouldn't be an issue unless the function is being looped over thousands of times.

You could also consider another option, which is to use a ternary rather than mutating a variable:
let prefix = phoneNumberString.startsWith('49') ? '49' : ''


Part 2: Create or update record via HTTP request This script was previously reviewed here: Part 1: Create or update record via HTTP request
I've made the changes that were suggested by @Reinderien.
Now that the code has been significantly refactored, are there any other improvements that could be made?

Description:
I have an external system that sends an HTTP request to a Jython script (in IBM's Maximo Asset Management platform).
The Jython 2.7.0 script does this:

Accepts an HTTP request: http://server:port/maximo/oslc/script/CREATEWO?_lid=wilson&_lpwd=wilson&f_wonum=LWO0382&f_description=LEGACY WO&f_classstructureid=1666&f_status=APPR&f_wopriority=1&f_assetnum=LA1234&f_worktype=CM
Loops through parameters:

Searches for parameters that are prefixed with f_ ('f' is for field-value)
Puts the parameters in a list
Removes the prefix from the list values (so that the parameter names match the database field names).

Updates or creates records via the parameters in the list:

If there is an existing record in the system with the same work order number, then the script updates the exiting record with the parameter values from the list.
If there isn't an existing record, then a new record is created (again, from the parameter values from the list).

Finishes by returning a message to the external system (message: updated, created, or other (aka an error)).

from psdi.mbo import SqlFormat
from psdi.server import MXServer
from psdi.mbo import MboSet

IGNORE_RULES = 2L

params = [
    param for param in request.getQueryParams()
    if param.startswith('f_')
]
paramdict = {
    p[2:]: request.getQueryParam(p)
    for p in params
}
resp=''
woset = MXServer.getMXServer().getMboSet("workorder",request.getUserInfo())

try:
    #Prevents SQL injection
    sqf = SqlFormat("wonum=:1")
    sqf.setObject(1,"WORKORDER","WONUM",request.getQueryParam("f_wonum"))
    woset.setWhere(sqf.format())
    woset.reset()
    woMbo = woset.moveFirst()

    if woMbo is None:
        woMbo=woset.add()
        verb = 'Created'
    else:
        verb = 'Updated'

    for k,v in paramdict.items():
        woMbo.setValue(k,v,2L)
    resp = verb + ' workorder ' + request.getQueryParam("f_wonum")
    woset.save()
    woset.clear()
    woset.close()
finally:
    woset.close()

responseBody = resp

You haven't used this:
IGNORE_RULES

in your call to setValue.
Also, you have a double call to close; delete the first one.
resp does not benefit from being pre-initialized to an empty string; you can delete that line.
The only other thing I see is that this:
params = [
    param for param in request.getQueryParams()
    if param.startswith('f_')
]
paramdict = {
    p[2:]: request.getQueryParam(p)
    for p in params
}

can be condensed to only the dictionary comprehension:
paramdict = {
    p[2:]: request.getQueryParam(p)
    for p in request.getQueryParams()
    if p.startswith('f_')
}


Three "alias-functions" in my .bashrc file These functions work well for me. Can they be improved or made more standard in some way?

Each function has a documentation header block, which contains a description of what it does, along with example usage and reference links where applicable.

:<<COMMENT
    Alias function for recursive deletion, with are-you-sure prompt.
    Example call:
        srf /home/myusername/django_files/rest_tutorial/rest_venv/

    Short description: Stored in SRF_DESC

    With the following setting, this is *not* added to the history:
        export HISTIGNORE="*rm -r*:srf *"
    - http://superuser.com/questions/232885/can-you-share-wisdom-on-using-histignore-in-bash

    See:
    - y/n prompt: http://stackoverflow.com/a/3232082/2736496
    - Alias w/param: http://stackoverflow.com/a/7131683/2736496
COMMENT
#SRF_DESC: For "aliaf" command (with an 'F'). Must end with a newline.
SRF_DESC="srf [path]: Recursive deletion, with y/n prompt\n"
srf()  {
    #Actual line-breaks required in order to expand the variable.
    #- http://stackoverflow.com/a/4296147/2736496
    read -r -p "About to
sudo rm -rf $1
Are you sure? [y/N] " response
    response=${response,,}    # tolower
    if [[ $response =~ ^(yes|y)$ ]]
    then
        echo -e "sudo rm -rf $1"
        sudo rm -rf "$1";
    else
        echo "Cancelled."
    fi
}

:<<COMMENT
    Delete item from history based on its line number. No prompt.

    Short description: Stored in HX_DESC

    Examples
        hx 112
        hx 3

    See:
    - http://unix.stackexchange.com/questions/57924/how-to-delete-commands-in-history-matching-a-given-string
COMMENT
#HX_DESC: For "aliaf" command (with an 'F'). Must end with a newline.
HX_DESC="hx [linenum]: Delete history item at line number\n"
hx()  {
    history -d "$1";
}

:<<COMMENT
    Deletes all lines from the history that match a search string, with a
    prompt. The history file is then reloaded into memory.

    Short description: Stored in HXF_DESC

    Examples
        hxf "rm -rf"
        hxf ^source

    See:
    - http://unix.stackexchange.com/questions/57924/how-to-delete-commands-in-history-matching-a-given-string
COMMENT
#HXF_DESC: For "aliaf" command (with an 'F'). Must end with a newline.
HXF_DESC="hxf [searchterm]: Delete all history items matching search term, with y/n prompt\n"
hxf()  {
    read -r -p "About to delete all items from history that match \"$1\". Are you sure? [y/N] " response
    response=${response,,}    # tolower
    if [[ $response =~ ^(yes|y)$ ]]
    then
        #Delete all matched items from the file, and duplicate it to a temp
        #location.
        echo -e "grep -v \"$1\" \"$HISTFILE\" > /tmp/history"
        grep -v "$1" "$HISTFILE" > /tmp/history

        #Clear all items in the current sessions history (in memory). This
        #empties out $HISTFILE.
        echo "history -c"
        history -c

        #Overwrite the actual history file with the temp one.
        echo -e "mv /tmp/history \"$HISTFILE\""
        mv /tmp/history "$HISTFILE"

        #Now reload it.
        echo -e "history -r \"$HISTFILE\""
        history -r "$HISTFILE"     #Alternative: exec bash
    else
        echo "Cancelled."
    fi
}

You never validate the contents/existence of $1

for instance: grep -v "" "$HISTFILE" returns nothing, so you're deleting your entire history.

You might want to use set -x, set +x instead of echo-ing the commands about to be done.


F# code to check whether there are three same digits within a given number I came across this problem by accident and gave it a go. I think my solution works as it has passed all the unit tests I wrote. Given I just started learning F# a couple of weeks I would love to have some feedback on the code regarding both its accuracy and its quality.

To be clear about the problem, given an integer number, check whether the number has at least 3 same digits in it e.g. 1222, 12222, 222, 123123123 are both counted as yes.

module Miscellaneous =
    let (|ThreeSame|_|) = function
        | (l : int list) when l.Length > 20 -> Some()
        | x :: y :: z :: _ when x = y && y = z -> Some()
        | _ -> None

    ///
    /// Given any integer convert its digits into a list
    ///
    ///<param name="number">The integer</param>
    ///<returns>
    ///A int list
    ///</returns>
    let convertNumberToList (number: bigint) =
        let numberString = string number
        numberString.ToCharArray()
        |> Array.map (string >> int) //use composition to replace (fun x -> int (string x))
        |> Array.toList

type NumberCheck() =
        ///
        /// A helper function to check whether a list of numbers contains three or more
        /// same numbers
        ///
        ///<param name="l">The list to be checked</param>
        ///<param name="sorted">The flag to indicate whether the given list is sorted or not</param>
        ///<returns>
        /// True if there are at least 3 same numbers; False otherwise
        ///</returns>
        static member private TripleNumberHelper(l : int list, ?sorted) =
            if l.Length < 3 then false
            else
                if defaultArg sorted false then
                    match l with
                    | Miscellaneous.ThreeSame -> true
                    | _ -> NumberCheck.TripleNumberHelper(l.Tail, true)
                else
                    let sortedList = l |> List.sort
                    match sortedList with
                    | Miscellaneous.ThreeSame -> true
                    | _ -> NumberCheck.TripleNumberHelper(sortedList.Tail, true)

        ///
        /// Test whether a number has 3 or more same digits in it
        ///
        ///<param name="number">The number to be checked</param>
        ///<returns>
        ///Ture of False
        ///</returns>
        static member TripleNumber(number : bigint) =
            let l = Miscellaneous.convertNumberToList number
            NumberCheck.TripleNumberHelper(l)

Followed upon @hocho's comment and agree that using groupBy is much better as it can generalise the solution to check any number of same digit rather than limited to just 3 from my initial implementation. I have also simplified the helper method which converts a number into a collection; returning a sequence is enough as there is no need to return an array.

let convertNumberToSeq (number: bigint) =
        let numberString = string number
        numberString.ToCharArray()
        |> Seq.map (string >> int)

///<summary>
/// Check whether a given number has at least n same digits
///</summary>
///<param name="number">The given number</param>
///<param name="n">How many same digits to be checked</param>
///<returns>
///True if there are n same digits otherwise False
///</returns>
let SameDigitCheck (number : bigint) (n : int) =
    if n < 2 then failwith "n must be at least 2"
    let digits = convertNumberToSeq number
    let q = query { for digit in digits do groupBy digit into g; select g }
    q |> Seq.exists (fun x -> Seq.length x >= n)


Assigning width and height to specify image dimension using jQuery After testing my site against GTMetrix, I get warnings on not specifying image dimensions. I am working on a WordPress site. Here is my solution:

My first solution was:

    // Specify image dimensions
    $('img').each(function() {
        var findImgWidth = $(this).width();
        var findImgHeight = $(this).height();

        $(this).attr('width', findImgWidth);
        $(this).attr('height', findImgHeight);
    });

Output: This method does fix the "Specify Image Dimensions" but it also removed some of my images on the website.

The second solution is:

// Make sure img have been loaded
$().ready(function() {
    imageSize();
});
// Assign width/height to img
function imageSize() {
    // Specify image dimensions
    $('img').each(function() {
        var findImgWidth = $(this).width();
        var findImgHeight = $(this).height();

        $(this).attr('width', findImgWidth);
        $(this).attr('height', findImgHeight);
    });
}

Output: This solution seems to work. My images does not disappear, but I would like someone to help me go over the code to see if it's an efficient way to write.

The reason GTMetrix complains is that the browser can render the page faster if it knows the size of the image.

Setting the size after the browser has rendered is not going to make your page faster, it will only make the warning go away and fractionally make your page load slower.

This means you will have to declare the height and width of the image on the server side, or ignore the warning of GTMetrix.


Custom dropdown selector I'm working on improving my knowledge of OOP in JS. I just created this custom dropdown selector. It's working nicely, but I'm not super happy about the Filter constructor. I would like to make all that code more readable. It's basically just defining some attributes and setting some event listeners.

function Filter(element) {
  this.element = element;
  this.dropdown = this.element.querySelector('.dropdown');

  var link = this.element.querySelector('.link');
  var options = this.dropdown.querySelectorAll('a');

  var selectOption = function(event) {
    event.preventDefault();

    link.textContent = event.target.textContent;
    this.toggle();
  }

  for (var i = 0; i < options.length; i++) {
    options[i].addEventListener('click', selectOption.bind(this));
  }

  link.addEventListener('click', this.toggle.bind(this));
};

Filter.prototype.isExpanded = function() {
  return !this.dropdown.classList.contains('hidden');
}

Filter.prototype.collapse = function() {
  if (this.isExpanded()) {
    this.dropdown.classList.add('hidden');
  }
}

Filter.prototype.toggle = function() {
  if (!this.isExpanded()) {
    var filterExpandedEvent = new CustomEvent('filterExpanded', {detail: this});
    document.dispatchEvent(filterExpandedEvent);
  }

  this.dropdown.classList.toggle('hidden');
}

How you would you simplify that constructor? Any ideas?

Public versus private fields
In Java, it is a custom to make all your fields private, unless they must be explicitly accessed by other sources. I believe this custom also applies to JavaScript.
Instead of having the element be a public (this) field, why don't you make it a private (var) field? There doesn't really seem to be another place you use it.
And, if there is another place that on using it in, why don't you create a getter? This would be a function that returns the field, and it might be named something like getElement.
This is up to you.
Defining the functions with Filter.prototype
You did exactly this, and that is perfect. Keep that up, it is a great practice.
querySelector versus getElementsByClassName
This is a matter of efficiency versus readability - whatever you chose to have.
It would be more efficient to use getElementsByClassName in place of querySelector in the link field, as the getElementsByClassName function doesn't have to do checks on the argument provided(whether or not the argument is a class, an id, a tag name, or a mixture); it knows it's looking for a one class and one class only.
It is slightly more readable having querySelector, as the syntax for it's arguments is the same syntax for CSS; it's a "universal" syntax.
Indentation
With only 2 spaces for an indent, your code looks fairly cramped. I recommend using tabs or 4 spaces as an indent.


Simplifying a Resizing Method I'm making a simple container class for fun and education, but my rebuilding/resizing method seems rather inefficient.  Is there an easier way to do this?

// If FromFront is true, cells should be added or
// subtracted from the front of the array rather than the back.
void Rebuild(std::size_t NewSize, bool FromFront = false)
{
    const std::size_t OldSize = Size;
    Size = NewSize; // Size is used in other methods.

    Datatype* TemporaryStorage = new Datatype[OldSize]; // Allocate space for the values to be preserved while a new main array is made.

    for (std::size_t Index = 0; Index < OldSize; Index++) TemporaryStorage[Index] = BasePointer[Index]; // Copy values over to the temporary array.

    delete[] BasePointer; // Delete the main array...
    BasePointer = new Datatype[NewSize]; // ...And rebuild it with the appropriate size.

    for (std::size_t Iteration = 0; (Iteration < OldSize) && (Iteration < NewSize); Iteration++)
    {
        std::size_t BasePointerIndex = Iteration;
        std::size_t TemporaryStorageIndex = Iteration;

        if (FromFront) // We need to take special measures to give the indices offsets.
        {
            if (NewSize > OldSize) BasePointerIndex += (NewSize - OldSize);
            else TemporaryStorageIndex += (OldSize - NewSize);
        }

        BasePointer[BasePointerIndex] = TemporaryStorage[TemporaryStorageIndex]; // Copy values from the temporary array to the new main array.
    }

    delete[] TemporaryStorage; // Finally, delete the temporary storage array.
}

void Rebuild(std::size_t NewSize, bool FromFront = false)
{
    const std::size_t OldSize = Size;

    Datatype* NewStorage = new Datatype[NewSize];

Rather then creating a temporary array and copying the existing data into it, just create the new array and copy into that.

    int CopyLength = std::min(OldSize, NewSize);

We start by determining how many elements we will actually copy

    if( FromFront )
    {
        std::copy(BasePointer + OldSize - CopyLength, BasePointer + OldSize,
                  NewStorage + NewSize - CopyLength);
    }
    else
    {
        std::copy(BasePointer, BasePointer + CopyLength, NewStorage);
    }

I use std::copy to avoid having to write a copying for loop myself.

    delete[] BasePointer; // Delete the main array...
    BasePointer = NewStorage;
    Size = NewSize; // Size is used in other methods.

delete the current array and replace it with my new array

}


Using method which return a String or creating wrapper for String Which approach is better and why?

First approach (inner class for wrapping):

public class OOP {
    private SomeService someService;

    private final class OrganizationPrimaryKeyString implements Serializable {
        private static final long serialVersionUID = -256615521081939636L;
        private final String pkString

        public OrganizationPrimaryKeyString(final OrganizationEnum organizationEnum) {
            final PK pk = someService.getValue(organizationEnum).getPk();

            if (pk == null) {
                throw new IllegalArgumentException("Can't be null");
            }

            pkString = pk.getLongValueAsString();
        }

        @Override
        public String toString() {
            return pkString;
        }
    }

    public List<Information> getInformation(final OrganizationEnum organizationEnum) {
        final OrganizationPrimaryKeyString organization = new OrganizationPrimaryKeyString(organizationEnum);

        try {
            final SessionContext ctx = createAndSetupLocalSessionContext(organization);

            (...)

            return (...);
        } catch (final FactoryException e) {
            throw new SystemException(e.getMessage(), e);
        } finally {
            closeLocalSessionContext();
        }
    }

    private SessionContext createAndSetupLocalSessionContext(final OrganizationPrimaryKeyString organization) {
        (...)
        sessionContext.setAttribute("organization", salesOrganization.toString());

        return sessionContext;
    }
}

Second approach (class method):

public class OOP {
    private SomeService someService;

    public List<Information> getInformation(final OrganizationEnum organizationEnum) {
        try {
            final SessionContext ctx = createAndSetupLocalSessionContext(organizationEnum);

            (...)

            return (...);
        } catch (final FactoryException e) {
        throw new SystemException(e.getMessage(), e);
        } finally {
            closeLocalSessionContext();
        }
    }

    private SessionContext createAndSetupLocalSessionContext(final OrganizationEnum organizationEnum) {
        (...)
        sessionContext.setAttribute("organization", getPkString(organizationEnum));

        return sessionContext;
    }

    private String getPkString(final OrganizationEnum organizationEnum) {
        final PK pk = someService.getValue(organizationEnum).getPk();

        if (pk == null) {
            throw new IllegalArgumentException("Can't be null");
        }

        return pk.getLongValueAsString();
    }
}

I think the first approach is the best; it uses more design-patterns:

You throw an IllegalArgumentExcaption but pk is not an argument of the method.

From JavaDoc:

  Thrown to indicate that a method has been passed an illegal or inappropriate argument.

You throw an IllegalArgumentExcaption because of a null. It should be a NullPointerException - it's more meaningful. Objects.requreNonNull() does it for you automatically. A NullPointerException would be automatically thrown in 2 lines later, so no need for a check except you would like to add a custom message to the NullPointerException. That would be a good idea.
The Class who has the .getPk()-method can have a null as a PrimaryKey; it looks like a design-flaw? Anyway, if this instance who has the .getPk()-method can have an illegal state, then an IllegalStateException should be thrown.
The toString() implementation is good.
OrganizationPrimaryKeyString is a good read-only object. Read-only objects are good for memory because you do not need to have two instances of the same organisation. But if you call getInformation() one million times for ibm, then every call to getInformation() returns one million equal ibm-objects that are not the same but require its own memory in the heap! A HashMap may help caching, so every call for ibm can return one of the same object.
The naming is good and meaningful. getInformation() should be named getInformations() and OrganizationPrimaryKeyString should only be named OrganizationPrimaryKey.
OrganizationPrimaryKey should be static because YDKJ: A subclass that is not static automatically has the wrapped-class as constructor-argument who internally is cached as a field of OrganizationPrimaryKey. Trust me, every Constructor of OrganizationPrimaryKey caches the OOP-Instance.


Custom Log4J Appender I have written a Custom Log4J Appender which is responsible for sending the log message to a third party service asynchronously. It is expected to be called every millisecond.

I ran FindBugs on this code, it didn't report anything unusual. But before releasing this code to production, how can I know if this code has any issues with it?

package com;

import java.util.ArrayList;
import java.util.Iterator;

import org.apache.log4j.AppenderSkeleton;
import org.apache.log4j.spi.LoggingEvent;

public class MyAppender extends AppenderSkeleton {

    public MyAppender() {

    }

    public void append(LoggingEvent event) {
        ArrayList<LoggingEvent> buffer = new ArrayList<LoggingEvent>();
        buffer.add(event);
        flushBuffer(buffer);
    }

    public void flushBuffer(ArrayList<LoggingEvent> buffer) {
        for (Iterator<LoggingEvent> i = buffer.iterator(); i.hasNext();) {
            try {
                LoggingEvent logEvent = (LoggingEvent) i.next();
                String messageRecievied = (String) logEvent.getMessage();
                //System.out.println(messageRecievied);

            } catch (Exception e) {

            }

        }

    }

    @Override
    public void close() {

    }

    @Override
    public boolean requiresLayout() {

        return false;
    }

}

First, I don't think the close() method is following the contract, from the javadoc:

  It is a programming error to append to a closed appender.

Therefore, your close() method should be implemented like this:

@Override
public void close() {
    this.closed = true;
}

Note that closed is a protected field in AppenderSkeleton.

Then, change append to something like (add the @Override as well):

@Override
public void append(LoggingEvent event) {
    if (!this.closed) {
        ArrayList<LoggingEvent> buffer = new ArrayList<LoggingEvent>();
        buffer.add(event);
        flushBuffer(buffer);
    } else {
        // It is a programming error to append to a closed appender.
        throw new RuntimeException("Attempting to append to a closed Appender.");
    }
}

Next, does flushBuffer need to be public? It should probably be private.

Finally—and this is more of a nitpick—you don't need to declare the default constructor since it's empty and the compiler will create one for you. It'll make your code a little cleaner as well.


Determining if the length of LinkedList is even or odd Today, I tackled this coding question:
Given a singly-linked list, check whether its length is even or odd in a single pass. An Empty list has 0 nodes which makes the number of nodes in it even.

Would the time complexity of my code be O(n)? Do you guys see any improvements that can be made?

public Boolean isListEven(ListNode head) {

    if(head == null){
        return true;
    }

    if(head.next.next == null){
        return false;
    }

    int count = 1;
    while(head.next != null){
        if(head.next.next != null){
            count = count + 2;
            head = head.next.next;
        }

        count = count + 1;
        head = head.next;
    }

    return (count % 2 == 0);
}

The method signature is a bit odd, returning a Boolean object instead of a primitive boolean.  A clearer name would be isListLengthEven().  The function could be static, since it does not rely on any object state.

If the list has one node, then your code will crash on if(head.next.next == null).  In general, you should avoid cluttering your code with unnecessary special cases.

You don't need to keep track of count.  This solution, like yours, is O(n), but it is much simpler:

public static boolean isListLengthEven(ListNode head) {
    for (; head != null; head = head.next.next) {
        if (head.next == null) return false;
    }
    return true;
}

The trick is to observe the loop invariant: each iteration through the loop, it advances head by two nodes, so that the picture at the top of the loop is always equivalent to the original.  If it can't advance by two, then the list has an odd length.


String constructible from short list of words? AtCoder ABC049C - Daydream I am practicing python, and in order to brush up my skill, I am trying the programming contest site AtCoder's past problems.

Daydream

Basically, you are given an input string and have to output YES if the string is constructed with choices of 4 different words ('dream', 'dreamer', 'erase', 'eraser'), otherwise NO.

I implemented as following.

I am aware that there are much better solutions out there, but mine also should work. However, even though first 3 cases were accepted as answers, but other 15 cases fails, and I cannot figure out why my code could fail.

ddee = ('dream', 'dreamer', 'erase', 'eraser')

def match_and_move(s, t, i):
    """checks if string s starts with any of the 4 choices starting from index i"""
    if s == t:
        return True
    for word in ddee:
        if s.startswith(word, i):
            if match_and_move(s, t+word, i+len(word)):
                return True
    return False

S = input()
T = ''
print('YES' if match_and_move(S, T, 0) else 'NO')

What am I doing wrong here?

Any assistance would be appreciated.

Thank you in advanced.

EDIT :

Because this is programming contest site, I was not provided with the input file for the all 18 tests.
I just know that first 3 tests were marked 'AC - Accepted' and other 15 tests were marked 'RE - Runtime Error' without actual error being displayed.
I performed test on my own based on the requirements (as well as the example provided by them on the question page).
At least I know that this code is working as they intended even if it is partially.

EDIT 2 / Answer :

Since this question is closed, I cannot place a separate answer.

Further testing my code, I was looking further in to the constraints.

1≦|S|≦10^5
s consists of lowercase English letters.

I prepared a string that is 7000 characters long 'dreamerdreamer...dreamer' which should return 'YES' as this is just a repeat of 'dreamer' which is one of the 4 phrases.

And I received following error.

RecursionError: maximum recursion depth exceeded in comparison

Which is self explanatory. (and is also something @VincentRG has suggested)

import sys
sys.setrecursionlimit(10**6)

By adding above, on my PC this program is working with this large input. 5 more problems are solved on the website, and 10 of the website's test were returning Memory Limit Exception, which also make sense.

It seems to be fine. What are the failing cases' inputs? Following your link I only see 3 cases which should indeed work.

However, I see there's a limitation of time and memory usage at the beginning, and that S max length is 100000. Your code might for example exceed maximum recursion depth.
I think a simpler solution would be to use regex. For example (and I added a long sample):

import re
import random

regexPattern = "^(dream(er)?|eraser?)*$"
sampleList = [
    "erasedream",
    "dreameraser",
    "dreamerer"]

# just add a looooooong sample
longSample = ""
mandatoryWords = ["dream", "dreamer", "erase", "eraser"]
while True:
    addWord = mandatoryWords[random.randint(0, 3)]

    if len(longSample) + len(addWord) > 100000:
        break
    else:
        longSample += addWord

sampleList.append(longSample)

for sample in sampleList:
    match = re.fullmatch(regexPattern, sample)

    if match != None:
        print("YES")
    else:
        print("NO")

outputs

YES
YES
NO
YES