Python program to remove invalid parentheses This is a Leetcode problem -

  Remove the minimum number of invalid parentheses in order to make the
  input string valid. Return all possible results.

  Note -

  The input string may contain letters other than the parentheses (
  and ).

Here is my solution to this challenge -

def remove_invalid_parentheses(s):

    removed = 0
    results = {s}
    count = {"(": 0, ")": 0}
    for i, c in enumerate(s):
        if c == ")" and count["("] == count[")"]:
            new_results = set()
            while results:
                result = results.pop()
                for j in range(i - removed + 1):
                    if result[j] == ")":
                        new_results.add(result[:j] + result[j + 1:])
            results = new_results
            removed += 1
        else:
            if c in count:
                count[c] += 1
    count = {"(": 0, ")": 0}
    i = len(s)
    ll = len(s) - removed
    for ii in range(ll - 1, -1, -1):
        i-=1
        c = s[i]
        if c == "(" and count["("] == count[")"]:
            new_results = set()
            while results:
                result = results.pop()
                for j in range(ii, ll):
                    if result[j] == "(":
                        new_results.add(result[:j] + result[j + 1:])
            results = new_results
            ll -= 1
        else:
            if c in count:
                count[c] += 1
    return list(results)

Explanation -

Scan from left to right, make sure count["("] >= count[")"].
Then scan from right to left, make sure count["("] <= count[")"].

Here are some inputs/outputs -

print(remove_invalid_parentheses("()())()"))
>>> ["()()()", "(())()"]

print(remove_invalid_parentheses("(a)())()"))
>>> ["(a)()()", "(a())()"]

print(remove_invalid_parentheses(")("))
>>> [""]

Here are the times taken for each output -

%timeit remove_invalid_parentheses("()())()")
6.54 µs ± 176 ns per loop (mean ± std. dev. of 7 runs, 100000 loops each)

%timeit remove_invalid_parentheses("(a)())()")
8.43 µs ± 1.29 µs per loop (mean ± std. dev. of 7 runs, 100000 loops each)

%timeit remove_invalid_parentheses(")(")
3.69 µs ± 67.3 ns per loop (mean ± std. dev. of 7 runs, 100000 loops each)

So, I would like to know whether I could make this program shorter and more efficient.

Any help would be highly appreciated.

Test, consistent behavior & data structure

Trying to add tests for the code based on the example of input/output provided led to the following result: the behavior is not consistent : in particular the order of the elements in the returned list is not always the same.

The root cause for this is that we are using sets in the logic only to convert to list at the end.

A solution would be to return sorted(results) at the end. An alternative would be to just return a set instead of trying to convert to list.

Then, we get the following automated tests:

print(remove_invalid_parentheses("()())()") == {"(())()", "()()()"})
print(remove_invalid_parentheses("(a)())()") == {"(a())()", "(a)()()"})
print(remove_invalid_parentheses(")(") == {""})

It would make sense to try to rewrite it as unit-tests using a proper unit-test framework but I will stop here as this is enough for me to keep working.

More explicit loop

I tend to find loops based on for ii in range(ll - 1, -1, -1) very error-prone/hard to understand because of the number of tweaks on both sides of the range. I'd suggest a much more explicit alternative: for ii in reversed(range(ll)):.

Similar indices

As ii will go over ]ll, 0] which is ]len(s) - removed, 0], i will go over ]len(s), removed].

The difference between the indices is always removed, we have ii + removed == i: we can get rid of the i variable. We can take this chance to rename ii into i.

At this stage, we have:

def remove_invalid_parentheses(s):
    removed = 0
    results = {s}
    count = {"(": 0, ")": 0}
    for i, c in enumerate(s):
        if c == ")" and count["("] == count[")"]:
            new_results = set()
            while results:
                result = results.pop()
                for j in range(i - removed + 1):
                    if result[j] == ")":
                        new_results.add(result[:j] + result[j + 1:])
            results = new_results
            removed += 1
        elif c in count:
                count[c] += 1
    count = {"(": 0, ")": 0}
    ll = len(s) - removed
    for i in reversed(range(ll)):
        c = s[i + removed]
        if c == "(" and count["("] == count[")"]:
            new_results = set()
            while results:
                result = results.pop()
                for j in range(i, ll):
                    if result[j] == "(":
                        new_results.add(result[:j] + result[j + 1:])
            results = new_results
            ll -= 1
        elif c in count:
                count[c] += 1
    return results

print(remove_invalid_parentheses("()())()") == {"(())()", "()()()"})
print(remove_invalid_parentheses("()())))()") == {"(())()", "()()()"})
print(remove_invalid_parentheses("(a)())()") == {"(a())()", "(a)()()"})
print(remove_invalid_parentheses(")(") == {""})

(I took this chance to add a new test case)

More explicit loop again

We want to iterate in reversed order over the beginning of the s string. Here again, we can try to make things more explicit and remove some computation as we go:

    for i, c in enumerate(reversed(s[removed:])):
        if c == "(" and count["("] == count[")"]:
            new_results = set()
            while results:
                result = results.pop()
                for j in range(len(s) - i - removed - 1, ll):
                    if result[j] == "(":
                        new_results.add(result[:j] + result[j + 1:])
            results = new_results
            ll -= 1
        elif c in count:
                count[c] += 1

In order to test this more throughly, I took this chance to add the following test case:

print(remove_invalid_parentheses("((((((((()(") == {"()"})

More beautiful loops

Instead of using a combination of while and pop to iterate over the elements of a set, you can just use for:

        for result in results:

Work in progress: I have to go. I may continue in the future.


Override values for overlapping objects between two scala lists Let's say I have a bunch of Widget, where a Widget is

class Widget (sku: Int, label: String, name: String, description: String)

And I have 2 sources of Widgets:

List[Widget] wInventory = getWidgetsInInventory()
List[Widget] wCatalog = getWidgetsFromCatalog()

I want to use the name & label from the Catalog to override any matching widget in Inventory. The match criteria is when sku == sku.
If the label is missing in the catalog, override with constant value of No Catalog Label instead of leaving label as is.
The wCatalog list may be empty.  There may be no intersections at all, or both lists can contain the same set of sku's.

First draft of the code below. There must be a way to avoid running find twice, but because find returns Optional[Widget], it started looking messy with lots getOrElse.  What's the cleanest way to do this?

case class MyWidget (sku: Int, label: String, name: String, description: String)

object HelloWorld {
  def main(args: Array[String]) {

    def setOverrideValues(inv : List[MyWidget], cat: List[MyWidget]) : List[MyWidget] = {
      inv.map (k => {
        val realName = cat.find( _.sku == k.sku).map(_.name)
        val realLabel = cat.find( _.sku == k.sku).map(_.label)

        k.copy(name = realName.getOrElse("No Catalog Label"), label=realLabel.getOrElse(k.label))
      })
    }

    val invItem1 = new MyWidget(sku=1, label="invLabel1", name="invName1", description="invDesc1");
    val invItem2 = new MyWidget(sku=2, label="invLabel2", name="invName2", description="invDesc2");
    val catItem1 = new MyWidget(sku=1, label="catLabel1", name="catName1", description="catDesc1");

    val wInventory = List(invItem1 , invItem2);
    val wCatalog = List(catItem1)

    val updated = setOverrideValues(wInventory, wCatalog)

    println("done")
  }
}

No need to run find twice, just run it once and store the result in a value of type Option[MyWidget].

You can then use Option#fold and pass in two functions: one to run when the option contains a matching widget, and another when no match was found.

def setOverrideValues(inv : List[MyWidget], cat: List[MyWidget]) : List[MyWidget] = {
  inv.map (k => {
    val wOpt = cat.find(_.sku == k.sku)

    wOpt.fold(ifEmpty = k.copy(name = "No Catalog Label")) {
      w => k.copy(name = w.name, label = w.label)
    }
  })
}

You could also use pattern matching

def setOverrideValues(inv : List[MyWidget], cat: List[MyWidget]) : List[MyWidget] = {
  for (k <- inv)
    yield cat.find(_.sku == k.sku) match {
      case Some(w) => k.copy(name = w.name, label = w.label)
      case _ => k.copy(name = "No Catalog Label")
    }
}

Both options would save you from repeated calls to find or getOrElse


Have I coded this small object grouping script Pythonically? I have a small script that takes a list of objects from an SQL table and groups them if the date-time on the object has less than 60 second difference with the last object.

It does this by converting the date-time to epoch and then subtracting the epoch of the previous object from the epoch of the current object. The code works but I feel as if it is too convoluted for such a simple task.

rows = cur.fetchall() #gets all the objects from table

source = []
list1 = []
old_epoch = 1404413062 #I use the same epoch time as the first object but this isn't very pythonic

for a in rows:
    date_time = a[12] #gets date_time

#strips date_time with regex to match the proper pattern format
#sample date_time from db "2014-07-07 11:10:18.867024-04"
    date_time = re.match(r'^.*?\.', date_time).group(0)
    date_time = date_time.replace(".", "")
    pattern = '%Y-%m-%d %H:%M:%S'

    #converts date_time to epoch
    epoch = int(time.mktime(time.strptime(date_time, pattern)))

    #had to add this small part to force the iterator to append the last chunk since no new epoch_time to compare and get difference as difference would be zero
    if a == rows[-1]:
        source.append(a[11])
        list1.append(source)

    else:
        if epoch - old_epoch < 60:
            source.append(a[11])

        else:
            list1.append(source)
            source = []
            source.append(a[11])

    old_epoch = epoch

EDIT: Updated this post for those asking what it does. It returns a list of categorized groups  based on the timestamp as displayed below when printing out list1. I replaced the actual objects with only their timestamp property for readability purposes. :

['2014-07-07 06:21:51.011377-04', '2014-07-07 06:21:51.347373-04', '2014-07-07 06:21:51.678615-04']
['2014-07-07 06:26:54.01491-04', '2014-07-07 06:26:54.347479-04', '2014-07-07 06:26:54.671736-04']
['2014-07-07 06:31:57.107409-04', '2014-07-07 06:31:57.437156-04', '2014-07-07 06:31:57.804104-04']
['2014-07-07 06:37:00.178693-04', '2014-07-07 06:37:00.509359-04', '2014-07-07 06:37:00.828529-04']
['2014-07-07 06:42:03.268083-04', '2014-07-07 06:42:03.594742-04', '2014-07-07 06:42:03.921035-04']
['2014-07-07 06:47:06.389844-04', '2014-07-07 06:47:06.717987-04', '2014-07-07 06:47:07.035479-04']
['2014-07-07 06:52:09.516735-04', '2014-07-07 06:52:09.846621-04', '2014-07-07 06:52:10.171093-04']
['2014-07-07 06:57:12.623216-04', '2014-07-07 06:57:12.952604-04', '2014-07-07 06:57:13.278989-04']

list1 is a bad variable name. What is it actually storing? I know that it is a list, but I don't know what it is being used for.

It looks like this code is a snippet from a bigger block of code, but at least in this context, list1 is never being read. You should make sure the value is used later or remove the variable.

a is also a bad name. A single letter variable name is rarely a good variable name.

Comments:

for a in rows:
    date_time = a[12] #gets date_time

#strips date_time with regex to match the proper pattern format
#sample date_time from db "2014-07-07 11:10:18.867024-04"
    date_time = re.match(r'^.*?\.', date_time).group(0)

Your comments should start at the same indentation level as the code. What you have now could make someone reading your code think there are two different blocks.

Most of your comments are saying what the code does. This is generally an indication that you need to make your code more descriptive. You can achieve this with better variable names and extracting chunks of code into well-named sub-functions. Comments are better suited at describing why something is being done.

if a == rows[-1]:
    source.append(a[11])
    list1.append(source)

else:
    if epoch - old_epoch < 60:
        source.append(a[11])

    else:
        list1.append(source)
        source = []
        source.append(a[11])

You don't need the extra line after the if/else blocks.

source = []
source.append(a[11])

is the same as

source = [a[11]]

pattern has a constant value, you can initialize it before the loop instead to resetting the value for every row.

if epoch - old_epoch < 60:

60 is a magic number. What does it mean? If it were to change, you would have to find all the places 60 appears and check if it means the same thing as this context. If it has a name, you can just change the value once.


echoing html tags in php I have the following code below I was wondering if there is a way to clean it up/improve it especially near the if statements. I am new to programming and this is what I came up with and its working fine I just think there might be a better way to accomplish it.

    <?php

require 'DB.php';

try {

    $stmt = $conn->prepare('SELECT * FROM equipment');
    $stmt->execute();
    } catch(PDOException $e){
        echo'ERROR: ' . $e->getMessage();
    }

echo "

Equipment Type
Unit Number
Last Updated
Current Hours
Scheduled PM

";

while($row = $stmt->fetch())
  {
  echo "
";
  echo "" . $row['equipType'] . "	";
  echo "" . $row['unitNumber'] . "	";
  echo "" . $row['lastUpdate'] . "	";

  if ($row['nextPM'] - $row['currentHours'] > 100) {

  echo "" . $row['currentHours'] . "	";
  echo "" . $row['nextPM'] . "	";

  } else if ($row['nextPM'] - $row['currentHours'] < 100 && $row['nextPM'] - $row['currentHours'] > 50){

  echo "" . $row['currentHours'] . "	";
  echo "" . $row['nextPM'] . "	";

  }

  else {
  echo "" . $row['currentHours'] . "	";
  echo "" . $row['nextPM'] . "	";
  }
  echo "
";
  }
echo "
";
?>

My own personal preference when outputting things like this is to put the portion where data is actually retrieved in a separate file and then use the alternate PHP structure syntax. While this probably isn't everyone's preferred coding style, in my opinion it makes it easier to structure HTML so that its readable among the PHP. From what I have seen, this appears to be relatively common in templates.

Just a quick once-over your file to show you what I mean:

<?php
require 'DB.php'

try {
    $stmt = $conn->query('SELECT * FROM equipment');
}
catch(PDOException $e){
    echo'ERROR: ' . $e->getMessage();
    die();
}

?>

    Equipment Type
    Unit Number
    Last Updated
    CurrentHours
    Scheduled PM

<?php while($row = $stmt->fetch()): ?>

    <?php echo htmlspecialchars($row['equipType']); ?>
    <?php echo htmlspecialchars($row['unitNumber']); ?>
    <?php echo htmlspecialchars($row['lastUpdate']); ?>
<?php if ($row['nextPM'] - $row['currentHours'] > 100): ?>
    <?php echo htmlspecialchars($row['currentHours']); ?>
    <?php echo htmlspecialchars($row['nextPM']); ?>
<?php elseif ($row['nextPM'] - $row['currentHours'] < 100 && $row['nextPM'] - $row['currentHours'] > 50): ?>
    <?php echo htmlspecialchars($row['currentHours']); ?>
    <?php echo htmlspecialchars($row['nextPM']); ?>
<?php else: ?>
    <?php echo htmlspecialchars($row['currentHours']); ?>
    <?php echo htmlspecialchars($row['nextPM']); ?>
<?php endif; ?>

<?php endwhile; ?>

Again, if I were doing this I would separate that top half into another file and have that file include your "template" which would output the actual HTML. You could also use a template engine like Savant3 to make the separation a little cleaner and the templating better.

Demonstration with savant3:

Your main file (index.php perhaps?):

<?php
require 'DB.php'
require_once 'Savant3.php'
$tpl = new Savant3();

try {

    $stmt = $conn->query('SELECT * FROM equipment');
    $tpl->data = $stmt;
}
catch(PDOException $e){
    echo'ERROR: ' . $e->getMessage();
    die();
}

$tpl->display('table.tpl.php');

?>

Then in your template file (table.tpl.php in this example):

    Equipment Type
    Unit Number
    Last Updated
    CurrentHours
    Scheduled PM

<?php foreach($this->data as $row): ?>
    <?php $class = "bad";
    if ($row['nextPM'] - $row['currentHours'] > 100) {
        $class = "good";
    }
    elseif ($row['nextPM'] - $row['currentHours'] < 100 && $row['nextPM'] - $row['currentHours'] > 50) {
        $class = "soon";
    }
    ?>

    <?php echo htmlspecialchars($row['equipType']); ?>
    <?php echo htmlspecialchars($row['unitNumber']); ?>
    <?php echo htmlspecialchars($row['lastUpdate']); ?>
    <td class='<?php echo $class; ?>'><?php echo htmlspecialchars($row['currentHours']); ?>
    <td class='<?php echo $class; ?>'><?php echo htmlspecialchars($row['nextPM']); ?>

<?php endforeach; ?>

Note that I changed the while into a foreach because I personally don't like doing an assignment and comparison in the same statement. You can foreach over a PDOStatement (as far as I know) just as well as you can fetch from it.

EDIT: Also, you don't necessarily need to prepare the statment with the PDOs if there are no arguments. Of course, that's all up to preference again. If you have arguments, though, definitely use the prepared statements.


Approach to string split by character in Haskell I'm trying to learn Haskell and the mindset of programming functionally. I have started off by trying to understand the basics by writing code without any Monads in it.

So far, I think I'm getting used to the type system and how the basics works.

I thought to myself, why not try to get some feedback on what I have written to learn even more. So hopefully I will get some valuable information!

I have written a function which iterates a String and splits it into [String] by using given Char as a delimiter. Similar to what PHP's explode and Python's split does.

Here's what I have got so far:

main :: IO()
main = do
    print $ groupBy "/hejsan/asdas" '/' []
    print $ groupBy "/hejsan/asdas/hoho" '/' []
    print $ groupBy "/hejsan/asdas/hahsdhashdas" '/' []
    print $ groupBy "/hejsan/99/hahsdhashdas" '/' []
    print $ groupBy "/123/asdas/hahsdhashdas///" '/' []
    print $ groupBy "/123/asdas/hahsdhashdas//20////hej" '/' []

groupBy :: String -> Char -> [String] -> [String]
groupBy "" _ [] = []
groupBy (x:xs) c []
    | x == c = groupBy xs c []
    | otherwise = groupBy xs c [[x]]
groupBy "" _ (r:rs) = reverse result
    where result = if r == "" then rs else [r] ++ rs
groupBy (x:xs) c (r:rs)
    | x == c = groupBy xs c $ nextGroup ++ rs
    | otherwise = groupBy xs c $ [r ++ [x]] ++ rs
    where nextGroup = if r == "" then [r] else [""] ++ [r]

With output:

["hejsan","asdas"]
["hejsan","asdas","hoho"]
["hejsan","asdas","hahsdhashdas"]
["hejsan","99","hahsdhashdas"]
["123","asdas","hahsdhashdas"]
["123","asdas","hahsdhashdas","20","hej"]

So the main idea is to only show groups which have values in them. I bet there is:

Some library function which can simplify the functions internals
Some way to eliminate the 3rd argument so that it will be transparent to the user (Currying?)

I welcome all types of feedback except for feedback like "there is a library function which does all that for you". I'd like feedback on the internals and this particular problem is just an example

Well, sorry, but it is really tempting to note that there is in fact a library routine that can do that, and it is even called the same as your function:

groupBy (\a b -> b /= '/') "/hejsan/asdas"

This code with groupBy from Data.List will give you ["/hejsan","/asdas"]. Even though it should probably be noted that this is taking advantage of how groupBy is implemented internally, which might not be the best idea.

But let's look at your implementation - two things jump out:

You are clearly using the first element in the rs list differently from the rest. Why not make it an additional parameter so you can skip de- and recomposing the list all the time?
Why do you even need an accumulation parameter for your groups? Once you append something to rs, you already know that it will be reversed in the end and end up at the start of the return value. So you can simply prepend the group to the result of the recursive function call.

With the two changes and a bit of refactoring (replacing your if by pattern matches), we get the following version:

groupBy :: String -> Char -> String -> [String]
groupBy ""     _ "" = []
groupBy ""     _ r  = [r]
groupBy (x:xs) c ""
    | x == c        = groupBy xs c ""
    | otherwise     = groupBy xs c [x]
groupBy (x:xs) c r
    | x == c        = r : groupBy xs c ""
    | otherwise     = groupBy xs c (r ++ [x])

This can be improved further:

This has clearly two modes of operation depending on r == "". When calling you always know whether that is the case, so you could easily split it into two functions. One of them can get rid of the r parameter completely, so you end up with a nice function to call from outside.
Building a list using ++ is inefficient, as it creates a copy of the list each time you append an element, leading O(n²) complexity. It's better to use x:r and then reverse once in the end, which is a more efficient O(n). A bit more involved refactoring enables you to construct the string in the right form right away (see span).

That's really all the difference to the "perfect" library-level implementation (see the library code). Note that generally, initialization of accumulation parameters is simply done by defining a wrapper function, even though we can easily put this function without even needing it.

Hope this helps.


Download file uisng ASP.Net WebApi I am using following code to download a csv file using Web API. Maximum file size will be less than 500mb. How can I improve this code?

[HttpPost, Route("api/files/getfile")]
public HttpResponseMessage GetDataFileResponse(string filePath) {
    try {

        FileStream fileStream = File.OpenRead(filePath);
        long fileLength = new FileInfo(filePath).Length;

        var response = new HttpResponseMessage();
        response.Content = new StreamContent(fileStream);
        response.Content.Headers.ContentDisposition = new ContentDispositionHeaderValue("attachment");
        response.Content.Headers.ContentDisposition.FileName = "mydata.csv";
        response.Content.Headers.ContentType = new MediaTypeHeaderValue("application/octet-stream");
        response.Content.Headers.ContentLength = fileLength;
        return response;
    } catch (Exception e) {
        Console.WriteLine(e);
        throw;
    }
}

There can be two improvements in above code block initially as below:

Method should be async, this will help your application in terms of performance for parallel requests
In order to delete the large file, one should always prefer to download the file in chunks, again we are talking about parallel programming.


Filling modal from JSON (dynamically) My Java app converts its modal object to JSON (using the GSON Library).

This is just fine, but I have to get it back into a modal in PHP. Now I've created a modal (in my case it's, for example, GeoLocation) which has several getters and setters.

Example:

class GeoLocation {

    private $cellTowerId;
    private $dateCreated;
    private $dateModified;
    private $geoLocationId;
    private $geoLocationLatitude;
    private $geoLocationLongitude;
    private $geoLocationName;
    private $geoLocationStatus;
    private $geoLocationTime;
    private $isSynch;
    private $geoLocationAccuracy;
    private $geoLocationAltitude;
    private $geoLocationBearing;
    private $geoLocationProvider;
    private $geoLocationSpeed;
    private $geoStreet;
    private $geoPostalCode;
    private $geoLocality;
    private $geoCountryName;
    private $geoCountryCode;
    private $geoFeatureName;
    private $geoPhone;

    public function getCellTowerId() {
        return $this->cellTowerId;
    }

    public function getDateCreated() {
        return $this->dateCreated;
    }

    public function getDateModified() {
        return $this->dateModified;
    }

    public function getGeoLocationId() {
        return $this->geoLocationId;
    }

    public function getGeoLocationLatitude() {
        return $this->geoLocationLatitude;
    }

    public function getGeoLocationLongitude() {
        return $this->geoLocationLongitude;
    }

    public function getGeoLocationName() {
        return $this->geoLocationName;
    }

// and many more...
}

I have a loader class which processes the JSON and should re-create the object as soon as the set<Varname>() method exists.

Are there any "security" problems so far or any way to improve it?

public function process() {
    foreach (parent::getData() as $key) {
        try {
            $geolocation = new GeoLocation();
            foreach ($key as $keyName => $keyValue) {
                if (method_exists($geolocation, "set" . ucfirst($keyName))) {
                    call_user_func(array($geolocation, "set" . ucfirst($keyName)), $keyValue);
                }
            }
        } catch (Exception $ex) {
            echo $ex->getMessage();
        }
    }
}

Since the class methods are always synchronized with the Java classes, the getter and setter methods should always exist. Is there maybe a way of doing the getters and setters dynamically using the JSON?

If there is JSON data which has no methods, like setJSONDataOfEvilStuff(), then it's not necessarily so far.

There's nothing wrong with what you're doing... well, not as such.
I would, however, like to stress that methods do not echo. If an exception is thrown, and all you do is catch it to echo the message, then the loop (and the rest of the code) will continue to be executed as if everything was fine.

However, you don't know if there is any code to come that will attempt to set the header(), or set a cookie or something. That code will then break (because echo has sent output to the client). This will result in lots of errors popping up all over the place, and will leave you puzzled as to what actually is the piece of code where all evil begins.

If an exception is thrown, the calling code must be notified. Let it fly, and let the registered handlers deal with it, or let the app crash.

Next: DRY (do not repeat yourself). While there is little repetition in your code, there is this one tiny block that I'd like to focus on:

if (method_exists($geolocation, "set" . ucfirst($keyName))) {
    call_user_func(array($geolocation, "set" . ucfirst($keyName)), $keyValue);
}

Within the if statement, you're calling ucfirst and concatenating a new string. Only to repeat the same function call + concatenation if the expression evaluates to true.
The way you call the method is questionable, too. call_user_func is, again, a function call that will only forward to a method call. I'd simply write the code like so:

$setter = 'set'.ucfirst($keyName);//create setter name
if (method_exists($geolocation, $setter))
{
    $geolocation->{$setter}($keyValue);//invoke method directly
}

That, to my eye, looks a lot cleaner.

More general issue
A more general issue I have with this code, is where and how you are calling these setters. What you're basically doing is constructing a new instance, and setting its properties.
It might happen that you want to set the properties of an instance that already exists, too. I, therefore, tend to create a bulk-setter on the model itself.
Because json_decode returns (on success) either an associative array or an instance of stdClass, and because both are iterable, you could add a method like this to your models:

function setByJSON($jsonData)
{
    foreach ($jsonData as $name => $val)
    {
        $setter = 'set'.ucfirst($name);
        if (method_exists($this, $setter))
            $this->{$setter}($val);
    }
    return $this;
}

This is just a basic example, though. But as luck would have it, I have an actual, real-life example of models that do exactly what you're trying to do: extract data out of the parsed json, and map the properties to their own setters. check out the setAll method of this class as an example.

The entire wrapper is still under development, though, but the models are quite handy already. Oh, and if you are fed up with creating all those pesky models for json data, check out the entire repo: the Freshdesk\Tool\Generator generates a class, complete with getters and setters for each property, and adds type-hints where possible.


Checking if String has unique values I have a small function which checks if the given String has all 0's. This method would be used quite often and want to know if it's effectively written. Any help would be greatly appreciated.

 /*
 * Input parameters: Employee Number
 * Returns true if sum of employee number is greater than 0,
 * else false.
 */
private boolean employeeNumberCheck(String employeeNumber){
    try{
        if(employeeNumber.length()==0){
            return false;
        }
        int counter=0;
        for(int i=0;i<employeeNumber.length();i++){
            counter=counter+Integer.parseInt(String.valueOf(employeeNumber.charAt(i)));
        }
        if(counter==0)
            return false;
        return true;
    } catch (Exception someException){
        logger.info("Error from method employeeNumberCheck.\n"+someException);
        return false;
    }
}

In this case I would go the regex route. Your code is basically saying: Anything that parses as a digit is fine, just so long as everything is a digit, and at least one of them is not zero.

Express that as a regular expression:

private static final Pattern validEmployeeNumber = Pattern.compile("^[0-9]*[1-9][0-9]*$");

Then your function becomes:

private static final Pattern validEmployeeNumber = Pattern.compile("^[0-9]*[1-9][0-9]*$");

private static boolean employeeNumberCheck(String employeeNumber){
    return validEmployeeNumber.matcher(employeeNumber).matches();
}

See it running in ideone


Filter request code handler I use the following code which works OK, the code is filtering requests and in case there are too many requests it sends some logs/errors
since I'm very new to Java my question if there is a better way to write it especially the while statement ...

 public final class filterBu implements Filter {

        private static AtomicInteger reqcnt;
        private static final String MAX_CUNCCURENT = "maxConcurrent";
        private static final Logger logger = LoggerFactory.getLogger(ContentValidationFilter.class);
        private static final String EXCEED_MAX_REQUESTS_MSG = "maximum reaced";
        private static int maxCunnccurent;
        private static final int TOO_MANY_REQUESTS = 429;

        @Override
        public void doFilter(ServletRequest req, ServletResponse resp, FilterChain chain) throws IOException, ServletException {
            while (true) {
                int value = reqcnt.get();
                if (value >= maxCunnccurent) {
                    ((HttpServletResponse) resp).sendError(TOO_MANY_REQUESTS, EXCEED_MAX_REQUESTS_MSG);
                    return;
                }
                if (reqcnt.compareAndSet(value, value + 1)) {
                    break;
                }
            }
            try {
                chain.doFilter(req, resp);
            } finally {
                reqcnt.decrementAndGet();
            }

        }

        @Override
        public void init(FilterConfig config) throws ServletException, NumberFormatException {
            maxCunnccurent = Integer.parseInt(config.getInitParameter(MAX_CUNCCURENT));
            if (maxCunnccurent < 1) {
                logger.error("excced limit");
            }
            reqcnt = new AtomicInteger(0);
        }

    }

Replace the AtomicInteger with a Semaphore holding maxCunnccurent (maxConcurrent, right?) permits. So :

reqcnt = new Semaphore(maxCunnccurent);

Now, each time you get a filter request, you try to get a permit. If you do get a permit, you go ahead and filter, making sure that afterwards you release the permit again. If you don't get a permit, you send your error response :

boolean acquired = reqcnt.tryAcquire();
if (!acquired) {
    ((HttpServletResponse) resp).sendError(TOO_MANY_REQUESTS, EXCEED_MAX_REQUESTS_MSG);
    return;
}
try {
    chain.doFilter(req, resp);
} finally {
    reqcnt.release();
}


Algorithm to multiply two matrices #include <iostream>
#include <vector>

void function(std::vector<std::vector<int>>& matrix1, const std::vector<std::vector<int>>& matrix2)
{
    std::vector<std::vector<int>> matrix(matrix1.size(), std::vector<int>(matrix1.size()));
    int result = 0;
    int column = 0;
    if (matrix1.size() == matrix2.at(0).size() && matrix1.at(0).size() == matrix2.size())
    {
        for (int i = 0; i < matrix1.size(); ++i)
        {
            for (int j = 0; j < matrix1.at(0).size(); ++j)
            {
                result += matrix1.at(i).at(j) * matrix2.at(j).at(column);
            }
            matrix.at(i).at(column) = result;
            if (column != matrix1.size() - 1)
            {
                --i;
                ++column;
            }
            else
                column = 0;
            result = 0;
        }
    }
    matrix1 = matrix;
}
int main()
{
    std::vector<std::vector<int>> matrix1{ {1,0,2}, {-1,3,1} };
    std::vector<std::vector<int>> matrix2{ {3,1}, {2,1}, {1,0} };
    function(matrix2, matrix1);
}

The function (i.e. function) multiplies two matrix variables and changes the first parameter(i.e. matrix1). Is it possible to improve my algorithm?

I know that I can change variable names, but that isn't important. Its only an example. Maybe I can avoid two variable ints? I think that I always must create helper variables (matrix).

Well, first you should create your own class for matrices. Your current ad-hoc representation is inefficient and lacks abstraction.
Options include:

Row-major or column-major.
Restricted to specific dimensions / square matrices.
Using an array of pointers to rows/colums or explicit mapping of coordinates to ids.

Is there a reason you insist that the result-matrix should be square? Seems gratuitious.
There's no advantage to modifying the first input instead of returning a new matrix. So, make both arguments constant and return the result instead.
If you don't need an object any longer after copying it, might moving be more appropriate, as it's potentially more efficient and less likely to throw?


Generators in Javascript; Project Euler #2 I'm trying to learn javascript using project euler and I decided to try to force myself to learn about generators in javascript using problem number 2 which asks us to:  Find the sum of the even-valued terms in the fibonacci (f(n)) sequence such that f(n) < 4M , beginning with n=1 is 1, n=2 is 2, n=3 is 3 and so on

My code below presently works but I'm hoping to get some input as I began learning javascript this week. I'm coming from a python background and any input that keeps that in mind would be heavily appreciated!

function* fib_gen() {
    var current = a = b = 1;

    while (true) {
        current = b;

        yield current;

        b = a + b;
        a = current;
    }
}

function solution() {
    sequence = fib_gen();
    even_fibs_total = 0;
    cur = sequence.next().value;

    while (cur < 4000000) {
        if (cur % 2 == 0) even_fibs_total += cur;
        cur = sequence.next().value;
    }

    return even_fibs_total
}

var time_pre = performance.now()
document.write(solution())
document.write('<br>')
var time_post = performance.now()
document.write('completed in ' + Math.round((time_post - time_pre)*100)/100 + ' seconds')

1) You have a few undeclared variables which means they become global. You want to use var (or let or const) always. Compare this:

var current = a = b = 1; // a and b are globals

To:

var current, a, b; // all variables are local
current = a = b = 1;

Same here:

sequence = fib_gen(); // not declared
even_fibs_total = 0; // not declared
cur = sequence.next().value; // not declared

Compare to:

var sequence = fib_gen();
var even_fibs_total = 0;
var cur = sequence.next().value;

2) Conventions:

Name your variables with camelCase instead snake_case.
Use === instead of == unless you know you really really need == (tip: you probably don't).
Always write if statements with braces, even if one line, for better readability and refactoring.

3) Generators are iterators, and iterators can be iterated with the for..of loop. If your environment supports generators it probably supports for..of:

function solution() {
  var evenFibsTotal = 0;

  for (var cur of fibGen()) {
    if (cur >= 4000000) {
      break;
    }
    if (cur % 2 === 0) {
      evenFibsTotal += cur;
    }
  }

  return evenFibsTotal;
}

4) If you have ES6 support (or compilation via Babel) you can use destructuring assignment to simplify the fibonnaci function:

function* fibGen() {
  var a = 1;
  var b = 1;
  while (true) {
    yield b;
    [a, b] = [b, a + b];
  }
}


Decrease CPU usage in while loop I've a application which runs with two threads.  The main thread runs in one infinity-while-loop:

while (true) {
   try {
      ArrayList<Info> infoList = zabbixHandler.APIRequest();
      handleInfo.handleInformation(infoList);
      Thread.sleep(5000);
   }
}

The second thread, which uses more than 50% of my CPU, must be able to stop and run again, which I asked already with this question on Stack Overflow.

The solution works perfectly, but as I already said, it uses >50% of the CPU.  Here are the two loops of the solution:

private volatile boolean finished = true;
public void run() {
   Process p;

   while (true) {
        // loop until the thread is finished
        while (!finished) {
            try {

                // let every lamp shine...
                p = Runtime.getRuntime().exec(onCommand + "-green");
                p.waitFor();

                // ... for 0.5 seconds ...
                Thread.sleep(500);

                // ... then turn it off again ...
                p = Runtime.getRuntime().exec(offCommand + "-green");
                p.waitFor();
                // ... and let the next lamp shine
                p = Runtime.getRuntime().exec(onCommand + "-yellow");
                p.waitFor();

                Thread.sleep(500);

                p = Runtime.getRuntime().exec(offCommand + "-yellow");
                p.waitFor();
                p = Runtime.getRuntime().exec(onCommand + "-red");
                p.waitFor();

                Thread.sleep(500);

                p = Runtime.getRuntime().exec(offCommand + "-red");
                p.waitFor();
                p = Runtime.getRuntime().exec(onCommand + "-blue");
                p.waitFor();

                Thread.sleep(500);

                p = Runtime.getRuntime().exec(offCommand + "-blue");
                p.waitFor();

            } catch (InterruptedException e) {
                e.printStackTrace();
            } catch (IOException e) {
                e.printStackTrace();
            }
        }
    }
}

How could I reduce the use of CPU while retaining the functionality?

EDIT:
The finish element will be changed by a getter and setter. Because it is volaitle, it shouldn't get complications.

don't use a busy wait but instead use a wait condition (here using the built-in monitor in Object):

public void run() {
   while(true){
      while(!finished){
         // do stuff
      }
      try{
          synchronized(this){
              while(finished)
                   wait();//wait until notify gets called in startThread
          }
      }catch(InterruptedException e){}
   }

}

public synchronized void startThread() {
   finished = false;
   notify();//wake up the wait
}

I note that finished should remain volatile.

I put the wait() in the synchronized block in a while loop to avoid the race where the thread has just tested the first while condition and is about to enter the synchronized block but startThread() then gets called; putting finished back to false and the thread waiting anyway. This can lead to the thread blocking while waiting on a notify() that never happens.

To help any more I need to see what exactly happens inside that while loop.


Better way to achieve looped output result I have 4 variables - say A, B, C and D

Each variable can have 4 values - say 1, 2, 3 and 4

I need to generate all possible combinations of values

Example of Expected Output:

 A B C D
---------
 1 1 1 1
 1 1 1 2
 1 1 1 3
 1 1 1 4
 1 1 2 1
 1 1 2 2
 1 1 2 3
 .
 .
 .
 4 4 4 1
 4 4 4 2
 4 4 4 3
 4 4 4 4

I have wrote following loop

for ($A = 1 ; $A < 5 ; $A++){
 for ($B = 1 ; $B < 5 ; $B++){
  for ($C = 1 ; $C < 5 ; $C++){
   for ($D = 1 ; $D < 5 ; $D++){
    echo $A.$B.$C.$D;
   }
  }
 }
}

Is there any faster or better way to achieve above result?

Review

Your answer is perfectly fine for this usage, I would keep that.  If you were that deep in loops generally it would be a sign that something was wrong, but for this task it is perfectly okay.

I would recommend using upper case only for constants.

Over Optimisation

This is actually a lot simpler than the other answers are making it out to be.  The best answer in terms of performance (which is a silly thing to aim for) that I can think of is:

$end = pow(4, 4);

for($i = 0; $i < $end; ++$i)
{
    echo 1111 + base_convert($i, 10, 4), PHP_EOL;
}

Addition is quite a cheap operation for a processor to perform.  Comparing the benchmarks from Elias Van Ootegem's answer shows that the above code is significantly quicker.  This is due to the loop running only on the numbers that it needs to and not having to check a string.  It also does not introduce any pre-computed magic numbers.

OP Time taken:    0.00325608253479
Elias Time taken: 0.0021388530731201
Pauls Time taken: 0.0016560554504395

The code I used for the benchmark was:

// OP version
$start = microtime(true);
for ($a=1;$a<5;++$a)
    for ($b=1;$b<5;++$b)
        for ($c=1;$c<5;++$c)
            for ($d=1;$d<5;++$d)
                echo $a, $b, $c, $d, PHP_EOL;
                //comma's are more efficient then dots here
echo 'OP Time taken: ', microtime(true) - $start, PHP_EOL;

// Elias version
$start = microtime(true);
for($i=156;$i<625;++$i)
{
    $str = base_convert($i, 10, 5);
    if (strstr($str, '0') === false)
        echo $str, PHP_EOL;
}
echo 'Elias Time taken: ', microtime(true) - $start, PHP_EOL;

// Paul Version
$start = microtime(true);
$end = base_convert(3333, 4, 10) + 1;

for($i = 0; $i < $end; ++$i)
{
    echo 1111 + base_convert($i, 10, 4), PHP_EOL;
}
echo 'Pauls Time taken: ', microtime(true) - $start, PHP_EOL;

Overkill

If you do want to generalise the output you could create a function such as the one below (This is complete overkill, but demonstrates that the solution does not come from magic numbers.):

/**
 * Count in a base that has been offset from zero.
 *
 * @param int $digits The number of digits to use.
 * @param int $offset The offset from zero that the base uses.
 * @param int $base   The base to count in.
 */
function countInAnOffsetBase($digits, $offset, $base)
{
    $end = pow($base, $digits);
    $offset = str_repeat($offset, $digits);

    for ($i = 0; $i < $end; ++$i)
    {
        echo $offset + base_convert($i, 10, $base), PHP_EOL;
    }
}

countInAnOffsetBase(4, 1, 4);
// Or try 3, 4, 5 to count from 444 to 888 using the digits 4, 5, 6, 7, 8

This is almost as quick as the version I proposed at the top of this post.

Note

The function and method above assume that the number will not roll over the base (for base 10, the offset + base must be less than 10).  An even more general function would specify the output base:

/**
 * Count in a base that has been offset from zero.
 *
 * @param int $digits The number of digits to use.
 * @param int $offset The offset from zero that the base uses.
 * @param int $base   The base to count in.
 * @param int $base   The base to output the count in.
 */
function countInAnOffsetBase($digits, $offset, $base, $outputBase)
{
    if ($offset + $base > $outputBase)
    {
        throw new OverflowException(
            'offset and base cannot be greater than outputBase otherwise ' .
            'the value will overflow.');
    }

    $end = pow($base, $digits);
    $offset = intval(str_repeat($offset, $digits), $outputBase);

    for ($i = 0; $i < $end; ++$i)
    {
        echo base_convert(
            $offset + intval(base_convert($i, 10, $base), $outputBase),
            10,
            $outputBase),
            PHP_EOL;
    }
}


Bash script to backup a LAMP server I wrote a bash script for backing up my LAMP server. To be honest that's my first experience in bash so I want to ask you for help in its improvement.

#!/bin/bash

date=`(date +%Y%m%d%H%M)`

declare -A users=(
    ["user1"]="password1"
    ["user2"]="password2"
)

for user in ${!users[@]}
do
    directory=$user

    directories=`find /var/www/$directory -maxdepth 1 -mindepth 1 -type d`

    for directory in $directories
    do
        cd $directory

        if [ ! -d backups ]
        then
            mkdir backups
        fi

        tar --exclude ./backups -czf backups/$date.tar.gz .

        password=${users[$user]}

        databases=`mysql -u $user -p$password -Bse "SHOW DATABASES;"`

        for database in $databases
        do
            if [ $database = ${directory##*/} ]
            then
                mysqldump -u $user -p$password $database | gzip > backups/$date.sql.gz
            fi
        done

        chmod 770 backups -R
        chown $user:$user backups -R
    done
done

Use $(...) instead of `...`

`...` is archaic, error-prone, and harder to read than $(...).

For example, instead of date=`(date +%Y%m%d%H%M)`, write date=$(date +%Y%m%d%H%M).
Note that the extra (...) around the date command was unnecessary.

Double-quote variables used in command arguments

For example instead of for user in ${!users[@]},
write for user in "${!users[@]}".

This way, if the users array has keys with spaces in it,
the program will still work correctly.
In the current example none of the keys contain spaces (they are user1 and user2),
so you don't strictly need it,
but it's a good habit to build.

Don't parse the output of find

This line shows a very bad practice:

directories=`find /var/www/$directory -maxdepth 1 -mindepth 1 -type d`

Let's first correct the form:

directories=$(find "/var/www/$directory" -maxdepth 1 -mindepth 1 -type d)

The problem here is that if any of the files printed by the command contain a newline character, then it will be impossible to tell complete filenames from partial filenames in $directories. If any of the filenames contain whitespace, then parsing the value of $directories will be complicated.

It's best to avoid parsing the output of find when possible.
Even if this code works in your example because you know that the filenames will be strictly alphanumeric, it's good to avoid such bad practices if possible.

In this case there's a fairly simple alternative:

for directory in "/var/www/$directory"/*/

This is basically a simple glob expansion,
with the small trick that the trailing / will make it match only directories.

Notice that */ is outside the double-quoted part. This is because * would not be expanded within double-quotes. For the record, this is an equivalent way to write the above:

for directory in /var/www/"$directory"/*/

Finally, there is a small caveat: this is not exactly equivalent to the find command, because that would also find directories whose name starts with .,
and the glob expansion won't, by default.
If you need to support such filenames, then write shopt -s dotglob before the loop statement. That will enable matching filenames starting with . in glob expansions.

Avoid changing the working directory (with cd) in scripts

Changing directories with cd in scripts is error-prone:

It may become confusing what is the current working directory, at any point in the script
The cd command may fail, for example due to permission issues or filesystem errors. If such failures are not handled (as in the posted script), the program may happily continue executing the rest of the commands, which can have serious consequences. Consider for example what will happen to cd "$foo"; rm -fr * if the cd command fails: important stuff could get deleted!

In the posted code, instead of this:

cd $directory

if [ ! -d backups ]
then
    mkdir backups
fi

tar --exclude ./backups -czf backups/$date.tar.gz .

You could write without using cd like this:

mkdir -p "$directory/backups"
tar -C "$directory" --exclude ./backups -czf "backups/$date.tar.gz" .

That is, use the -C flag of tar to specify its effective directory,
and use the correct paths in other commands appropriately.
Keep in mind that the rest of the commands in the loop body that rely on the path of backups will also need to be adjusted accordingly.
For example change gzip > backups/$date.sql.gz to gzip > "$directory/backups/$date.sql.gz".

Notice that I double-quoted the parameters using the $date variable,
as explained earlier in this posted.
And I replaced a conditional statement by taking advantage of the -p flag of mkdir.

Don't give permissions willy-nilly

Using chmod with unnecessarily relaxed permissions is a very bad practice.
Question every permission bit if it's really needed and why.

Let's take a closer look at this:

chmod 770 backups -R
chown $user:$user backups -R

The chmod gives read-write-execute permission to the user and the group of files and directories under backups, recursively.
Why? The loop body creates .gz files under backups.
These files don't need to be executable.
Only the backups directory needs to be executable, so that the user can access its contents.

It would be better to set an appropriate umask before the loop.
You will be able to drop the chmod command,
because the umask, combined with the chown will be enough to ensure correct permissions.

Strictly speaking, the chown is only necessary for the newly created files and directories, instead of a blanket chown -R on the parent directory.
But this level of laziness is probably acceptable in a Bash script,
so until the chown -R becomes a practical concern (for example due to an excessively large number of files in the directory tree), I wouldn't complicate the script with more precise treatment.

Test your backup solutions

Can you spot the bug in this code?

for database in $databases
do
    if [ $database = ${directory##*/} ]
    then
        mysqldump -u $user -p$password $database | gzip > backups/$date.sql.gz
    fi
done

The $database variable is not used in the output filename.
If there are multiple databases,
only the last one will be saved.
A few months later, if you try to fix a corrupted database by recovering from backups,
you would be in for a terrible surprise,
discovering that some database backups are completely missing.

Be sure to test thoroughly your backup solution, accounting for the most complex supported scenarios.

(And as pointed out multiple times earlier, double-quote variables used in command arguments.)


Unique Email Addresses The task
was taken from leetcode

  Every email consists of a local name and a domain name, separated by
  the @ sign.

  For example, in alice@leetcode.com, alice is the local name, and
  leetcode.com is the domain name.

  Besides lowercase letters, these emails may contain '.'s or '+'s.

  If you add periods ('.') between some characters in the local name
  part of an email address, mail sent there will be forwarded to the
  same address without dots in the local name.  For example,
  "alice.z@leetcode.com" and "alicez@leetcode.com" forward to the same
  email address.  (Note that this rule does not apply for domain names.)

  If you add a plus ('+') in the local name, everything after the first
  plus sign will be ignored. This allows certain emails to be filtered,
  for example m.y+name@email.com will be forwarded to my@email.com.
  (Again, this rule does not apply for domain names.)

  It is possible to use both of these rules at the same time.

  Given a list of emails, we send one email to each address in the list.
  How many different addresses actually receive mails?

  Example 1:

  Input:
  ["test.email+alex@leetcode.com","test.e.mail+bob.cathy@leetcode.com","testemail+david@lee.tcode.com"]

  Output: 2

  Explanation: "testemail@leetcode.com" and "testemail@lee.tcode.com"
  actually receive mails

  Note:

  1 <= emails[i].length <= 100

  1 <= emails.length <= 100

  Each emails[i] contains exactly one '@' character.

  All local and domain names are non-empty.

  Local names do not start with a '+' character.

My declarative solution

/**
 * @param {string[]} emails
 * @return {number}
 */
var numUniqueEmails = emails => {
    return emails.reduce((validMails, mail) => {
      const names = mail.split('@');
      let [local, domain] = names;
      const iPlus = [...local].findIndex(x => x === '+');
      if (iPlus !== -1) { local = local.substr(0, iPlus); }
      const key = local.split('.').join('') + '@' + domain;
      if (!validMails.has(key)) { validMails.add(key); }
      return validMails
    }, new Set).size;
};

My Imperative solution

/**
 * @param {string[]} emails
 * @return {number}
 */
var numUniqueEmails2 = emails => {
  const validMails = new Set();
  for (const mail of emails) {
    let [local, domain] = mail.split('@');
    const iPlus = [...local].findIndex(x => x === '+');
    if (iPlus !== -1) { local = local.substr(0, iPlus); }
    const key = [...local].filter(x => x !== '.').join() + '@' + domain;
    if (!validMails.has(key)) { validMails.add(key); }
  }
  return validMails.size;
};

My solution with regex

/**
 * @param {string[]} emails
 * @return {number}
 */
var numUniqueEmails3 = emails => {
  const validMails = new Set();
  for (const mail of emails) {
    let [local, domain] = mail.split('@');
    local = local.replace(/\+(.*)$/, '')
      .replace(/\./g, '');
    console.log(local);
    const key = `${local}@${domain}`;
    if (!validMails.has(key)) { validMails.add(key); }
  }
  return validMails.size;
};

Addendum

in numUniqueEmails ran the code with this snippet:

  const names = mail.split('@');
  let [local, domain] = names;

and got 90th percentile. Running it again with this snippet:

  let [local, domain] = mail.split('@');

gives me 25th percentile. I'd assume it should be the opposite. Does anyone know why this is the case and also how come the difference is so big?

I don't know what you mean by declarative in this case. All these functions are pure (except the console.log), but the implementations of the functions are all imperative (which is fine of course)
When adding to a set, you don't need to check if the key exists first. Just add it.
Splitting the string into an array of chars isn't necessary. You can use string utils instead, for example indexOf('+'). You can also do a split('+')[0] to avoid having the iPlus variable.
If you swap the reduce for a map, and wrap that in a Set, you don't need to add to set explicitly. It's probably a bit slower though.
You could add some more newlines to the code to group related concepts, and make it more readable

I came up with the solution below:

const getUniqueEmailKey = email => {
    const [local, domain] = email.split('@')
    return local
        .split('+')[0] // Take everything before +
        .split('.').join('') // Remove dots
        + '@' + domain
}

const numUniqueEmails = emails => new Set(emails.map(getUniqueEmailKey)).size

About leetcode:
Try running the code a few more times. The time you get is a bit random...


Get the next bigger number by rearranging the digits of the input integer from itertools import permutations
def next_bigger(n):
    nlist = list(str(n))
    perm = permutations(nlist,len(str(n)))
    perm = set(perm)
    listofperm = []
    nlist = int("".join(nlist))
    for x in perm:
        form = int("".join(x))
        if form < nlist:
            continue
        else:
            listofperm.append(form)
    listofperm = sorted(listofperm)

    if (listofperm.index(n) + 1) == len(listofperm):
        indexofn = listofperm.index(n)
    else:
        indexofn = listofperm.index(n) + 1
    return listofperm[indexofn]

I'm trying to get the next bigger number by rearranging the digits of n but my code is very inefficient can you suggest ways to make my code more efficient and faster?
Inputs would be any integer.

It's not so much the implementation.
With problems like these (typical coding competition ones) it's always the same, that you have to find a clever algorithm instead of writing down as code a straightforward implementation of the problem description.
I haven't myself analyzed the problem, but I'd try to answer some questions:

Of all possible permutations, might it be enough to just exchange 2 digits? Or: Can you be sure that including a third digit into the exchange is irrelevant (result below the original number, result identical to the 2-digit case, or result higher than the 2-digit result, but not between the original number and the 2-digit result)?
If you can prove two digits to be enough, which are the two digits that give the smallest result (greater than the original number)?

Maybe it helps to do some examples by hand and see some pattern.


Flattening the nested Objects The original idea behind the problem is that the API I'm working with returns one type of a response for GET, and requires another type to do POST / PUT. The object I'm receiving from the GET has a format like this:

 const rawObject = {
  data1: {
    id: 1,
    value: 444
  },
  data2: null
 }

In order to be able to update it, or do a POST request to create a new one, I had to convert it into this format (This is the final output of my solution as well):

const convertedObject = {
  data1: 444,
  data2: null
};

The object I'm converting consists of another objects within it (One level max - as in the first example) or null. If the object is not null, I only care about the value of a nested object, therefore I flatten it as - data1: 444.

The solution I came up with is this (It works perfectly):

const rawObject = {
  data1: {
    id: 1,
    value: 444
  },
  data2: null
}

let convertedObject: any = {};

Object.entries(rawObject).map((item: any) => {
  const rawObjectKey = item[0];
  const rawObjectValue = item[1];
  if (rawObjectValue) {
    convertedObject = {...convertedObject, ...{[rawObjectKey]: rawObjectValue.value}};
    return;
  }
  convertedObject = {...convertedObject, ...{[rawObjectKey]: null}};
  return;
});
console.log(convertedObject); // Check the Output in example above

Stackblitz

I decided to use Object.entries() as I need both key:value pairs available. The if check I'm doing is there because object could have null value, therefore rawObjectValue would be null.

Review

Your final return statement is redundant.
The use of const is correct here (rawObjectKey, rawObjectValue) because you only assign these variables once. Many developers tend to use let or var here incorrectly.
Your method is a bit convoluted with those almost-equal code blocks. convertedObject = {...convertedObject, ...

Alternative

You could write this more compact, DRY and using built-in function reduce. We start with an empty object {} and inject each entry with the flattened data to obtain the result.

 const convertedObject = Object.entries(rawObject).reduce(
  (acc, item) => {
    acc[item[0]] = item[1] != null ? item[1].value : null;
    return acc;
  }, {});

Future Releases

There is a thing called optional chaining aka null propagation aka the Elvis operator. It's not standard Typescript (yet?), but is already supported in babel-plugin-proposal-optional-chaining.

This is a feature request to track. If ever implemented, it would allow us to call something like item[1]?.value:null instead of item[1] != null ? item[1].value : null.


Replacing specific words in text with words from another list python I've written a short script to use Reddit's PRAW API to search a subreddit for articles and if certain word/words are in the article title, replace those words with a random word taken from a list of other words.  I'm putting it in a dictionary because I'm going to write the url & article title to a database before tweeting it out so I don't send out duplicates.  I'm mostly curious if there's a better way to write the replacer() function.

For example: "World leaders brace for Trump's speech" --> "World leaders brace for spongebob's speech"

nicknames in matches() is a list of nicknames from another file.

def replacer(title, words, randoms):
    '''title is string you want to split.
       words is a list of words you are looking to match.
       randoms is a list of random words to replace with words.'''

    new_title = []
    for word in title.split(' '):
        if word in words:
            word = random.choice(randoms)
        new_title.append(word)

    return ' '.join(new_title)

def matches(login, subreddit):

    page = login.subreddit(subreddit)

    # used for the any statement
    combined_names = ["Name's", "Mister Name's", "Firstname Lastname", "Firstname", "Lastname", "Mister Firstname", "Mister Lastname"]

    # target words to replace
    names_apost = ["Name's", "Mister Name's"]
    names_no_apost = ["Firstname Lastname", "Firstname", "Lastname", "Mister Firstname", "Mister Lastname"]

    # for appropriate conjugation
    nicknames_apostrophe = [i + "'s" for i in nicknames]

    post_dict = {}
    top_posts = page.hot(limit=100)
    for post in top_posts:
        title = post.title
    if any(word in title for word in combined_names):
        first_iter = replacer(post.title, names_apost, nicknames_apostrophe)
        second_iter = replacer(first_iter, names_no_apost, nicknames)
        if post.ups > 1000:
            value = post.ups, post.url
            post_dict[second_iter] = value
    return post_dict

Note that the condition if word in words in replacer performs a linear search on words if it's a list, as in the current code. Since this condition is executed for every word in the input, it would be good to optimize it a bit. You could for example pass a set of words instead of a list.

The names first_iter and second_iter are not very meaningful. It would be good to come up with some better names for these.


OOP approach to using DOMDocument I've tried to write an "OOP" approach to printing a website using the DOMDocument class. I'm kind of stuck here because I feel like I am blundering in how I am passing and using the DOMDocument. I found that I can just return the object from the class and use it that way.

What are some safe practices here? Best approach? I know it's far from OOP, but I at least want to be considerate of the model while learning. I realize my approach is probably all backwards.  How bad is this code and what are some ways to improve this?

All styling is done in CSS, any and all critique is welcome about any aspect of the code.

If this isn't considered to be a useful question let me know and I will remove it.

<?php
require 'viewer.php'
//new viewer class
$v= new viewer();
//read html document structure from a template
$page= $v->setPage('template.html');
//find body
$body= $v->returnBody();
//create new element and attach it to body
$v->newElement('div', $body, "id=container");

$cont = $v->getElement("container");

$v->newElement('div', $cont, "id=title", "My Website");
$v->newElement('ul', $cont, "id=menu;class=menu");
$v->newElement('li', $v->getElement("menu"), NULL, "Forums" );

$v->newElement('div', $cont, "id=content");
$v->newElement('div', $v->getElement("content"), "id=content_title", "Content Title Here!");
$v->newElement('div', $v->getElement("content"), "id=content_box");

$mb = $v->getElement("content_box");
$v->newElement('a',$mb,NULL,"Some Link");

$content = " And after the link, this is the content.";

$dd=$v->rDOM();
$c=$dd->createTextNode($content);
$mb->appendChild($c);
$v->printHTML();
?>

Where the Viewer class is:

<?php
class viewer {
    protected $dom;

    public function __construct(){
        $this->dom = new DOMDocument();
    }

    //Creates an element of the type passed in string, appends parameters passed in string
    //and finally inserts into the passed node
    public function newElement($type, $insertInto = NULL, $params=NULL, $content=""){
        $element = $this->dom->createElement($type, $content);
        if(gettype($params) == "string" && strlen($params) > 0){
            $attrCollec = preg_split("/;/", $params);
            foreach($attrCollec as $attr){
                $value = preg_split("/=/", $attr);
                $element->setAttribute($value[0], $value[1]);
            }
        }
        $insertInto->appendChild($element);
    }
    public function getElement($param){
        //if($this->dom->validate()){
            $elem = $this->dom->getElementById($param);

            return $elem;
        //}
    }
    public function setPage($fileName){
        //does this even work? should validate before each getElement call?
        $this->dom->validateOnParse = true;
        $this->dom->loadHTMLFile($fileName);
    }
    public function printHTML(){

        echo $this->dom->saveHTML();
    }
    public function returnBody(){
        $nodeList = $this->dom->getElementsByTagName('body');
        $body = $nodeList->item(0);

        return $body;
    }
    public function rDOM(){
        return $this->dom;
    }
}
?>

Some things I spot on the fly:

Lack of documentation. PHPDoc works well, every method, every class, every file should have a DocBlock.
Naming conventions. It's considered good naming convention to have methodNames() and $variableNames in camelCase, but ClassNames and ObjectNames in UppercaseCamelCase.
Code style. Conforming to a single code style is great. Conforming to an established standard is awesome!. Check out the PSR Code style standards page.
On the newElement method, preg_split is redundant. explode by ; or = is enough, and is faster!
Last but not least, if the entire file is PHP (without embedding HTML), it's considered safe (and even recommended!) to omit the closing ?> at the end of the file. This helps with line break and whitespace issues.

I'll continue editing as I explore more.


Find and report all occurrences of duplicate lines in a text file I would like you to take a look at this simple script I wrote that's supposed to emulate the Unix sort [FILE] | uniq -cd command. What's different about my script though is that it also lists line numbers telling the user where occurrences of duplicate lines are located in the file. Please, tell me what you think as well as what parts of the script, if any, you think I should rework to make it better.

################################################################################
# File name: uniq.awk
# ===================
#
# Find and report all occurrences of duplicate lines in a text file.
#
#
# Usage: awk -f uniq.awk [FILE]
#
################################################################################
{
    x = lines[$0]["count"]++; # Count the number of occurrences of a line
    lines[$0]["NR"][x] = NR;  # Also save the number lines

    # Find the length of the longest line to make it the column width
    if (x > 0) {
        if (length($0) > max) {
            max = length($0);
        }
    }
}
END {
    # If the file contains no lines to process, that is, it's empty,
    # return an exit status code of 1 to indicate the fact.
    if (!(NR > 0)) {
        exit 1;
    }

    # Prepare the format string
    # Column #1: number of occurrences of the line
    # Column #2: line itself
    # Column #3: line numbers where all the lines are located
    fmt_s = "%s: %" max "-s (%s)\n";

    for (i in lines) {
        if (lines[i]["count"] > 1) {
            for (j = 0; j < lines[i]["count"]; j++) {
                s = s lines[i]["NR"][j] ", ";
            }
            # Get rid of the trailing comma and space
            s = substr(s, 1, length(s) - 2);
            printf(fmt_s, lines[i]["count"], i, s);
            s = "";
        }
    }
}

Test:

$ cat > data
car
baby
car
man
woman
woman
key
woman
$
$ cat -n data
     1  car
     2  baby
     3  car
     4  man
     5  woman
     6  woman
     7  key
     8  woman
$
$ awk -f uniq.awk data
2: car   (1, 3)
3: woman (5, 6, 8)

I don't know about Awk, but for this section of code:

END {
# If the file contains no lines to process, that is, it's empty,
# return an exit status code of 1 to indicate the fact.
if (!(NR > 0)) {
    exit 1;
}

You should print out an appropriate message to the terminal instead of just exiting with exit code 1. This, in my opinion, provides users with better feedback.


Connection to MySQL database I'd created a C# class to connect to a MySQL database (Connection.cs) and a separate class for the user validation (User.cs).

Connection.cs

public static MySqlConnection GetConnection()
{
    try
    {
        var connectionString = ConfigurationManager.ConnectionStrings["ConnectionString"].ConnectionString;
        _connection = new MySqlConnection(connectionString);
        _connection.Open();
    }
    catch (MySqlException ex) {

        switch (ex.Number)
        {
            //0: Cannot connect to server.
            case 0:
                MessageBox.Show("Cannot connect to server.  Contact administrator");
                break;
            //1045: Invalid user name and/or password.
            case 1045:
                MessageBox.Show("Invalid username/password, please try again");
                break;
        }

    }

    return _connection;
}

User.cs

public bool AuthenticateUser()
{
    string query = "SELECT * FROM users WHERE user_name = @userName LIMIT 1";
    bool password = false;
    try
    {
        Console.Write("connection");
        using (MySqlConnection connection = Connection.GetConnection())
        {
            Console.Write("Cmd");
            using (var cmd = new MySqlCommand(query, connection))
            {
                cmd.Parameters.AddWithValue("@userName", this.UserName);
                using (var reader = cmd.ExecuteReader())
                {
                    while (reader.Read())
                    {
                        password = BCrypt.Net.BCrypt.Verify(this._password + reader["salt"].ToString(), reader["password"].ToString());
                        this._user_id = int.Parse(reader["id"].ToString());
                    }
                }
            }
        }
    }
    catch (MySqlException ex)
    {
        throw new ArgumentException(ex.Message);
    }
    catch (Exception ex) {
        throw new ArgumentException(ex.Message);
    }

    return password;

}

I didn't created a method to dispose/close the connection anymore, since as far as I know, the using statement automatically disposes the type of IDisposable. I was looking for a suggestion before I continue coding for more functionalities.

A couple of issues I found:

GetConnection method:

var connectionString = ConfigurationManager.ConnectionStrings["ConnectionString"].ConnectionString;
_connection = new MySqlConnection(connectionString);
_connection.Open();

Here you're creating a new connection object and storing it in _connection. This is not defined anywhere in the code, but seems like a private field. This shouldn't be necessary, as you're simply returning it and leaving it to the caller to handle it. Just use a normal variable and get rid of the field:

MySqlConnection connection = new MySqlConnection(connectionString);
connection.Open();
return connection;

    catch (MySqlException ex) {

        switch (ex.Number)
        {
            //0: Cannot connect to server.
            case 0:
                MessageBox.Show("Cannot connect to server.  Contact administrator");
                break;
            //1045: Invalid user name and/or password.
            case 1045:
                MessageBox.Show("Invalid username/password, please try again");
                break;
        }

    }

This seems like a bad idea to me. For one, you're using MessageBox.Show to display errors, which ties it to the presentation layer. This breaks Separation of concerns, as the class should only do one thing. Leave it to the caller to display errors the way it likes.

Another issue is that any further detail about the error is lost, like the stack trace, message or any inner exceptions. A second, major issue is that any error with a number other than 0 or 1045 is simply shallowed and the caller has no idea it failed, leaving it with a closed, failed connection. My suggestion is to simply remove the try-catch althogether and make the caller handle errors as it finds appropriate.

AuthenticateUser method:

string query = "SELECT * FROM users WHERE user_name = @userName LIMIT 1";

A small issue with the LIMIT 1 is that it implies that there could be more than one user in the DB with the same username. That can be an indication of an issue on the DB design, lacking an UNIQUE key in the user_name column, it shouldn't be needed. Another problem is the use of SELECT * which brings more data than needed. Specify those columns you actually use and leave the rest out.

Console.Write("connection");

This is odd to put here. For one, other pieces of code refer to MessageBox to display errors and messages, while this uses the console. Why this inconsistency? Either use one display technology or the other, but clearly not both. As before, this creates a poor user experience in seeing random debug messages out of nothing. Most likely, this could be replaced by some kind of log to a file or similar, but remember to add more context to it, so you can extract some useful information out of the file.

cmd.Parameters.AddWithValue("@userName", this.UserName);

It's always nice to see properly parametrized queries. But remember that AddWithValue is a bad practice, declare the exact data type for the parameter and make sure it matches the server side declaration exactly.

while (reader.Read())

There is no need to loop here. As before, the query will return at most only 1 row, or nothing is the user name is wrong, so just replace it by a single call. if (!reader.Read()) return false; will do, and will exit early is no user exist.

password = BCrypt.Net.BCrypt.Verify(this._password + reader["salt"].ToString(), reader["password"].ToString());

Great to see a properly stored password and a nice validation! There is a small thing though. Keep the scope of the password variable as small as possible, possibly with a return at this point to avoid having to read until the end of the method. Also, "password" don't seems like a proper name for this one, something like "validPassword" would better describe what's going on.

this._user_id = int.Parse(reader["id"].ToString());

This is possibly the gravest problem. The method uses class fields all other the place (to input the username and password and to return the user id), which seems odd to me, because that ties the result of this method to the state of the class, which can be used elsewhere (although we don't have that much context here). Why not use method parameters and return values instead? So you can get rid of all private class-level variables. I find a method declaration like this to be much better:

public int? AuthenticateUser(string userName, string password)

The username and password are passed as parameters instead of class fields, and the return value is the id of the logged user or null if nothing is found. This clearly expresses what's going on and what the method expects and return. Also, this allows to simplify the body by doing something like this:

//Check if the password matches, and exit if not
if(!BCrypt.Net.BCrypt.Verify(this._password + reader["salt"].ToString(), reader["password"].ToString())) return null;
//We've got a valid user and password, return its id
return int.Parse(reader["id"].ToString());

   catch (MySqlException ex)
    {
        throw new ArgumentException(ex.Message);
    }

Once again the same problem. The exception is caught and a new one is thrown. But here the problem is that it's misleading. For one, the type ArgumentException is wrong, you didn't make a mistake in the arguments, something blew up while contacting the DB. Moreover, you keep only the message, but you discard the stacktrace, any inner exception and DB-specific exception information which can help debugging. This is will likely give you problems later on. My suggestion is to simply remove the try-catch and leave it to the caller to handle the exception and log, display something or retry. At most, throw a specific exception adding some context, but keep all of the original exception as an inner exception.


Calculate the number of palindrome numbers in the given ranges I've written a program that calculates all of the palindrome numbers in the given range, but the code is slightly slower than needed. I've tried to improve algorithmic complexity to the best of my abilities, but still  overhead over passing solution (1 second). Is it possible to improve the runtime performance (in time) of the code?

  \$n\$ - number of tests

  \$a, b\$ - range in which I need to calculate number of palindromes

Here is the code:

#include<stdio.h>
int ifpalin(int g)
{
    int rev=0;
    int tmp=g;
    while(tmp>0)
    {
        rev=rev*10+(tmp%10);
        tmp=tmp/10;
    }
    if(rev==g)
        return 1;
    else
        return 0;
}
int findpalin(int a1,int b1)
{
    int sm=0;
    for(int i=a1;i<=b1;i++)
    {
      if  (ifpalin(i)==1)
        sm++;
    }
    printf("%d",sm);
    printf("\n");
    return 0;
}
int main()
{
int a,b,n;
    scanf("%d",&n);
    for(int i=0;i<n;i++)
    {
        scanf("%d",&a);
        scanf("%d",&b);
        findpalin(a,b);
    }
    return 0;
}

Need new algorithm

The current algorithm you are using takes \$O(n \log m)\$ time, where \$n\$ is the size of the range and \$m\$ is the larger end of the range.  For example, if you are trying to find the number of palindromes between 1 and 1000, you iterate through all 1000 numbers (\$n\$) and for each number you do a palindrome check that iterates through the 3 digits of the number (\$\log m\$).  Given a large range, such as 1..1000000000, your program will be iterating for a long time.

Can be done in \$O(\log m)\$ time

This problem can actually be done in logarithmic time.  First, change the problem to finding the number of palindromic numbers between 1 and n.  Then the answer to the actual problem will be num(b) - num(a-1).

Now, to find how many palindromes there are between 1 and n, there is a trick you can use.  You don't have to count/find actual palindromes.  You can just look at the front half of the given number to get an estimate of the number of palindromes.

Example for even number of digits

How many palindromes are there between 1 and 12345678?

If you take the front half ("prefix") of the number, i.e. 1234, you can use that to estimate the number of palindromes.  Each number between 1 and 1234 can "generate" two palindromes: one with the last digit not repeated (making an odd number of digits), and one with all digits repeated (making an even number of digits).

Example: take the number 25.  This number can generate two palindromic numbers:

25 -> 252   (the last digit 5 is not repeated = odd number of digits)
25 -> 2552  (all digits repeated = even number of digits)

If you take all the numbers between 1 and 1234, you can generate 1234*2 palindromes this way.  You can also easily prove that all of the generated palindromes are less than 12345678.

However, there are also some palindromes that are missing.  These are generated by prefixes greater than 1234.  For each of the prefixes from 1235..9999, there is another palindrome that should be counted.  For example, take the number 2345.

2345 -> 2345432  (should be counted)
2345 -> 23455432 (too large)

So in addition to the prefix*2 palindromes already counted, we need to add 10^prefixLength - prefix - 1 extra palindromes.  The final answer is:

number of palindromes = prefix * 2 + 10^prefixLength - prefix - 1

Example for odd number of digits

How many palindromes are there between 1 and 123456789?

The answer for an odd length number is similar for an even length number.  I won't go through the whole proof, but here are the differences:

The prefix length is rounded up.  For the example, the prefix is 12345.
Some of the prefix*2 palindromes generated are not valid because they are larger than the original number.  All of the prefixes from 10000..12345 generate an extra prefix that is too large.  So there is an adjustment to subtract from the count of prefix - 10^(prefixLength-1) + 1.

The final answer for an odd length is:

number of palindromes = prefix * 2 - prefix + 10^(prefixLength-1) - 1

Final adjustment

I skipped one detail in all of the above.  Sometimes you need to subtract one from the prefix because the palindrome it generates is less than the original number.  For example, for 12345678, 1234 is the correct prefix to use because 12344321 is less than 12345678.  But if the original number were 12340000, then 1234 would be too high because 12344321 is greater than 12340000.  So you would use 1233 instead.

Example program

Here is a program that utilizes the method above to solve the problem in \$O(\log m)\$ time:

// Counts the number of palindromic numbers between two given numbers a and b.
//
// 121 is palindromic, 123 is not.
//
// Input is "n", the number of tests, and then n pairs of "a b".

#include <stdio.h>

static int numPalindrome(int num);
static int constructPalindrome(int palPrefix, int numLength);

int main(void)
{
    int a = 0;
    int b = 0;
    int n = 0;

    scanf("%d", &n);
    while (n-- > 0) {
        scanf("%d %d", &a, &b);
        printf("%d\n", numPalindrome(b) - numPalindrome(a-1));
    }
    return 0;
}

// Returns the number of palindromes between 1 and num.  If the input is 0,
// this function will return 0.
static int numPalindrome(int num)
{
    int numLength = 0;
    int palLength = 0;
    int palPrefix = 0;
    int temp      = 0;
    int i         = 0;

    // Example 1: num = 12345678
    // Example 2: num = 123456789

    // Find the length of the number, in digits.  Examples: 8 and 9
    for (numLength=0, temp = num; temp != 0; temp /= 10)
        numLength++;

    // Find the palindrome prefix, which is the front half of the number,
    // rounding up the length.
    //
    // Examples: palLength = 4    and 5
    //           palPrefix = 1234 and 12345
    palLength = (numLength+1) / 2;
    palPrefix = num;
    for (i=0; i < numLength - palLength; i++)
        palPrefix /= 10;

    // Check whether the palindrome formed by palPrefix is greater than num.
    // If it is, we subtract one from palPrefix because the last palindrome
    // is not usable.
    //
    // Example 1: Is 12344321  greater than 12345678?
    // Example 2: Is 123454321 greater than 123456789?
    if (constructPalindrome(palPrefix, numLength) > num)
        palPrefix--;

    // So right now, we have palPrefix being 1234 and 12345 for the two
    // examples.  The number of palindromes less than num is close to
    // palPrefix*2.  The reason is, for each number from 1..palPrefix, you can
    // can construct two palindromes, one of odd length where the last digit
    // is not repeated, and one of even length where all digits are repeated.
    //
    // 25  -> 252    2552
    // 256 -> 25652  256652
    //
    // Starting with all these, palindromes, we adjust the count depending
    // on whether the original number had an even or odd number of digits.
    //
    // For even numLength, we are missing some palindromes, for example:
    //
    // num       = 12345678
    // palPrefix = 1234
    //
    // We can create a palindrome with prefix higher than 1234 that is still
    // less than num.  For example, choose 2345:
    //
    // 2345 -> 2345432
    //
    // So for each number from 1235..9999, we should add one to the count.
    // In other words, add 10^prefixLength - 1 - palPrefix to the count.
    //
    // For odd numLength, we have too many palindromes.  Some of the
    // palindromes already created are not usable.  For example:
    //
    // num       = 12345678
    // palPrefix = 12345
    //
    // 12345 -> 1234554321 (too big, 10 digits)
    //
    // So for all the max digit prefixes, we can only use 1 of the 2
    // palindromes.  So we should subtract some from the count.  The number we
    // need to subtract in this case is: (12345 - 9999).  In other words,
    // subtract palPrefix - (10^(prefixLength-1) - 1) prefixes.
    palPrefix *= 2;
    if (numLength & 1) {
        // Subtract off adjustment for odd length.
        int adjustment = 1;
        for (i=1;i<palLength;i++)
            adjustment *= 10;
        palPrefix -= (palPrefix/2 - adjustment + 1);
    } else {
        // Add adjustment for even length.
        int adjustment = 1;
        for (i=0;i<palLength;i++)
            adjustment *= 10;
        palPrefix += (adjustment - 1 - palPrefix/2);
    }
    return palPrefix;
}

/**
 * Constructs a palindrome from a prefix.  Examples:
 *
 * Prefix = 1234  numLength = 8, returns 12344321
 * Prefix = 12345 numLength = 9, returns 123454321
 */
static int constructPalindrome(int palPrefix, int numLength)
{
    int front = palPrefix; // Starts at 1234, builds to 12340000
    int back  = 0;         // Starts at 0   , builds to 4321

    // If the number length is odd, we do not want to repeat the last digit
    // of the prefix, so skip it here.
    if (numLength & 1)
        palPrefix /= 10;

    // For each digit in the remaining prefix, remove the last digit and
    // place it on the back of "back".  In other words, we are reversing
    // palPrefix into "back".
    while (palPrefix != 0) {
        int digit = palPrefix % 10;

        palPrefix /= 10;
        back       = back * 10 + digit;
        front     *= 10;
    }
    return front + back;
}


Converting Shell input into Windows DOS compliant input I have a shell and batch script to make specific calls to a MBean on a JMX server. The shell input args are not working in windows because windows handles " differently than linux.

Assuming this is a bean name : bean = "hostName=\"localhost\",name=\"foobar\"\""; and the whole input args on linux is something like this:

String[] params = new String[]{"-srv", "localhost:1234", "-usr", "foo", "-pwd", "bar", "-b", bean};

This works perfectly fine on a shell but not for windows.

I wrote one method to convert the bean name to be windows dos compliant. This is essentially accomplished by adding 2 double quotes to a single double quote and adding only 2 double quotes to two double quotes:

" -> """ and "" -> """"

  public static void main(String[] args) throws Exception
  {

    String bean = "hostName=\"localhost\",name=\"foobar\"\"";
    String[] params = new String[]{"-srv", "localhost:1234", "-usr", "foo", "-pwd", "bar", "-b", bean};

    for ( int i = 0; i < params.length; i++)
    {
      if (params[i].equals("-b"))
      {
        // this is most likely the bean name
        String beanName = params[i + 1];
        StringBuilder dosCompliantName = new StringBuilder();
        // commence magic to make double quotes (") correct for windows
        for (int j = 0; j < beanName.length(); j++)
        {
          char c = beanName.charAt(j);
          if (c == '"')
          {
            try
            {
              if (beanName.charAt(j + 1) != '"')
              {
                // this is only one double quote so we add 2 additional double quotes to it
                dosCompliantName.append(c);
                dosCompliantName.append(c);
                dosCompliantName.append(c);
              }
              else if (beanName.charAt(j + 1) == '"')
              {
                // we have 2 double quotes here so we add only one because the next loop run will then also add only
                // one and in the end we have 4 double quotes:
                // "" -> """ -> """"
                dosCompliantName.append(c);
                dosCompliantName.append(c);
              }
            }
            catch (StringIndexOutOfBoundsException e)
            {
              // we have reached the end and the last char was a double quote so we add 2
              dosCompliantName.append(c);
              dosCompliantName.append(c);
            }
          }
          else
          {
            dosCompliantName.append(c);
          }
        }

        System.out.println(dosCompliantName);
        params[i + 1] = dosCompliantName.toString();
      }
    }
  }

The input/output for Linux and Windows on the console should look like this:

Linux : hostName="localhost",name="foobar""

Windows:hostName="""localhost""",name="""foobar""""

This looks like to work but to me it looks terribly inefficient, ugly and the readability is also not really great. How can I improve this?

Note: of course this whole stuff is extracted into its own method but for the sack of demonstration I put it into the main method

First some comments:

using Exception to control flow of the program is considered to be
a bad programming style - exceptions represent an exceptional state
and checking the condition of being at the end of the string is not
an exception
command line arguments' names are actually constant as long as the
interface they represent remains unchanged, so they should be
represented as constants and not string literals.
there is also only a limited set of them, so they should be
implemented as an Enum.

I assume you get the arguments as a String array - given the above you should first transform it into a Map - the most efficient would be an EnumMap, but I would suggest using a LinkedHashMap which will preserve the order of the arguments.

This will also validate the input and only accept valid arguments, thus preventing the program to crash because of an invalid input.

Now what you actually do to the beanName is basically enclosing a single or double apostrophe within a pair of apostrophes. There is a way to do it without iterating over the string variable character by character - use a regular expression, all you need is:

beanName = beanName.replaceAll("([\"]{1,2})", "\"$1\"")

Now this means "find all groups of single or double apostrophes and replace each of them a with starting apostrophe followed by the found group and an ending apostrophe". This works only for single or double apostrophes - so does probably the right thing for your use case.

To enclose any number of input apostrophes in a pair of the same, you'd use:

beanName = beanName.replaceAll("([\"]+)", "\"$1\"")

meaning "find all groups consisting of one or more apostrophes and replace each of them a with starting apostrophe followed by the found group and an ending apostrophe". Now try to do this without a regular expression - it would require much more complicated code.

All this will reduce the very act of processing the beanName to a mere one liner if you exclude checking for null:

public static enum Param {
  SRV("-srv"), USR("-usr"), PWD("-pwd"), BEAN("-b");

  private final String paramString;

  private Param(String paramString) {
    this.paramString = paramString;
  }

  public String getParamString() {
    return paramString;
  }

  public static Param paramOfValue(String value) {
    for (Param p : values()) {
      if (p.paramString.equals(value)) {
        return p;
      }
    }
    return null;
  }
}

public static void main(String[] args) throws Exception {

  // the bean name to test
  String bean = "hostName=\"localhost\",name=\"foobar\"\"";

  // input is always a String array
  String[] params = new String[] { "-srv", "localhost:1234", "-usr", "foo", "-pwd", "bar", "-b", bean };

  // convert the String array to a Map - only elements which have a predecessor
  // being a valid argument are being put into the map
  Map<Param, String> paramsMap = new LinkedHashMap<>();
  Arrays.stream(params).reduce((first, second) -> {
    Param param = Param.paramOfValue(first);
    if (param != null) {
      paramsMap.put(param, second);
    }
    return second;
  });
  // process the bean's name if present
  String beanName = paramsMap.get(Param.BEAN);
  if (beanName != null) {
    paramsMap.put(Param.BEAN, beanName.replaceAll("([\"]{1,2})", "\"$1\""));
  }
  System.out.println(paramsMap);

  // create a String array out of the map
  String[] processedParams = paramsMap
      .entrySet()
      .stream()
      .map(e -> Stream.of(e.getKey().getParamString(), e.getValue()))
      .flatMap(e -> e)
      .collect(Collectors.toList())
      .toArray(new String[0]);
  System.out.println(Arrays.toString(processedParams));
}

The output is:

{SRV=localhost:1234, USR=foo, PWD=bar, BEAN=hostName="""localhost""",name="""foobar""""}
[-srv, localhost:1234, -usr, foo, -pwd, bar, -b, hostName="""localhost""",name="""foobar""""]

The code above is less efficient performance-wise since it does more - yet it is safer and much more readable (and IMO elegant :-)). Simplicity and readability reduces errors and thus testing and maintenance costs.

If you refactor processing of the input parameters (validation, conversion into a map) and creating the String array out of the method it gets very short, very straightforward and more efficient. Since the input validation should be done anyway its cost shouldn't actually be taken into account.

Now the command line processing is pretty ad-hoc, there are better ways to do it - for example using org.apache.commons.cli.


Indexing columns to make a MySQL query faster I have this query here that I am trying to make more efficient as it is taking an extremely long amount of time, about 40s on 3 million records.

The query basically takes X amount of rows per company and turns them into columns for a given existing column.

SQLFiddle

SET @sql = NULL;
SET @sql1 = NULL;
SET @sql2 = NULL;
SET @sql3 = NULL;
SET @sql4 = NULL;
SET @sql5 = NULL;
SELECT GROUP_CONCAT(DISTINCT CONCAT( 'max(case when year = ', year, ' then experience_rate end) AS `', year, '-Pen`' ) ORDER BY year ) INTO @sql1 FROM spooner_pec;
SELECT GROUP_CONCAT(DISTINCT CONCAT( 'max(case when year = ', year, ' then mco_name end) AS `', year, '-MCO`' ) ORDER BY year ) INTO @sql2 FROM spooner_pec;
SELECT GROUP_CONCAT(DISTINCT CONCAT( 'max(case when year = ', year, ' then premium_range end) AS `', year, '-Prem`' ) ORDER BY year ) INTO @sql3 FROM spooner_pec;
SELECT GROUP_CONCAT(DISTINCT CONCAT( 'max(case when year = ', year, ' then employer_rating_plan end) AS `', year, '-Rating`' ) ORDER BY year ) INTO @sql4 FROM spooner_pec;
SELECT GROUP_CONCAT(DISTINCT CONCAT( 'max(case when year = ', year, ' then risk_group_number end) AS `', year, '-Gr Num`' ) ORDER BY year ) INTO @sql5 FROM spooner_pec;
SET @sql = CONCAT(
    'SELECT policy_number AS PolicyNumber,
    coverage_status_code As CoverageStatusCode,
    primary_name AS PrimaryName,
    primary_dba_name AS DBA,
    address1 AS Address1,
    address2 AS Address2,
    city AS CityName,
    state AS StateID,
    zipcode AS ZipCode,
    zip_plus_four AS ZipCode4,
    business_area_code AS PhoneAreaCode,
    business_phone AS PhoneNumber,
    business_extension AS PhoneExtension,
    business_contact_first_name AS FirstName,
    business_contact_last_name AS LastName,
    county_description AS County, ', @sql1, ', ', @sql2, ',', @sql3, ',', @sql4, ',', @sql5, '
    FROM spooner_pec GROUP BY policy_number');
PREPARE stmt FROM @sql;
EXECUTE stmt;
DEALLOCATE PREPARE stmt;

One of my coworkers mentioned indexing but I think the columns that are taking the longest are the ones that I am creating to pivot the data. Is there any way to index those?

Currently your code scans the spooner_pec table 6 times, once each for the year-based columns, and again for the final result.

You can reduce this to just 2 times by caching the list of years in a temp table:

CREATE TEMPORARY TABLE GotYears (year varchar(10));

INSERT INTO GotYears
SELECT DISTINCT year
FROM spooner_pec

Now you have a small table with just a few years in it, instead of 3,000,000 records. You have scanned the table just once to do that too.

Now use that table to build your SQL select clauses....

SELECT GROUP_CONCAT(CONCAT(
     'max(case when year = ', year, ' then experience_rate end) AS `', year, '-Pen`' )
ORDER BY year ) INTO @sql1 FROM GotYears;

Having made that recommendation, though, I also recommend having an index on the year column which will make the scan a whole lot faster too.

The query is still really ugly ™ but that's a problem in your application and data storage.

Additionally, your final query is horrible, horrible, and horrible. A group-by query with many columns, but only grouping by the policy_id is very messy, and is MySQL specific. The values returned in the other columns are unspecified in terms of behaviour. I hope all your values in all the columns are the same or otherwise your output values are unspecified. It is a problem with your database structure too, that the data is not normalized. Why do you record the same values for the same policies in multiple records? Your database may be a data warehouse, but even then it is a bit of a mess.

You should audit your tables, and reconsider the design and normalization of your tables.

Still, if you apply the changes I suggest (add the year index, and have a single temp table for the year values), then I imagine your query will execute in about 5 seconds instead of 30.