Return to Answer

deleted 7 characters in body

Source Link

edited May 9, 2018 at 10:49

There are severals flaws in the current approach:

The program doesn't check if the generated key already exists for a different URL. And it might. And when it does, the new value will overwrite the old one. The old value will be no longer retrievable.
encode returns a different output for the same input, with very high probability. In fact you can make the program run out of memory by calling encode on the same string enough times.
The implementation wastes a lot of memory. There's no need to use the full tiny URL as the key of the cache, the unique unique part would be enough, and divide the storage size of the keys by 3.
It's unrealistic to expect a URL shortening service to run forever without server reboots. Since this implementation doesn't use any storage outside RAM, all previously stored URLs will be completely lost when the process is restarted.

There are many technical implementation flaws:

No need to create a new Random in every call of encode. Better use a shared instance.
Magic numbers and strings should be constants or class parameters: the target key length (6), the alphabet.
The names are not great. urlLen is not the length of a URL, but the length of the unique part only (6 letters). randChars are not "random chars", but an alphabet from which to pick a random character.
It's a bad practice to print to stdout from methods that are not particularly designed to print stuff.
Prefer to declare variables using interface types: Map instead of HashMap.
Using a StringBuilder is overkill, a simple concatenation with + would be better.

There are severals flaws in the current approach:

The program doesn't check if the generated key already exists for a different URL. And it might. And when it does, the new value will overwrite the old one. The old value will be no longer retrievable.
encode returns a different output for the same input, with very high probability. In fact you can make the program run out of memory by calling encode on the same string enough times.
The implementation wastes a lot of memory. There's no need to use the full tiny URL as the key of the cache, the unique unique part would be enough, and divide the storage size of the keys by 3.
It's unrealistic to expect a URL shortening service to run forever without server reboots. Since this implementation doesn't use any storage outside RAM, all previously stored URLs will be completely lost when the process is restarted.

There are many technical implementation flaws:

No need to create a new Random in every call of encode. Better use a shared instance.
Magic numbers and strings should be constants or class parameters: the target key length (6), the alphabet.
The names are not great. urlLen is not the length of a URL, but the length of the unique part only (6 letters). randChars are not "random chars", but an alphabet from which to pick a random character.
It's a bad practice to print to stdout from methods that are not particularly designed to print stuff.
Prefer to declare variables using interface types: Map instead of HashMap.
Using a StringBuilder is overkill, a simple concatenation with + would be better.

There are severals flaws in the current approach:

The program doesn't check if the generated key already exists for a different URL. And it might. And when it does, the new value will overwrite the old one. The old value will be no longer retrievable.
encode returns a different output for the same input, with very high probability. In fact you can make the program run out of memory by calling encode on the same string enough times.
The implementation wastes a lot of memory. There's no need to use the full tiny URL as the key of the cache, the unique part would be enough, and divide the storage size of the keys by 3.
It's unrealistic to expect a URL shortening service to run forever without server reboots. Since this implementation doesn't use any storage outside RAM, all previously stored URLs will be completely lost when the process is restarted.

There are many technical implementation flaws:

No need to create a new Random in every call of encode. Better use a shared instance.
Magic numbers and strings should be constants or class parameters: the target key length (6), the alphabet.
The names are not great. urlLen is not the length of a URL, but the length of the unique part only (6 letters). randChars are not "random chars", but an alphabet from which to pick a random character.
It's a bad practice to print to stdout from methods that are not particularly designed to print stuff.
Prefer to declare variables using interface types: Map instead of HashMap.
Using a StringBuilder is overkill, a simple concatenation with + would be better.

Source Link

answered May 5, 2018 at 13:30

There are severals flaws in the current approach:

The program doesn't check if the generated key already exists for a different URL. And it might. And when it does, the new value will overwrite the old one. The old value will be no longer retrievable.
encode returns a different output for the same input, with very high probability. In fact you can make the program run out of memory by calling encode on the same string enough times.
The implementation wastes a lot of memory. There's no need to use the full tiny URL as the key of the cache, the unique unique part would be enough, and divide the storage size of the keys by 3.
It's unrealistic to expect a URL shortening service to run forever without server reboots. Since this implementation doesn't use any storage outside RAM, all previously stored URLs will be completely lost when the process is restarted.

There are many technical implementation flaws:

No need to create a new Random in every call of encode. Better use a shared instance.
Magic numbers and strings should be constants or class parameters: the target key length (6), the alphabet.
The names are not great. urlLen is not the length of a URL, but the length of the unique part only (6 letters). randChars are not "random chars", but an alphabet from which to pick a random character.
It's a bad practice to print to stdout from methods that are not particularly designed to print stuff.
Prefer to declare variables using interface types: Map instead of HashMap.
Using a StringBuilder is overkill, a simple concatenation with + would be better.